Обсуждение: [HACKERS] RFC: ALTER SYSTEM [...] COMMENT

-hackers,

We have had ALTER SYSTEM for some time now. It is awesome to be able to 
make changes that can be system wide without ever having to hit a shell 
but it does lack a feature that seems like an oversight, the ability to 
comment.

Problem we are trying to solve:

Having documentation for changes to GUC parameters that are modified via 
ALTER SYSTEM.

Why?

Because documentation is good and required for a proper production system.

How?

I propose:

Add a column to pg_settings comment(text)
Change the grammar to allow:

ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' | 
DEFAULT } COMMENT 'comment'

Example:

ALTER SYSTEM SET maintenance_work_mem TO '1GB' COMMENT IS 'Increased to 
allow autovacuum to be more efficient';

Potential issues:

Does not use existing comment functionality. Alternate solution which 
would decrease functionality is:

COMMENT ON SETTING setting IS 'comment';

Looking forward, we may want to do the following:

1. Make it so any object can have a comment with creation, e.g;

CREATE TABLE table () COMMENT IS '';

2. Make it so comments are appended not replaced.

Thanks in advance,

JD



-- 
Command Prompt, Inc.                  http://the.postgres.company/                        +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.

JD,

* Joshua D. Drake (jd@commandprompt.com) wrote:
> Does not use existing comment functionality. Alternate solution
> which would decrease functionality is:
>
> COMMENT ON SETTING setting IS 'comment';

That seems like a pretty reasonable idea, at least from where I sit.

> Looking forward, we may want to do the following:
>
> 1. Make it so any object can have a comment with creation, e.g;
>
> CREATE TABLE table () COMMENT IS '';

I'm pretty sure this has been discussed previously.

> 2. Make it so comments are appended not replaced.

Having COMMENT ON accept a general query whose result is then cast to
text and stored as the comment would allow this to be done, eg:

COMMENT ON table IS (pg_get_comment('table') || ' new text');

We could also have new syntax along these lines, for this specific case:

COMMENT ON table ADD ' new text';

Though we have this pretty powerful language, seems a bit of a shame to
invent something new for working with comments.

Thanks!

Stephen

On 26 April 2017 at 18:03, Joshua D. Drake <jd@commandprompt.com> wrote:
> -hackers,
>
> We have had ALTER SYSTEM for some time now. It is awesome to be able to make
> changes that can be system wide without ever having to hit a shell but it
> does lack a feature that seems like an oversight, the ability to comment.
>
> Problem we are trying to solve:
>
> Having documentation for changes to GUC parameters that are modified via
> ALTER SYSTEM.
>
> Why?
>
> Because documentation is good and required for a proper production system.
>
> How?
>
> I propose:
>
> Add a column to pg_settings comment(text)
> Change the grammar to allow:
>
> ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' |
> DEFAULT } COMMENT 'comment'
>
> Example:
>
> ALTER SYSTEM SET maintenance_work_mem TO '1GB' COMMENT IS 'Increased to
> allow autovacuum to be more efficient';
>
> Potential issues:
>
> Does not use existing comment functionality. Alternate solution which would
> decrease functionality is:
>
> COMMENT ON SETTING setting IS 'comment';
>
> Looking forward, we may want to do the following:
>
> 1. Make it so any object can have a comment with creation, e.g;
>
> CREATE TABLE table () COMMENT IS '';
>
> 2. Make it so comments are appended not replaced.

Yeah, this is something I've also suggested previously:

https://www.postgresql.org/message-id/CAA-aLv50MZdjdVk_=Tep6na94dNmi1Y9XkCp3er7FQqvX=DagQ@mail.gmail.com

Thom

On 04/26/2017 10:14 AM, Stephen Frost wrote:
> JD,

> Having COMMENT ON accept a general query whose result is then cast to
> text and stored as the comment would allow this to be done, eg:
>
> COMMENT ON table IS (pg_get_comment('table') || ' new text');

Dig it, although we probably want the equivalent of:

COMMENT ON table IS (pg_get_comment('table') || '\n\n' || ' new text');

Or something like that.

>
> We could also have new syntax along these lines, for this specific case:
>
> COMMENT ON table ADD ' new text';
>
> Though we have this pretty powerful language, seems a bit of a shame to
> invent something new for working with comments.

Agreed and I think that using existing COMMENT ON capability is likely 
to get this pushed farther down the road.

I wouldn't fight hard to change it but really if we think about it, what 
makes more sense from usability perspective?

CREATE TABLE foo() COMMENT IS

or

CREATE TABLE foo;
COMMENT ON TABLE foo IS

Thanks,

JD




-- 
Command Prompt, Inc.                  http://the.postgres.company/                        +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.

"Joshua D. Drake" <jd@commandprompt.com> writes:
> I wouldn't fight hard to change it but really if we think about it, what 
> makes more sense from usability perspective?

> CREATE TABLE foo() COMMENT IS

I think it's likely to be impossible to shoehorn such a thing into every
type of CREATE command without making COMMENT a fully reserved word,
which is going to be a very hard sell.

> 2. Make it so comments are appended not replaced.

Backwards compatibility fail ... not to mention that you haven't offered
an argument as to why everyone would think this is an improvement.
        regards, tom lane

Stephen Frost <sfrost@snowman.net> writes:
> Having COMMENT ON accept a general query whose result is then cast to
> text and stored as the comment would allow this to be done, eg:

