pg_get_triggerdef in pg_dump

"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, Pennsylvania 19073

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, Pennsylvania 19073

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;
}

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, Pennsylvania 19073

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, Pennsylvania 19073

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, Pennsylvania 19073

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 foo
GROUP 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