Re: doc: Clarify that empty COMMENT string removes the comment
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-02-27T03:19:44Z
Lists: pgsql-hackers
On Thu, Feb 26, 2026 at 9:27 PM 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:
+1.
>
> 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 }
>
There are other ways to specify an empty string e.g $$$$. I don't
think we want to list all of them here.
> <phrase>where <replaceable>aggregate_signature</replaceable> is:</phrase>
>
> @@ -80,13 +80,12 @@ COMMENT ON
> <title>Description</title>
>
> <para>
> - <command>COMMENT</command> stores a comment about a database object.
> + <command>COMMENT</command> stores, replaces, or removes the comment for a database object.
> </para>
>
> <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.
I like this.
> Comments are automatically dropped when their object is dropped.
> </para>
>
> @@ -266,7 +265,8 @@ COMMENT ON
> <term><replaceable class="parameter">string_literal</replaceable></term>
> <listitem>
> <para>
> - The new comment contents, written as a string literal.
> + The new comment contents, written as a string literal. An empty string
> + drops the comment.
> </para>
> </listitem>
> </varlistentry>
>
> I don't see a strong need to mention NULL in the description for string_literal; just say what specifying an empty string does directly.
+1.
>
> The command Description should cover this if we are going to mention it in the Parameters section. Otherwise a lone comment in the Notes section would be better IMO. A lone mention in Parameters for string_literal is unlikely to be noticed - people aren't questioning how string_literal behaves here and then go look at its description.
+1.
>
> On the last point, I choose to show the literal '' value as an explicit syntax option to point out immediately that writing it as the string_literal invokes special consideration.
See above.
>
> I do think the rewording of the Description paragraphs nicely adds the new information about the empty string while making the whole thing more concise with no loss of content. "the comment" is more precise than "a comment", as are the effects of executing the command.
+1.
--
Best Wishes,
Ashutosh Bapat
Commits
-
doc: Clarify that empty COMMENT string removes the comment.
- 7ab6ac1d7f50 14.23 landed
- 4574dd44da3c 15.18 landed
- 228addbbf2c1 16.14 landed
- ec80fab7a1fd 17.10 landed
- 47ad672a76d3 18.4 landed
- bae42a54e3df 19 (unreleased) landed
-
Hello.
- 577e21b34f86 7.1.1 cited