On Sat, Feb 28, 2026 at 1:45 AM David G. Johnston <[email protected]> wrote: > > On Thu, Feb 26, 2026 at 8:44 PM Chao Li <[email protected]> wrote: >> >> >> >> > On Feb 26, 2026, at 23:56, David G. Johnston <[email protected]> >> > wrote: >> > >> > On Thu, Feb 26, 2026 at 5:02 AM Ashutosh Bapat >> > <[email protected]> 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
