Allow COMMENT ON to accept an expression rather than just a string

On Fri, Apr 10, 2009 at 11:47 PM, Abhijit Menon-Sen <ams@oryx.com> wrote:

Hi.

There's a TODO item about making COMMENT ON accept an expression.

really? what's the use case for that?

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

Jaime Casanova wrote:

On Fri, Apr 10, 2009 at 11:47 PM, Abhijit Menon-Sen <ams@oryx.com> wrote:

Hi.

There's a TODO item about making COMMENT ON accept an expression.

really? what's the use case for that?

I assume it is for creating comments using things like || or maybe
CURRENT_TIMESTAMP:

test=> CREATE TABLE test (x INT);
CREATE TABLE
test=> COMMENT ON TABLE test IS 'asdf';
COMMENT
test=> COMMENT ON TABLE test IS 'asdf' || 'bb';
ERROR: syntax error at or near "||"
LINE 1: COMMENT ON TABLE test IS 'asdf' || 'bb';

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Abhijit Menon-Sen <ams@oryx.com> writes:

There's a TODO item about making COMMENT ON accept an expression. The
grammar change is simple (SConst|NULL_P->a_expr), but as far as I can
see, there are no similar utility commands that take expressions, and
I'm not very familiar with the planner and executor, so I could use
some advice about how to evaluate the expression.

There aren't *any* utility commands that take expressions, and I would
say that that TODO item is seriously mis-scoped. What is the use of
making COMMENT in particular do this? Fixing all of them might be
interesting.

regards, tom lane

Tom Lane wrote:

Abhijit Menon-Sen <ams@oryx.com> writes:

There's a TODO item about making COMMENT ON accept an expression. The
grammar change is simple (SConst|NULL_P->a_expr), but as far as I can
see, there are no similar utility commands that take expressions, and
I'm not very familiar with the planner and executor, so I could use
some advice about how to evaluate the expression.

There aren't *any* utility commands that take expressions, and I would
say that that TODO item is seriously mis-scoped. What is the use of
making COMMENT in particular do this? Fixing all of them might be
interesting.

I remember adding the TODO after a request from the user, but I have not
seen further requests. I have remove the item; let's see if we get any
further requests for it.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

2009/4/11 Bruce Momjian <bruce@momjian.us>:

Tom Lane wrote:

Abhijit Menon-Sen <ams@oryx.com> writes:

There's a TODO item about making COMMENT ON accept an expression. The
grammar change is simple (SConst|NULL_P->a_expr), but as far as I can
see, there are no similar utility commands that take expressions, and
I'm not very familiar with the planner and executor, so I could use
some advice about how to evaluate the expression.

There aren't *any* utility commands that take expressions, and I would
say that that TODO item is seriously mis-scoped.  What is the use of
making COMMENT in particular do this?  Fixing all of them might be
interesting.

I remember adding the TODO after a request from the user, but I have not
seen further requests.  I have remove the item;  let's see if we get any
further requests for it.

I thing so this TODO point is little bit step in bad direction. The
people use comment for storing some info about database object, that
should be available from dictionary. Typical use case is info about
last modification or create time. Now I am working in bigger
organisation and I see, so this informations are very important. So
well solution is add some table to dictionary, that store "create
timestamp, modification timestamp (ALTER), version (number of
changes)" to every database object where this has sense. Comment
should be stable and only text.

regards
Pavel Stehule

Pavel Stehule <pavel.stehule@gmail.com> writes:

2009/4/11 Bruce Momjian <bruce@momjian.us>:

I remember adding the TODO after a request from the user, but I have not
seen further requests. I have remove the item; let's see if we get any
further requests for it.

I thing so this TODO point is little bit step in bad direction.

I agree with Pavel that the originally suggested use-case seems like
a poor man's substitute for a missing database facility. But Abhijit's
question about parameters reminds me that there is a use-case from the
point of view of client-side libraries. You might wish to do something
like (pseudo-code here)

execute('COMMENT ON foo IS $1', some_string);

and let the out-of-line-parameter mechanism take care of quoting and
escaping your string. This doesn't work today, and I remember having
seen complaints about that on the JDBC list. So there's a use-case at
least for allowing parameter symbols in place of string literals, if not
fully general expressions. But again, I think we'd want such a thing
across all utility statements that can take string literals, not only
COMMENT.

