Re: doc: Clarify that empty COMMENT string removes the comment

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: doc: Clarify that empty COMMENT string removes the comment
Date: 2026-02-27 17:00:17
Message-ID: CAHGQGwEy5ggRG4OcR5rCc6KOTojLnQ2Z8Ow5_DhyRLwHNj0yFQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Feb 28, 2026 at 1:45 AM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>
> On Thu, Feb 26, 2026 at 8:44 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>
>>
>> > On Feb 26, 2026, at 23:56, David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>> >
>> > On Thu, Feb 26, 2026 at 5:02 AM Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)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 reads as 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 where we use this style of communication in the description (i.e., telling the user directly what they should be doing, as opposed 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 page conform to the standard established elsewhere. I'm not seeing any particular virtue to leaving this page unique in that regard. 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2026-02-27 17:06:13 Re: SQL Property Graph Queries (SQL/PGQ)
Previous Message Jacob Champion 2026-02-27 16:55:41 Re: [oauth] Bug: when is shutdown_cb called?