> COMMENT ON table IS (pg_get_comment('table') || ' new text');

Putting general subexpressions into utility statements has some
implementation issues.  Plus, it's not really all that powerful.
It'd be better to invent inverse pg_get_comment and pg_set_comment
functions, then you could do bulk-update things like
select pg_set_comment('table', pg_get_comment('table') || ' more')from pg_class where ...

The main thing lacking to make that into a real proposal would be
a way of naming the object the comment is for; but I think Alvaro's
already exposed something corresponding to ObjectAddress to SQL, no?

> We could also have new syntax along these lines, for this specific case:
> COMMENT ON table ADD ' new text';

Wouldn't have a big problem with that, as it'd address a common case
for not much work.
        regards, tom lane

On 04/26/2017 10:31 AM, Tom Lane wrote:
> "Joshua D. Drake" <jd@commandprompt.com> writes:
>> I wouldn't fight hard to change it but really if we think about it, what
>> makes more sense from usability perspective?
>
>> CREATE TABLE foo() COMMENT IS
>
> I think it's likely to be impossible to shoehorn such a thing into every
> type of CREATE command without making COMMENT a fully reserved word,
> which is going to be a very hard sell.

Well if it is a complete uphill battle, this is certainly not the 
feature that I am going to dig my heels in about.

>
>> 2. Make it so comments are appended not replaced.
>
> Backwards compatibility fail ... not to mention that you haven't offered
> an argument as to why everyone would think this is an improvement.

"Everyone" is a bit of a stretch for every single feature we have.

I would think that most people that work with production systems would 
like to know the history of any object modification.

Thanks,

jD



>
>             regards, tom lane
>


-- 
Command Prompt, Inc.                  http://the.postgres.company/                        +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > Having COMMENT ON accept a general query whose result is then cast to
> > text and stored as the comment would allow this to be done, eg:
>
> > COMMENT ON table IS (pg_get_comment('table') || ' new text');
>
> Putting general subexpressions into utility statements has some
> implementation issues.  Plus, it's not really all that powerful.
> It'd be better to invent inverse pg_get_comment and pg_set_comment
> functions, then you could do bulk-update things like
>
>     select pg_set_comment('table', pg_get_comment('table') || ' more')
>     from pg_class where ...
>
> The main thing lacking to make that into a real proposal would be
> a way of naming the object the comment is for; but I think Alvaro's
> already exposed something corresponding to ObjectAddress to SQL, no?

Yes and yes.  I like the 'pg_set_comment' idea, and I think we do have
something or other now through ObjectAddress and friends.

> > We could also have new syntax along these lines, for this specific case:
> > COMMENT ON table ADD ' new text';
>
> Wouldn't have a big problem with that, as it'd address a common case
> for not much work.

We could have this also, I suppose.

Of course, once we start thinking about what kind of comments people
might be interested in, as alluded to elsewhere, it's entirely likely
they'll want to get things like timestamps included and other
information that, ultimately, would be better if it was structured and
not just thrown together into a free-form text field.  Not sure how much
we want to try and go down that road, or if we are happy to tell people
to just use:

select pg_set_comment('table','q',pg_get_comment('table','q') || 'E\n' || now() || ': new comment'));

Not sure why anyone would grouse about that though ...

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> It'd be better to invent inverse pg_get_comment and pg_set_comment
>> functions, then you could do bulk-update things like
>> select pg_set_comment('table', pg_get_comment('table') || ' more')
>> from pg_class where ...

> Of course, once we start thinking about what kind of comments people
> might be interested in, as alluded to elsewhere, it's entirely likely
> they'll want to get things like timestamps included and other
> information that, ultimately, would be better if it was structured and
> not just thrown together into a free-form text field.

It's not hard to imagine people deciding that all their comments will
be (the text form of) JSON objects containing certain fields.  But
I don't think it's appropriate for us to mandate that sort of thing.
Given get/set comment functions as above, users could write wrapper
functions implementing their local conventions.
        regards, tom lane

On Wed, Apr 26, 2017 at 10:33 PM, Joshua D. Drake <jd@commandprompt.com> wrote:
> I propose:
>
> Add a column to pg_settings comment(text)
> Change the grammar to allow:
>
> ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' |
> DEFAULT } COMMENT 'comment'
>
> Example:
>
> ALTER SYSTEM SET maintenance_work_mem TO '1GB' COMMENT IS 'Increased to
> allow autovacuum to be more efficient';
>
> Potential issues:
>
> Does not use existing comment functionality. Alternate solution which would
> decrease functionality is:
>
> COMMENT ON SETTING setting IS 'comment';
>

I think syntax wise, it makes sense to have both the forms in some way
as proposed by you.  I think providing COMMENT along with ALTER SYSTEM
has the advantage that user can specify it when it specifies the
setting and by providing a separate command facilitates to use it even
when we just want to add/change the comments for a particular setting.

I think Robert [1] and I [2] have made some comments when this topic
was previously discussed which are worth looking if you or someone is
planning to write a patch for this feature.


[1] -
https://www.postgresql.org/message-id/CA%2BTgmoZBDLhDexHyTJ%3DH0xZt7-6M%3Dh%2Bv2Xi0Myj7Q37QGZQb4g%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAA4eK1%2B%3DovYQqYGHcX2OBU3mk%2BhSHjFDpvmrHCos1Vgj8_b6vg%40mail.gmail.com

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com