Thread: pg_get_triggerdef in pg_dump
Is there any point using pg_get_triggerdef in pg_dump to generate trigger definitions? We'd still have to keep the old code so that we can dump pre 7.4, but it might mean we don't have to touch that code again if we add triggers on columns or something... Also, it doesn't format them as nicely as the current pg_dump code... Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > Is there any point using pg_get_triggerdef in pg_dump to generate trigger > definitions? We'd still have to keep the old code so that we can dump pre > 7.4, but it might mean we don't have to touch that code again if we add > triggers on columns or something... Seems like a good idea to me --- we've been trying to reduce pg_dump's knowledge of backend nitty-gritty, and this would be another small step in the right direction. > Also, it doesn't format them as nicely as the current pg_dump code... That's fixable no? I guess you might want to consider what psql's \d display will look like too, but I don't recall that we ever promised anyone that the pg_get_xxx functions would output no unnecessary whitespace. regards, tom lane
> Seems like a good idea to me --- we've been trying to reduce pg_dump's > knowledge of backend nitty-gritty, and this would be another small step > in the right direction. > > > Also, it doesn't format them as nicely as the current pg_dump code... > > That's fixable no? I guess you might want to consider what psql's \d > display will look like too, but I don't recall that we ever promised > anyone that the pg_get_xxx functions would output no unnecessary > whitespace. We make pg_get_xxx2 functions that return a formatted version. Internally, we just add an extra boolean parameter to the pg_get_triggerdef() function in ruleutils and we call that true or false depending... Chris
Christopher Kings-Lynne wrote: > >We make pg_get_xxx2 functions that return a formatted version. Internally, >we just add an extra boolean parameter to the pg_get_triggerdef() function >in ruleutils and we call that true or false depending... > > > That's what I got too! Several weeks ago I proposed such functions as contribute module to this list, with no result. Seems I'm not the only one that wants to read his/her trigger/view/rules after pushing them into pgsql... Difference from Christopher's solution is that mine utilizes completely separatated (copied) code, so ruleutils code is still unchanged. This was a concession to Tom who claimed concerns about pg_dump not being able to reproduce things correctly if there was *any* error in it. Maybe we get some progress now on this topic? Regards, Andreas
Andreas Pflug <Andreas.Pflug@web.de> writes: > Difference from Christopher's solution is that mine utilizes completely > separatated (copied) code, so ruleutils code is still unchanged. This > was a concession to Tom who claimed concerns about pg_dump not being > able to reproduce things correctly if there was *any* error in it. I recall objecting to someone who wanted to remove "unnecessary" parentheses, but I can't see any risk in inserting unnecessary whitespace. regards, tom lane
Tom Lane wrote: > >I recall objecting to someone who wanted to remove "unnecessary" >parentheses, but I can't see any risk in inserting unnecessary >whitespace. > That "someone" was me indeed, and as I mentioned the code is completely separated from the code that pg_dump uses. Thus, there's *no way* that this could break backup integrity. I consider these original functions as pg_dump helper functions, not meant to be human readable. There are *many* parentheses that are not necessary, and the code trying to figure out is quite conservative. All is decided in one single routine, depending on two parameters only, and thus failing to locate several cases when parentheses would be avoidable (not even */ over +- will be noticed!). I've been trying hard to make pgsql as maintainable as mssql, and there's only this point left. Any attempts to contribute this so far just have been answered with "dunno but there might eventually perhaps maybe some problem" without having a look at that function. I feel that I am asked to prove the validity of my code, which is as impossible as it is for software in general, but I haven't seen any case where my solution failed to reproduce correctly. If you know one, tell me. If you know a case where my core routine decides falsely, tell me. What I *really* want is having the original source stored, including comments, version info, ... Currently, it's argued that underlying table and column might change, braking the view/rule. This could be restricted, or source could be dropped (alter table ... cascaded). Is it really only me who tries to put complicated views into pgsql and wants to understand them 10 days later? We do have an enterprise grade RDBMS, don't we? Regards, Andreas
> What I *really* want is having the original source stored, including > comments, version info, ... Currently, it's argued that underlying table > and column might change, braking the view/rule. This could be > restricted, or source could be dropped (alter table ... cascaded). Is it > really only me who tries to put complicated views into pgsql and wants > to understand them 10 days later? We do have an enterprise grade RDBMS, > don't we? You could argue that comments should be converted to an 'information' node within the query structure which contains comments. They would then be dumped back out to the user. But I think you would be dissapointed if you were returned the view that is no longer correct since someone renamed the tables. -- Rod Taylor <rbt@rbt.ca> PGP Key: http://www.rbt.ca/rbtpub.asc
Rod Taylor wrote: >>What I *really* want is having the original source stored, including >>comments, version info, ... Currently, it's argued that underlying table >>and column might change, braking the view/rule. This could be >>restricted, or source could be dropped (alter table ... cascaded). Is it >>really only me who tries to put complicated views into pgsql and wants >>to understand them 10 days later? We do have an enterprise grade RDBMS, >>don't we? >> >> > >You could argue that comments should be converted to an 'information' >node within the query structure which contains comments. They would >then be dumped back out to the user. > >But I think you would be dissapointed if you were returned the view that >is no longer correct since someone renamed the tables. > > > Rod, this arguments are quite academic. On one side, this could be restricted, thats what pg_depends is good for (this already happens for inherited tables). On the other side, how often do you rename columns or tables? On mssql, nobody cares. If you fool around with names, your views will be broken without warning. pgsql could be better easily. I'd really prefer to have full view sources available rather than the gimmick of stable views despite renamed cols/tabs. Regards, Andreas
> this arguments are quite academic. You what! > On one side, this could be > restricted, thats what pg_depends is good for (this already happens for > inherited tables). > On the other side, how often do you rename columns or tables? You what! > On mssql, nobody cares. You what! > If you fool around with names, your views will > be broken without warning. pgsql could be better easily. > I'd really prefer to have full view sources available rather than the > gimmick of stable views despite renamed cols/tabs. Gimmick! You what!!!!!!
Christopher Kings-Lynne wrote: >>this arguments are quite academic. >> >> > >You what! > > > >>On one side, this could be >>restricted, thats what pg_depends is good for (this already happens for >>inherited tables). >>On the other side, how often do you rename columns or tables? >> >> > >You what! > > > >>On mssql, nobody cares. >> >> > >You what! > > > >>If you fool around with names, your views will >>be broken without warning. pgsql could be better easily. >>I'd really prefer to have full view sources available rather than the >>gimmick of stable views despite renamed cols/tabs. >> >> > >Gimmick! You what!!!!!! > > > Christopher, I'm not natively english speaking, and so I don't understand what you want to say with this. Maybe this is some kind of Australian slang? Do you agree or disagree? I'm trying to explain my concerns and proposals, and it would be kind if I'm answered seriously and understandably. Regards, Andreas
Hi Andreas, > I'm not natively english speaking, and so I don't understand what you > want to say with this. Maybe this is some kind of Australian slang? Do > you agree or disagree? I'm trying to explain my concerns and proposals, > and it would be kind if I'm answered seriously and understandably. Sorry if I offended you. 'You what!' is what you say when you cannot believe what someone is saying... Calling 'stable views that work when you rename columns' a gimmick is quite an incredible thing to say... You honestly would rather be able to view accurate source of views that don't work rather than complicted source of views that actually work? Anyway, there's no reason why we can't have both with a bit of effort... Chris
Hi Christopher, >Sorry if I offended you. > I wasn't offended because I wasn't sure what you wanted to say to me. You may call me the biggest fool of all, as long you do it in Sualheli, or Korean... :-) > 'You what!' is what you say when you cannot >believe what someone is saying... Calling 'stable views that work when >you rename columns' a gimmick is quite an incredible thing to say... > >You honestly would rather be able to view accurate source of views that >don't work rather than complicted source of views that actually work? > Yes, that's right. I've been working this way for years, and all MSSQL users do because there's no other way for them. These automatic name change propagation is very limited, concerning all possible changes you can have in a table. - drop column -> will restrict now, or need cascade - rename column -> propagates to plan tree, why not restrict or require cascaded to drop source? - alter column size/type -> not possible, need to create alternate column, drop old (which is restricted...) and rename You see, only few changes that can be done are handled at the moment. In my experience with large and complicated data models, I found that I hardly ever would rename a table or a column. There's nothing like an automatic column name update of applications... After years, it's hard to tell where everything's used, especially if queries are created at runtime. I consider a view more as being a part of an application, rather than part of the data model (unless rules are used), and thus the same problems apply. What I need again and again, is changing the size of a column (rarely the type). For pgsql, I'd have to drop the column, and need to recreate all views. For MSSQL, it won't matter if the column is dropped/recreated or just resized, the view won't notice until it's used again. >Anyway, there's no reason why we can't have both with a bit of effort... > > This certainly would be nice, maybe there could be back-pointers from nodes into the source so identifier names can be identified and modified? Just like debugging-enabled code has references to the source. Another way could be storing the source in a translated form, like SELECT X.($88012.1) FROM $88012 AS X instead of SELECT X.bar FROM public.foo AS X Regards, Andreas
On Wed, Jun 18, 2003 at 12:59:36PM +0200, Andreas Pflug wrote: > What I need again and again, is changing the size of a column (rarely > the type). For pgsql, I'd have to drop the column, and need to recreate > all views. For MSSQL, it won't matter if the column is dropped/recreated > or just resized, the view won't notice until it's used again. If that's what you need you can always change the system catalogs manually. For CHAR(n) and VARCHAR(n) you change pg_attribute.atttypmod to (n+4). For NUMERIC(n,m) it's something like (n<<16) + m + 4 or maybe (m<<16) + n + 4, don't remember right now. Be sure to check that your data is in a safe place before you do this, and double check before you commit the transaction if you do it manually. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Porque Kim no hacia nada, pero, eso si, con extraordinario exito" ("Kim", Kipling)
Alvaro Herrera wrote: >On Wed, Jun 18, 2003 at 12:59:36PM +0200, Andreas Pflug wrote: > > > >>What I need again and again, is changing the size of a column (rarely >>the type). For pgsql, I'd have to drop the column, and need to recreate >>all views. For MSSQL, it won't matter if the column is dropped/recreated >>or just resized, the view won't notice until it's used again. >> >> > >If that's what you need you can always change the system catalogs >manually. For CHAR(n) and VARCHAR(n) you change pg_attribute.atttypmod >to (n+4). For NUMERIC(n,m) it's something like (n<<16) + m + 4 or maybe >(m<<16) + n + 4, don't remember right now. > >Be sure to check that your data is in a safe place before you do this, >and double check before you commit the transaction if you do it >manually. > > > Hm, you're right, 'thou I wouldn't recommend this to the average user, and wonder if this will be possible for all future pgsql versions too. I'm considering adding safe support for this type of column change to pgAdmin3. There might be other cases of legal direct change of system catalog entries, e,g. varchar to text, if they all are only names for internally identical data structures. Can you tell which datatypes may be legally interchanged? Regards, Andreas
> There might be other cases of legal direct change of system catalog > entries, e,g. varchar to text, if they all are only names for internally > identical data structures. Can you tell which datatypes may be legally > interchanged? If pg_cast.castfunc is 0, you should might be able to do a datatype change safely. http://candle.pha.pa.us/main/writings/pgsql/sgml/catalog-pg-cast.html#AEN49071 -- Rod Taylor <rbt@rbt.ca> PGP Key: http://www.rbt.ca/rbtpub.asc
Andreas Pflug <Andreas.Pflug@web.de> writes: > There might be other cases of legal direct change of system catalog > entries, e,g. varchar to text, if they all are only names for internally > identical data structures. Can you tell which datatypes may be legally > interchanged? Right offhand I think text<->varchar and adjustment of length limits in char, varchar, and perhaps numeric would be the only things useful enough to worry about handling. regards, tom lane
> If that's what you need you can always change the system catalogs > manually. For CHAR(n) and VARCHAR(n) you change pg_attribute.atttypmod > to (n+4). For NUMERIC(n,m) it's something like (n<<16) + m + 4 or maybe > (m<<16) + n + 4, don't remember right now. > > Be sure to check that your data is in a safe place before you do this, > and double check before you commit the transaction if you do it > manually. Is there demand for an ALTER COLUMN/SET TYPE that is restricted to binary compatible casts and increasing length changes? Chris
> Hm, you're right, 'thou I wouldn't recommend this to the average user, > and wonder if this will be possible for all future pgsql versions too. > I'm considering adding safe support for this type of column change to > pgAdmin3. > There might be other cases of legal direct change of system catalog > entries, e,g. varchar to text, if they all are only names for internally > identical data structures. Can you tell which datatypes may be legally > interchanged? Yes, you can check if they're binary compatible from the pg_cast table.... Chris
> Right offhand I think text<->varchar and adjustment of length limits in > char, varchar, and perhaps numeric would be the only things useful > enough to worry about handling. I'd love to have adding and removing precision and timezones on timestamp* fields Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: >> There might be other cases of legal direct change of system catalog >> entries, e,g. varchar to text, if they all are only names for internally >> identical data structures. Can you tell which datatypes may be legally >> interchanged? > Yes, you can check if they're binary compatible from the pg_cast table.... But nearly all of the interesting cases require you to understand the type's interpretation of typmod, and you can't learn that from a table. How many cases are there where blindly looking for a binary-compatible cast in pg_cast will really do you much good? AFAICS you'd have to set atttypmod to -1 if you change atttypid without knowing very specifically what you are changing from and to. regards, tom lane
Tom Lane wrote: >>Yes, you can check if they're binary compatible from the pg_cast table.... >> >> > >But nearly all of the interesting cases require you to understand the >type's interpretation of typmod, and you can't learn that from a table. >How many cases are there where blindly looking for a binary-compatible >cast in pg_cast will really do you much good? AFAICS you'd have to set >atttypmod to -1 if you change atttypid without knowing very specifically >what you are changing from and to. > > > AFAICS there's few interpretation about atttypmod necessary because only few datatypes binary convertible (castfunc=0) do use atttypmod at all. Most special case is varchar<->text, one supporting length, the other not; both need atttypmod=-1. bpchar<->varchar both allow typmod in a similar fashion. It's already implemented in pgAdmin3 this way. Regards, Andreas
OK, added to TODO: Modify pg_get_triggerdef() to take a boolean to pretty-print,and use that as part of pg_dump along with psql Andreas, can you work on this? I like the idea of removing extra parens, and merging it into the existing code rather than into contrib makes sense. --------------------------------------------------------------------------- Andreas Pflug wrote: > Tom Lane wrote: > > > > >I recall objecting to someone who wanted to remove "unnecessary" > >parentheses, but I can't see any risk in inserting unnecessary > >whitespace. > > > That "someone" was me indeed, and as I mentioned the code is completely > separated from the code that pg_dump uses. Thus, there's *no way* that > this could break backup integrity. I consider these original functions > as pg_dump helper functions, not meant to be human readable. > > There are *many* parentheses that are not necessary, and the code trying > to figure out is quite conservative. All is decided in one single > routine, depending on two parameters only, and thus failing to locate > several cases when parentheses would be avoidable (not even */ over +- > will be noticed!). > > I've been trying hard to make pgsql as maintainable as mssql, and > there's only this point left. Any attempts to contribute this so far > just have been answered with "dunno but there might eventually perhaps > maybe some problem" without having a look at that function. I feel that > I am asked to prove the validity of my code, which is as impossible as > it is for software in general, but I haven't seen any case where my > solution failed to reproduce correctly. If you know one, tell me. If you > know a case where my core routine decides falsely, tell me. > > What I *really* want is having the original source stored, including > comments, version info, ... Currently, it's argued that underlying table > and column might change, braking the view/rule. This could be > restricted, or source could be dropped (alter table ... cascaded). Is it > really only me who tries to put complicated views into pgsql and wants > to understand them 10 days later? We do have an enterprise grade RDBMS, > don't we? > > Regards, > Andreas > > > ---------------------------(end of broadcast)--------------------------- > TIP 9: the planner will ignore your desire to choose an index scan if your > joining column's datatypes do not match > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Add to TODO: o Allow ALTER TABLE to modify column lengths and change to binary compatible types --------------------------------------------------------------------------- Tom Lane wrote: > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > >> There might be other cases of legal direct change of system catalog > >> entries, e,g. varchar to text, if they all are only names for internally > >> identical data structures. Can you tell which datatypes may be legally > >> interchanged? > > > Yes, you can check if they're binary compatible from the pg_cast table.... > > But nearly all of the interesting cases require you to understand the > type's interpretation of typmod, and you can't learn that from a table. > How many cases are there where blindly looking for a binary-compatible > cast in pg_cast will really do you much good? AFAICS you'd have to set > atttypmod to -1 if you change atttypid without knowing very specifically > what you are changing from and to. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote: >OK, added to TODO: > > Modify pg_get_triggerdef() to take a boolean to pretty-print, > and use that as part of pg_dump along with psql > >Andreas, can you work on this? I like the idea of removing extra >parens, and merging it into the existing code rather than into contrib >makes sense. > Yes, I can. At the moment, I have a runnable contrib module, which replaces all pg_get_xxxdef by pg_get_xxxdef2 functions. It's no problem to apply that code back to the original ruleutils.c when the isSimpleNode algorithm is reviewed independently and proved being correct. For safety reasons, I can make this dependent on a bool pretty-print parameter. I also could implement line break and indentation formatting. I implemented a keyword-based algorithm in pgAdmin3, and having the original tree the job is obviously easier. Do we need any flexibility about indent char (tab or space) and indentation size (2 chars)? The pgAdmin3 algorithm uses 4 spaces, and tries to align keywords so they line up nicely, and I'd prefer doing it this way again. SELECT foo FROM bar b JOIN chair c USING (thekeycol) WHERE ... Regards, Andreas /**************************************** check if given node is simple.* false : not simple* true : simple in the contextof parent node's type***************************************/ static bool isSimpleNode(Node *node, NodeTag parentNodeType) { if (!node)return true; switch (nodeTag(node)) {// single words: always simplecase T_Var:case T_Const:case T_Param: // function-like: name(..) or name[..]case T_ArrayRef:case T_FuncExpr:case T_ArrayExpr:case T_CoalesceExpr:case T_NullIfExpr:caseT_Aggref: // CASE keywords act as parenthesescase T_CaseExpr: return true; // appears simple since . has top precedence, unless parent is T_FieldSelect itself!case T_FieldSelect: return(parentNodeType == T_FieldSelect ? false : true); // maybe simple, check argscase T_CoerceToDomain: return isSimpleNode((Node*) ((CoerceToDomain*)node)->arg, T_CoerceToDomain);caseT_RelabelType: return isSimpleNode((Node*) ((RelabelType*)node)->arg, T_RelabelType); // depends on parent node type; needs further checkingcase T_SubLink:case T_NullTest:case T_BooleanTest:case T_OpExpr:caseT_DistinctExpr: if (parentNodeType == T_BoolExpr) return true; // else check the same as for T_BoolExpr;no break here!case T_BoolExpr: switch (parentNodeType) { case T_ArrayRef: case T_FuncExpr: caseT_ArrayExpr: case T_CoalesceExpr: case T_NullIfExpr: case T_Aggref: case T_CaseExpr: return true; default: break; } return false; // these are not completely implemented; so far, they're simplecase T_SubPlan:case T_CoerceToDomainValue: returntrue; default: break; } // those we don't know: in dubio complexo return false; }
Great. I recommend using spaces rather than tabs for indenting in psql and pg_dump. --------------------------------------------------------------------------- Andreas Pflug wrote: > > > Bruce Momjian wrote: > > >OK, added to TODO: > > > > Modify pg_get_triggerdef() to take a boolean to pretty-print, > > and use that as part of pg_dump along with psql > > > >Andreas, can you work on this? I like the idea of removing extra > >parens, and merging it into the existing code rather than into contrib > >makes sense. > > > > Yes, I can. At the moment, I have a runnable contrib module, which > replaces all pg_get_xxxdef by pg_get_xxxdef2 functions. It's no problem > to apply that code back to the original ruleutils.c when the > isSimpleNode algorithm is reviewed independently and proved being correct. > > For safety reasons, I can make this dependent on a bool pretty-print > parameter. > > I also could implement line break and indentation formatting. I > implemented a keyword-based algorithm in pgAdmin3, and having the > original tree the job is obviously easier. Do we need any flexibility > about indent char (tab or space) and indentation size (2 chars)? The > pgAdmin3 algorithm uses 4 spaces, and tries to align keywords so they > line up nicely, and I'd prefer doing it this way again. > > SELECT foo > FROM bar b > JOIN chair c USING (thekeycol) > WHERE ... > > > > Regards, > > Andreas > > /*************************************** > * check if given node is simple. > * false : not simple > * true : simple in the context of parent node's type > ***************************************/ > > static bool isSimpleNode(Node *node, NodeTag parentNodeType) > { > if (!node) > return true; > > switch (nodeTag(node)) > { > // single words: always simple > case T_Var: > case T_Const: > case T_Param: > > // function-like: name(..) or name[..] > case T_ArrayRef: > case T_FuncExpr: > case T_ArrayExpr: > case T_CoalesceExpr: > case T_NullIfExpr: > case T_Aggref: > > // CASE keywords act as parentheses > case T_CaseExpr: > return true; > > // appears simple since . has top precedence, unless parent is T_FieldSelect itself! > case T_FieldSelect: > return (parentNodeType == T_FieldSelect ? false : true); > > > // maybe simple, check args > case T_CoerceToDomain: > return isSimpleNode((Node*) ((CoerceToDomain*)node)->arg, T_CoerceToDomain); > case T_RelabelType: > return isSimpleNode((Node*) ((RelabelType*)node)->arg, T_RelabelType); > > > // depends on parent node type; needs further checking > case T_SubLink: > case T_NullTest: > case T_BooleanTest: > case T_OpExpr: > case T_DistinctExpr: > if (parentNodeType == T_BoolExpr) > return true; > // else check the same as for T_BoolExpr; no break here! > case T_BoolExpr: > switch (parentNodeType) > { > case T_ArrayRef: > case T_FuncExpr: > case T_ArrayExpr: > case T_CoalesceExpr: > case T_NullIfExpr: > case T_Aggref: > case T_CaseExpr: > return true; > default: > break; > } > return false; > > // these are not completely implemented; so far, they're simple > case T_SubPlan: > case T_CoerceToDomainValue: > return true; > > default: > break; > } > // those we don't know: in dubio complexo > return false; > } > > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Andreas Pflug wrote: > I also could implement line break and indentation formatting. I > implemented a keyword-based algorithm in pgAdmin3, and having the > original tree the job is obviously easier. Do we need any flexibility > about indent char (tab or space) and indentation size (2 chars)? The > pgAdmin3 algorithm uses 4 spaces, and tries to align keywords so they > line up nicely, and I'd prefer doing it this way again. > > SELECT foo > FROM bar b > JOIN chair c USING (thekeycol) > WHERE ... Oh, one more thing --- right justify isn't as accepted as left-justify --- sorry. > SELECT foo > FROM bar b > JOIN chair c USING (thekeycol) > WHERE ... -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
> Oh, one more thing --- right justify isn't as accepted as left-justify But it looks so much better... -- Rod Taylor <rbt@rbt.ca> PGP Key: http://www.rbt.ca/rbtpub.asc
Rod Taylor wrote: >>Oh, one more thing --- right justify isn't as accepted as left-justify >> >> > >But it looks so much better... > > Yessss! Consider this: SELECT foo FROM bar b LEFT JOIN chair c USING (thekeycol)WHERE ... :-) versus SELECT foo FROM bar b LEFT JOIN chair c USING (thekeycol) WHERE ... The keywords are separated :-( SELECT foo FROM bar b LEFT JOIN chair c USING (thekeycol) WHERE ... No more lineup :-( Admittedly, when you type it yourself, it's a bit annoying, because you can't use just tabs. But if it's generated, it won't do any harm. Why not giving PostgreSQL this extra portion of elegance... Regards, Andreas
I don't think an option to do such justification would be good unless we can do it consistenly in the code, and there is enough demand. --------------------------------------------------------------------------- Andreas Pflug wrote: > Rod Taylor wrote: > > >>Oh, one more thing --- right justify isn't as accepted as left-justify > >> > >> > > > >But it looks so much better... > > > > > Yessss! > Consider this: > > SELECT foo > FROM bar b > LEFT JOIN chair c USING (thekeycol) > WHERE ... > :-) > > versus > > SELECT foo > FROM bar b > LEFT JOIN chair c USING (thekeycol) > WHERE ... > The keywords are separated :-( > > SELECT foo > FROM bar b > LEFT JOIN chair c USING (thekeycol) > WHERE ... > No more lineup :-( > > > > Admittedly, when you type it yourself, it's a bit annoying, because you > can't use just tabs. But if it's generated, it won't do any harm. > Why not giving PostgreSQL this extra portion of elegance... > > Regards, > Andreas > > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
> SELECT foo > FROM bar b > LEFT JOIN chair c USING (thekeycol) > WHERE ... > :-) Sub-selects are much nicer: SELECT foo , bar , (SELECT anotherfoo FROM tab2 WHERE tab2.col = tab1.col) FROM tab JOIN yet_another_table AS yat ON (yat.c = tab.c) LEFT JOIN tab1 USING (anothercol) WHERE stuff IS TRUE AND ( optional IS NULL OR optional > 5) HAVING count(*)> (SELECT total FROM total_table)ORDER BY fooGROUP BY foo , bar , 3; -- Rod Taylor <rbt@rbt.ca> PGP Key: http://www.rbt.ca/rbtpub.asc
Bruce Momjian wrote: >I don't think an option to do such justification would be good unless we >can do it consistenly in the code, and there is enough demand. > It's no big additional effort to do this, and going back and forth is not a big problem. I wouldn't invent an option for that, just do it. Let's see what's happening. At least, there seems agreement on not using tabs but spaces. Still, I'd like to have somebody having a look at my algorithm. It's the key component, which should be proven right theoretically. Regards, Andreas
Bruce Momjian wrote: >OK, added to TODO: > > Modify pg_get_triggerdef() to take a boolean to pretty-print, > and use that as part of pg_dump along with psql > >Andreas, can you work on this? I like the idea of removing extra >parens, and merging it into the existing code rather than into contrib >makes sense. > > Just an announcement: I'll be sending a patch for ruleutils.c and pg_proc.h tomorrow, after I performed some further testing. pg_get_ruledef, pg_get_viewdef, pg_get_viewdef_name, pg_get_indexdef, pg_get_constraintdef and pg_get_expr get an additional parameter int4 each which controls pretty-print (0: none, 1: parentheses, 1: indentation, 3: both). I had to make several conditionals for the old parenthesing code, but I believe the functions still generate as usual if pretty-print is disabled. At the moment, I assigned oids 2504-2509 (last used was 2503 when I updated from cvs) to the additional functions, is that ok? Regards, Andreas