Обсуждение: Allow COMMENT ON to accept an expression rather than just a string
Hi. 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. Also: what expressions should the code accept? I want to be able to use a parameter. Does that require extra work? What about subqueries, would they be useful in this context? (Or would it be better to just have functions that can set object and column descriptions instead?) Thanks. -- ams
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 TABLEtest=> COMMENT ON TABLE test IS 'asdf';COMMENTtest=> 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 > > -- > Bruce Momjian <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
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
>
> regards, tom lane
>
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 > regards, tom lane >
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