Re: doc: Clarify that empty COMMENT string removes the comment - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: doc: Clarify that empty COMMENT string removes the comment
Date
Msg-id CAHGQGwEy5ggRG4OcR5rCc6KOTojLnQ2Z8Ow5_DhyRLwHNj0yFQ@mail.gmail.com
Whole thread
In response to Re: doc: Clarify that empty COMMENT string removes the comment  ("David G. Johnston" <david.g.johnston@gmail.com>)
List pgsql-hackers
On Sat, Feb 28, 2026 at 1:45 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> On Thu, Feb 26, 2026 at 8:44 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>> > On Feb 26, 2026, at 23:56, David G. Johnston <david.g.johnston@gmail.com> wrote:
>> >
>> > On Thu, Feb 26, 2026 at 5:02 AM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>> > At the beginning of this synopsis there's following sentence. I think
>> > we need to update it too.
>> > To remove a
>> >    comment, write <literal>NULL</literal> in place of the text string.
>> >
>> >
>> > I concur this should be a bit less surgical than what is presently proposed.  I would do the following:
>> >
>> > diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
>> > index 8d81244910b..45d39e1fc45 100644
>> > --- a/doc/src/sgml/ref/comment.sgml
>> > +++ b/doc/src/sgml/ref/comment.sgml
>> > @@ -66,7 +66,7 @@ COMMENT ON
>> >    TRIGGER <replaceable class="parameter">trigger_name</replaceable> ON <replaceable
class="parameter">table_name</replaceable>| 
>> >    TYPE <replaceable class="parameter">object_name</replaceable> |
>> >    VIEW <replaceable class="parameter">object_name</replaceable>
>> > -} IS { <replaceable class="parameter">string_literal</replaceable> | NULL }
>> > +} IS { <replaceable class="parameter">string_literal</replaceable> | '' | NULL }
>>
>> I don’t think this is necessary, as I guess we don’t want to encourage the usage of empty string, NULL is clearer.
>
>
> I accept that.  The proscriptive form of the Description paragraph below is even worse in this regard though.  It
readsas permission to write an empty string to remove a comment.  The descriptive form is more neutral. 
>
>>
>> >    <para>
>> > -   Only one comment string is stored for each object, so to modify a comment,
>> > -   issue a new <command>COMMENT</command> command for the same object.  To remove a
>> > -   comment, write <literal>NULL</literal> in place of the text string.
>> > +   Each object may have one comment, which will be nonempty.  Setting the
>> > +   comment to an empty string or <literal>NULL</literal> drops the comment.
>> >     Comments are automatically dropped when their object is dropped.
>> >    </para>
>>
>> I didn’t completely take your wording, but I added “empty string” in this paragraph.
>>
>
>     <para>
>     Only one comment string is stored for each object, so to modify a comment,
>     issue a new <command>COMMENT</command> command for the same object.  To remove a
> -   comment, write <literal>NULL</literal> in place of the text string.
> +   comment, use <literal>NULL</literal> or an empty string (<literal>''</literal>).
>     Comments are automatically dropped when their object is dropped.
>    </para>
>
> I can live with this but am not a fan.  I'd like you to point out other examples in the documentation references
wherewe use this style of communication in the description (i.e., telling the user directly what they should be doing,
asopposed to explaining how certain commands and values are interpreted).  I've looked at Grant, Reindex, Vacuum,
Update,and Execute: they are all descriptive, not proscriptive, in tone.  We should take this opportunity to make this
pageconform to the standard established elsewhere.  I'm not seeing any particular virtue to leaving this page unique in
thatregard.  And mention above a reason not to. 


Besides updating the documentation, isn't it better to also add a regression
test to check that COMMENT with an empty string behaves as expected?

Regards,

--
Fujii Masao



pgsql-hackers by date:

Previous
From: Jacob Champion
Date:
Subject: Re: [oauth] Bug: when is shutdown_cb called?
Next
From: Ashutosh Bapat
Date:
Subject: Re: SQL Property Graph Queries (SQL/PGQ)