regards, tom lane

like (pseudo-code here)

       execute('COMMENT ON foo IS $1', some_string);

and let the out-of-line-parameter mechanism take care of quoting and
escaping your string.  This doesn't work today, and I remember having
seen complaints about that on the JDBC list.  So there's a use-case at
least for allowing parameter symbols in place of string literals, if not
fully general expressions.  But again, I think we'd want such a thing
across all utility statements that can take string literals, not only
COMMENT.

I afraid, this should have some not wanted impacts. When we allows
parametrisation in utility statements (and it should be nice), I
expect, so there will be people, that will write code like

CREATE OR REPLACE FUNCTION some_proc(tabname varchar)
RETURNS void AS $$
BEGIN
CREATE TABLE tabname(a integer); -- will work
INSERT INTO tabname VALUES(10); -- will not work
END;
$$ LANG ....

Now people know, so this cannot do it.

But the proc
CREATE OR REPLACE FUNCTION some_proc(tabname varchar)
RETURNS void AS $$
BEGIN
EXECUTE 'CREATE TABLE $1(a integer)' USING tabname;

is more readable than EXECUTE 'CREATE TABLE ' || tabname || '(....

so this isn't too simple

regards
Pavel Stehule

On 4/10/09 10:13 PM, Jaime Casanova wrote:

On Fri, Apr 10, 2009 at 11:47 PM, Abhijit Menon-Sen<ams@oryx.com> wrote:

Hi.

There's a TODO item about making COMMENT ON accept an expression.

really? what's the use case for that?

Easy: if you want to add comments to database objects generated in
stored procedures or simple SQL scripts. I can see wanting it. Not a
critical feature, but if someone wants to write the code? Sure.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Pavel Stehule <pavel.stehule@gmail.com> writes:

But the proc
CREATE OR REPLACE FUNCTION some_proc(tabname varchar)
RETURNS void AS $$
BEGIN
EXECUTE 'CREATE TABLE $1(a integer)' USING tabname;

is more readable than EXECUTE 'CREATE TABLE ' || tabname || '(....

I was intentionally excluding the idea of substituting parameters for
names as opposed to constants. For one thing it's fundamentally
ambiguous --- given

string_var := 'foo';

EXECUTE 'SELECT $1 FROM bar' USING string_var;

is that supposed to mean SELECT 'foo' FROM bar or SELECT foo FROM bar?

The other problem is that if you allow name substitution it becomes
entirely impossible to do any planning or even validity checking before
the parameter values are available. So while string assembly is kind
of a pain in the rear when you really do need a dynamic name reference,
I think we should keep it firmly separate from parameter substitution.

regards, tom lane

2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

But the proc
CREATE OR REPLACE FUNCTION some_proc(tabname varchar)
RETURNS void AS $$
BEGIN
  EXECUTE 'CREATE TABLE $1(a integer)' USING tabname;

is more readable than EXECUTE 'CREATE TABLE ' || tabname || '(....

I was intentionally excluding the idea of substituting parameters for
names as opposed to constants.  For one thing it's fundamentally
ambiguous --- given

       string_var := 'foo';

       EXECUTE 'SELECT $1 FROM bar' USING string_var;

is that supposed to mean SELECT 'foo' FROM bar or SELECT foo FROM bar?

The other problem is that if you allow name substitution it becomes
entirely impossible to do any planning or even validity checking before
the parameter values are available.  So while string assembly is kind
of a pain in the rear when you really do need a dynamic name reference,
I think we should keep it firmly separate from parameter substitution.

+1

At 2009-04-11 14:08:21 -0400, tgl@sss.pgh.pa.us wrote:

So there's a use-case at least for allowing parameter symbols in place
of string literals, if not fully general expressions. But again, I
think we'd want such a thing across all utility statements that can
take string literals, not only COMMENT.

Sounds good to me. I'll implement it if someone will help me to figure
out the details of what exactly needs to be done. (As I said, I'm not
familiar with this part of the code; and there is no obvious example
to follow here.)

-- ams