| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(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>, 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 03:19:44 |
| Message-ID: | CAExHW5tahqPf=V0x6FhsMENm=2upJTVcOa+cF5CqaHi4Frptww@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Feb 26, 2026 at 9:27 PM 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:
+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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David G. Johnston | 2026-02-27 03:37:31 | Re: doc: Clarify that empty COMMENT string removes the comment |
| Previous Message | Michael Paquier | 2026-02-27 03:05:28 | Non-compliant SASLprep implementation for ASCII characters |