| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
| Cc: | 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 03:44:15 |
| Message-ID: | 9E0B8DED-4A75-4E93-B731-62A4E7F92B2C@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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.
>
> <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>
>
I like this. Truly, the command not only adds a new comment, but also update/remove an existing comment.
> <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.
>
> @@ -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.
>
Agreed.
Thanks all for your review. Please see the attached v2.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch | application/octet-stream | 2.8 KB |
| unknown_filename | text/plain | 3 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-02-27 03:52:48 | Re: Defects with invalid stats data for expressions in extended stats |
| Previous Message | David G. Johnston | 2026-02-27 03:37:31 | Re: doc: Clarify that empty COMMENT string removes the comment |