pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feeddoc: Clarify that empty COMMENT string removes the comment
20+ messages / 7 participants
[nested] [flat]
* doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 06:07 Chao Li <li.evan.chao@gmail.com>
0 siblings, 4 replies; 20+ messages in thread
From: Chao Li @ 2026-02-26 06:07 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi,
While reviewing patch [1] before the holiday vacation, I noticed that the COMMENT ON command treats an empty string as NULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the same problem.
From the code, it seems the behavior is intentional:
```
/* Reduce empty-string to NULL case */
if (comment != NULL && strlen(comment) == 0)
comment = NULL;
```
However, the documentation does not explain this behavior. It currently only says:
```
string_literal
The new comment contents, written as a string literal.
```
Is it a common pattern that an empty string is treated as NULL, so that the documentation does not need to mention it explicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an empty string.
This tiny patch enhances the documentation of COMMENT ON to clarify that an empty string is treated as NULL.
[1] https://postgr.es/m/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
[2] https://postgr.es/m/CAHGQGwFJKg-aT5vajSU1v_B4K129u_SjZ3EGGHFSBhW967gcMA@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch (1.2K, ../../26476097-B1C1-4BA8-AA92-0AD0B8EC7190@gmail.com/2-v1-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch)
download | inline diff:
From 469348fb023b127002dca977e93191ddf8981243 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 26 Feb 2026 13:43:24 +0800
Subject: [PATCH v1] doc: Clarify that empty COMMENT string removes the comment
Clarify the documentation of COMMENT ON to state that specifying an empty
string is treated as NULL, meaning that the comment is removed.
This makes the behavior explicit and avoids possible confusion about how
empty strings are handled.
Author: Chao Li <lic@highgo.com>
Reviewed-by:
Discussion: https://postgr.es/m/
---
doc/src/sgml/ref/comment.sgml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 8d81244910b..e30c7c2917e 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -266,7 +266,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
+ is treated as <literal>NULL</literal> (that is, it drops the comment).
</para>
</listitem>
</varlistentry>
--
2.50.1 (Apple Git-155)
=
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 07:52 shengbin Zhao <zshengbin91@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
3 siblings, 0 replies; 20+ messages in thread
From: shengbin Zhao @ 2026-02-26 07:52 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Feb 26, 2026 at 2:07 PM Chao Li <li.evan.chao@gmail.com> wrote:
> Hi,
>
> While reviewing patch [1] before the holiday vacation, I noticed that the
> COMMENT ON command treats an empty string as NULL, which effectively
> removes the comment from the object. Today I also saw discussion [2], which
> mentioned the same problem.
>
> From the code, it seems the behavior is intentional:
> ```
> /* Reduce empty-string to NULL case */
> if (comment != NULL && strlen(comment) == 0)
> comment = NULL;
> ```
>
> However, the documentation does not explain this behavior. It currently
> only says:
> ```
> string_literal
> The new comment contents, written as a string literal.
> ```
>
> Is it a common pattern that an empty string is treated as NULL, so that
> the documentation does not need to mention it explicitly? I don’t think so.
> For example, a similar command, SECURITY LABEL ON, treats an empty string
> as just an empty string.
>
> This tiny patch enhances the documentation of COMMENT ON to clarify that
> an empty string is treated as NULL.
>
> [1]
> https://postgr.es/m/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
> [2]
> https://postgr.es/m/CAHGQGwFJKg-aT5vajSU1v_B4K129u_SjZ3EGGHFSBhW967gcMA@mail.gmail.com
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
Hi Evan,
I've looked at your patch, and I think your change is helpful. I applied it
locally and rendered the html page, it looks good to me.
Thanks!
B.R,
/Shengbin Zhao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 12:02 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
3 siblings, 2 replies; 20+ messages in thread
From: Ashutosh Bapat @ 2026-02-26 12:02 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Feb 26, 2026 at 11:37 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi,
>
> While reviewing patch [1] before the holiday vacation, I noticed that the COMMENT ON command treats an empty string as NULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the same problem.
>
> From the code, it seems the behavior is intentional:
> ```
> /* Reduce empty-string to NULL case */
> if (comment != NULL && strlen(comment) == 0)
> comment = NULL;
> ```
This code goes all the way back to
577e21b34f8629ce76651a6388298891f81be99a. So there's no point in
changing it now. Doc update is better.
>
> However, the documentation does not explain this behavior. It currently only says:
> ```
> string_literal
> The new comment contents, written as a string literal.
> ```
>
> Is it a common pattern that an empty string is treated as NULL, so that the documentation does not need to mention it explicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an empty string.
>
> This tiny patch enhances the documentation of COMMENT ON to clarify that an empty string is treated as NULL.
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.
For the sake of consistency, I would word the sentence "An empty ... "
to read more like NULL i.e. "Write an empty string to drop the
comment".
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 14:10 Fujii Masao <masao.fujii@gmail.com>
parent: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-02-26 14:10 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Feb 26, 2026 at 9:02 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Thu, Feb 26, 2026 at 11:37 AM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> > Hi,
> >
> > While reviewing patch [1] before the holiday vacation, I noticed that the COMMENT ON command treats an empty string as NULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the same problem.
> >
> > From the code, it seems the behavior is intentional:
> > ```
> > /* Reduce empty-string to NULL case */
> > if (comment != NULL && strlen(comment) == 0)
> > comment = NULL;
> > ```
>
> This code goes all the way back to
> 577e21b34f8629ce76651a6388298891f81be99a. So there's no point in
> changing it now. Doc update is better.
+1
> > However, the documentation does not explain this behavior. It currently only says:
> > ```
> > string_literal
> > The new comment contents, written as a string literal.
> > ```
> >
> > Is it a common pattern that an empty string is treated as NULL, so that the documentation does not need to mention it explicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an empty string.
> >
> > This tiny patch enhances the documentation of COMMENT ON to clarify that an empty string is treated as NULL.
Thanks for the patch! LGTM.
> 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.
>
> For the sake of consistency, I would word the sentence "An empty ... "
> to read more like NULL i.e. "Write an empty string to drop the
> comment".
At least for me, the supplementary description that the patch adds
looks sufficient.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 14:40 Jim Jones <jim.jones@uni-muenster.de>
parent: Fujii Masao <masao.fujii@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Jim Jones @ 2026-02-26 14:40 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On 26/02/2026 15:10, Fujii Masao wrote:
> At least for me, the supplementary description that the patch adds
> looks sufficient.
Same here. This now clearly states the behaviour of comments with empty
strings.
Thanks for the patch!
Best, Jim
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-26 15:56 David G. Johnston <david.g.johnston@gmail.com>
parent: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
1 sibling, 2 replies; 20+ messages in thread
From: David G. Johnston @ 2026-02-26 15:56 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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:
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 }
<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.
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.
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.
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.
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.
David J.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-27 03:19 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Ashutosh Bapat @ 2026-02-27 03:19 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-27 03:37 David G. Johnston <david.g.johnston@gmail.com>
parent: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: David G. Johnston @ 2026-02-27 03:37 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thursday, February 26, 2026, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
wrote:
> On Thu, Feb 26, 2026 at 9:27 PM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> >
>
> >
> > 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.
>
We don’t, but we get the intended benefit by listing the by far most likely
way anyone would actually write an empty string here. The comment within
string_literal covers any edge cases sufficiently. The ‘’ can behave as a
label of sorts just like “string_literal”.
I haven’t looked for similar precedent for this idea though and don’t feel
strongly that we need to introduce it here, just something to consider to
make the most read portion of any reference chapter imply this behavior.
David J.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-27 03:44 Chao Li <li.evan.chao@gmail.com>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Chao Li @ 2026-02-27 03:44 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On Feb 26, 2026, at 23:56, 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:
>
> 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/
Attachments:
[application/octet-stream] v2-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch (2.8K, ../../9E0B8DED-4A75-4E93-B731-62A4E7F92B2C@gmail.com/2-v2-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch)
download | inline diff:
From b5a833ddec47d90f63fc386bedbac5b78356b74e Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 26 Feb 2026 13:43:24 +0800
Subject: [PATCH v2] doc: Clarify that empty COMMENT string removes the comment
Clarify the documentation of COMMENT ON to state that specifying an empty
string is treated as NULL, meaning that the comment is removed.
This makes the behavior explicit and avoids possible confusion about how
empty strings are handled.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Shengbin Zhao <zshengbin91@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/26476097-B1C1-4BA8-AA92-0AD0B8EC7190@gmail.com
---
doc/src/sgml/ref/comment.sgml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 8d81244910b..6157fdf934c 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -80,13 +80,14 @@ 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 on 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.
+ comment, use <literal>NULL</literal> or an empty string (<literal>''</literal>).
Comments are automatically dropped when their object is dropped.
</para>
@@ -266,7 +267,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
+ (<literal>''</literal>) removes the comment.
</para>
</listitem>
</varlistentry>
@@ -275,7 +277,7 @@ COMMENT ON
<term><literal>NULL</literal></term>
<listitem>
<para>
- Write <literal>NULL</literal> to drop the comment.
+ Write <literal>NULL</literal> to remove the comment.
</para>
</listitem>
</varlistentry>
@@ -362,6 +364,7 @@ COMMENT ON TRANSFORM FOR hstore LANGUAGE plpython3u IS 'Transform between hstore
COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI';
COMMENT ON TYPE complex IS 'Complex number data type';
COMMENT ON VIEW my_view IS 'View of departmental costs';
+COMMENT ON VIEW my_view IS NULL;
</programlisting></para>
</refsect1>
--
2.50.1 (Apple Git-155)
=
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-27 16:44 David G. Johnston <david.g.johnston@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 2 replies; 20+ messages in thread
From: David G. Johnston @ 2026-02-27 16:44 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Feb 26, 2026 at 8:44 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> > On Feb 26, 2026, at 23:56, 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:
> >
> > 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.
David J.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-27 17:00 Fujii Masao <masao.fujii@gmail.com>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 0 replies; 20+ messages in thread
From: Fujii Masao @ 2026-02-27 17:00 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sat, Feb 28, 2026 at 1:45 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> On Thu, Feb 26, 2026 at 8:44 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>> > On Feb 26, 2026, at 23:56, 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:
>> >
>> > 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-28 00:12 Chao Li <li.evan.chao@gmail.com>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Chao Li @ 2026-02-28 00:12 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; Fujii Masao <masao.fujii@gmail.com>; +Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On Feb 28, 2026, at 00:44, David G. Johnston <david.g.johnston@gmail.com> wrote:
>
> <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.
>
I understand your point. To be honest, as a non-native English speaker, I’m not very sensitive to tone, and I didn’t consider the stylistic aspect when I made the change.
Based on your suggestion, I’ve rewritten the paragraph to use a descriptive tone:
```
<para>
Only one comment string is stored for each object. Issuing a new
<command>COMMENT</command> command for the same object replaces the
existing comment. Specifying <literal>NULL</literal> or an empty
string (<literal>''</literal>) removes the comment. Comments are
automatically dropped when their object is dropped.
</para>
```
Does this look better?
> From: Fujii Masao <masao.fujii@gmail.com>
>
> 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?
Fair point. I searched through the regression tests and noticed that COMMENT ON tests are spread across different object-specific test files. I was hesitant to add empty-string cases everywhere, so for now I added a test case in create_view.sql to verify that IS ‘' removes the comment as expected.
If you think it would be better to add an IS '' test wherever IS NULL appears, I’m happy to expand the coverage. Please let me know your preference.
PFA v3.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v3-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch (4.8K, ../../6DE4E50D-C195-4731-BF1C-C1134B69FA99@gmail.com/2-v3-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch)
download | inline diff:
From 4c2c602381dac1db2d20874e4b9c8320ed4aa5af Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 26 Feb 2026 13:43:24 +0800
Subject: [PATCH v3] doc: Clarify that empty COMMENT string removes the comment
Clarify the documentation of COMMENT ON to state that specifying an empty
string is treated as NULL, meaning that the comment is removed.
This makes the behavior explicit and avoids possible confusion about how
empty strings are handled.
Also adds a regress test case that uses empty string to remove a comment.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Shengbin Zhao <zshengbin91@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/26476097-B1C1-4BA8-AA92-0AD0B8EC7190@gmail.com
---
doc/src/sgml/ref/comment.sgml | 18 +++++++++++-------
src/test/regress/expected/create_index.out | 14 ++++++++++++++
src/test/regress/sql/create_index.sql | 4 ++++
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 8d81244910b..fa71144c914 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -80,14 +80,16 @@ 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 on 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.
- Comments are automatically dropped when their object is dropped.
+ Only one comment string is stored for each object. Issuing a new
+ <command>COMMENT</command> command for the same object replaces the
+ existing comment. Specifying <literal>NULL</literal> or an empty
+ string (<literal>''</literal>) removes the comment. Comments are
+ automatically dropped when their object is dropped.
</para>
<para>
@@ -266,7 +268,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
+ (<literal>''</literal>) removes the comment.
</para>
</listitem>
</varlistentry>
@@ -275,7 +278,7 @@ COMMENT ON
<term><literal>NULL</literal></term>
<listitem>
<para>
- Write <literal>NULL</literal> to drop the comment.
+ Write <literal>NULL</literal> to remove the comment.
</para>
</listitem>
</varlistentry>
@@ -362,6 +365,7 @@ COMMENT ON TRANSFORM FOR hstore LANGUAGE plpython3u IS 'Transform between hstore
COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI';
COMMENT ON TYPE complex IS 'Complex number data type';
COMMENT ON VIEW my_view IS 'View of departmental costs';
+COMMENT ON VIEW my_view IS NULL;
</programlisting></para>
</refsect1>
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c743fc769cb..55538c4c41e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -32,6 +32,20 @@ COMMENT ON INDEX six_wrong IS 'bad index';
ERROR: relation "six_wrong" does not exist
COMMENT ON INDEX six IS 'good index';
COMMENT ON INDEX six IS NULL;
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+ six_comment_is_null
+---------------------
+ t
+(1 row)
+
+COMMENT ON INDEX six IS 'add the comment back';
+COMMENT ON INDEX six IS ''; -- empty string removes the comment, same as NULL
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+ six_comment_is_null
+---------------------
+ t
+(1 row)
+
--
-- BTREE partial indices
--
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index eabc9623b20..82e4062a215 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -45,6 +45,10 @@ CREATE INDEX six ON shighway USING btree (name text_ops);
COMMENT ON INDEX six_wrong IS 'bad index';
COMMENT ON INDEX six IS 'good index';
COMMENT ON INDEX six IS NULL;
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+COMMENT ON INDEX six IS 'add the comment back';
+COMMENT ON INDEX six IS ''; -- empty string removes the comment, same as NULL
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
--
-- BTREE partial indices
--
2.50.1 (Apple Git-155)
=
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-02-28 01:21 David G. Johnston <david.g.johnston@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: David G. Johnston @ 2026-02-28 01:21 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> PFA v3.
>
>
Looks good to me.
David J.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-02 23:41 Chao Li <li.evan.chao@gmail.com>
parent: David G. Johnston <david.g.johnston@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Chao Li @ 2026-03-02 23:41 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; Fujii Masao <masao.fujii@gmail.com>; +Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On Feb 28, 2026, at 09:21, David G. Johnston <david.g.johnston@gmail.com> wrote:
>
> On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> PFA v3.
>
>
> Looks good to me.
>
> David J.
>
Added to CF for tracking: https://commitfest.postgresql.org/patch/6560/
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 00:28 Fujii Masao <masao.fujii@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-03-03 00:28 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Mar 3, 2026 at 8:42 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Feb 28, 2026, at 09:21, David G. Johnston <david.g.johnston@gmail.com> wrote:
> >
> > On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> > PFA v3.
Thanks for updating the patch! LGTM
One more comment: the handling of empty-string comments exists in both
CreateComments() and CreateSharedComments(). For better test coverage,
how about adding also the regression test to check that COMMENT with
empty-string works as expected for shared objects such as roles?
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 02:38 zhangqiang <zhang_qiang81@163.com>
parent: Chao Li <li.evan.chao@gmail.com>
3 siblings, 0 replies; 20+ messages in thread
From: zhangqiang @ 2026-03-03 02:38 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
At 2026-03-03 09:58:57, "Chao Li" <li.evan.chao@gmail.com> wrote:
>Hi,
>
>While reviewing patch [1] before the holiday vacation, I noticed that the COMMENT ON command treats an empty string as NULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the same problem.
>
>From the code, it seems the behavior is intentional:
>```
> /* Reduce empty-string to NULL case */
> if (comment != NULL && strlen(comment) == 0)
> comment = NULL;
>```
>
>However, the documentation does not explain this behavior. It currently only says:
>```
>string_literal
> The new comment contents, written as a string literal.
>```
>
>Is it a common pattern that an empty string is treated as NULL, so that the documentation does not need to mention it explicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an empty string.
>
>This tiny patch enhances the documentation of COMMENT ON to clarify that an empty string is treated as NULL.
>
>[1] https://postgr.es/m/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
>[2] https://postgr.es/m/CAHGQGwFJKg-aT5vajSU1v_B4K129u_SjZ3EGGHFSBhW967gcMA@mail.gmail.com
>
>Best regards,
>--
>Chao Li (Evan)
>HighGo Software Co., Ltd.
>https://www.highgo.com/
>
>
Hi hackers,I have verified that using an empty string does indeed delete comments. I agree this patch content enables users to better understand how the database program handles empty strings when using the COMMENT ON command in documentation, thereby improving the user experience.I think it's a good patch.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re:doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 03:07 zhangqiang <zhang_qiang81@163.com>
parent: Chao Li <li.evan.chao@gmail.com>
3 siblings, 0 replies; 20+ messages in thread
From: zhangqiang @ 2026-03-03 03:07 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
>At 2026-03-03 09:58:57, "Chao Li" <li.evan.chao@gmail.com> wrote:
>Hi,
>
>While reviewing patch [1] before the holiday vacation, I noticed that the COMMENT ON command treats an empty string as NULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the same problem.
>
>From the code, it seems the behavior is intentional:
>```
> /* Reduce empty-string to NULL case */
> if (comment != NULL && strlen(comment) == 0)
> comment = NULL;
>```
>
>However, the documentation does not explain this behavior. It currently only says:
>```
>string_literal
> The new comment contents, written as a string literal.
>```
>
>Is it a common pattern that an empty string is treated as NULL, so that the documentation does not need to mention it explicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an empty string.
>
>This tiny patch enhances the documentation of COMMENT ON to clarify that an empty string is treated as NULL.
>
>[1] https://postgr.es/m/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
>[2] https://postgr.es/m/CAHGQGwFJKg-aT5vajSU1v_B4K129u_SjZ3EGGHFSBhW967gcMA@mail.gmail.com
>
>Best regards,
>--
>Chao Li (Evan)
>HighGo Software Co., Ltd.
>https://www.highgo.com/
>
>
>
Hi hackers,I have verified that using an empty string does indeed delete comments. I agree this patch content enables users to better understand how the database program handles empty strings when using the COMMENT ON command in documentation, thereby improving the user experience.I think it's a good patch.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 03:16 Chao Li <li.evan.chao@gmail.com>
parent: Fujii Masao <masao.fujii@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Chao Li @ 2026-03-03 03:16 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; zhang_qiang81@163.com
> On Mar 3, 2026, at 08:28, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Mar 3, 2026 at 8:42 AM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Feb 28, 2026, at 09:21, David G. Johnston <david.g.johnston@gmail.com> wrote:
>>>
>>> On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>> PFA v3.
>
> Thanks for updating the patch! LGTM
>
> One more comment: the handling of empty-string comments exists in both
> CreateComments() and CreateSharedComments(). For better test coverage,
> how about adding also the regression test to check that COMMENT with
> empty-string works as expected for shared objects such as roles?
>
Sure. CreateSharedComments() handles database, tablespace and role, so as you suggested, I added tests for role in v4.
> From: zhangqiang <zhang_qiang81@163.com>
>
> Hi hackers,I have verified that using an empty string does indeed delete comments. I agree this patch content enables users to better understand how the database program handles empty strings when using the COMMENT ON command in documentation, thereby improving the user experience.I think it's a good patch.
>
Thanks for your review. Sounds good.
PFA v4:
* Add tests that use NULL and ‘’ to remove comments from a role object.
Best regards
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v4-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch (7.4K, ../../F88CAD6A-7CD2-445D-AE79-76F3F827B296@gmail.com/2-v4-0001-doc-Clarify-that-empty-COMMENT-string-removes-the.patch)
download | inline diff:
From fc703fb4f32818e6f7d3ba1fb2434e4f094bc415 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 26 Feb 2026 13:43:24 +0800
Subject: [PATCH v4] doc: Clarify that empty COMMENT string removes the comment
Clarify the documentation of COMMENT ON to state that specifying an empty
string is treated as NULL, meaning that the comment is removed.
This makes the behavior explicit and avoids possible confusion about how
empty strings are handled.
Also adds regress test cases that use empty string to remove a comment.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Shengbin Zhao <zshengbin91@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/26476097-B1C1-4BA8-AA92-0AD0B8EC7190@gmail.com
---
doc/src/sgml/ref/comment.sgml | 18 +++++++++------
src/test/regress/expected/create_index.out | 14 +++++++++++
src/test/regress/expected/create_role.out | 27 ++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 4 ++++
src/test/regress/sql/create_role.sql | 7 ++++++
5 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 8d81244910b..fa71144c914 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -80,14 +80,16 @@ 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 on 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.
- Comments are automatically dropped when their object is dropped.
+ Only one comment string is stored for each object. Issuing a new
+ <command>COMMENT</command> command for the same object replaces the
+ existing comment. Specifying <literal>NULL</literal> or an empty
+ string (<literal>''</literal>) removes the comment. Comments are
+ automatically dropped when their object is dropped.
</para>
<para>
@@ -266,7 +268,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
+ (<literal>''</literal>) removes the comment.
</para>
</listitem>
</varlistentry>
@@ -275,7 +278,7 @@ COMMENT ON
<term><literal>NULL</literal></term>
<listitem>
<para>
- Write <literal>NULL</literal> to drop the comment.
+ Write <literal>NULL</literal> to remove the comment.
</para>
</listitem>
</varlistentry>
@@ -362,6 +365,7 @@ COMMENT ON TRANSFORM FOR hstore LANGUAGE plpython3u IS 'Transform between hstore
COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI';
COMMENT ON TYPE complex IS 'Complex number data type';
COMMENT ON VIEW my_view IS 'View of departmental costs';
+COMMENT ON VIEW my_view IS NULL;
</programlisting></para>
</refsect1>
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c743fc769cb..55538c4c41e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -32,6 +32,20 @@ COMMENT ON INDEX six_wrong IS 'bad index';
ERROR: relation "six_wrong" does not exist
COMMENT ON INDEX six IS 'good index';
COMMENT ON INDEX six IS NULL;
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+ six_comment_is_null
+---------------------
+ t
+(1 row)
+
+COMMENT ON INDEX six IS 'add the comment back';
+COMMENT ON INDEX six IS ''; -- empty string removes the comment, same as NULL
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+ six_comment_is_null
+---------------------
+ t
+(1 row)
+
--
-- BTREE partial indices
--
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 46d4f9efe99..5d0f21820ed 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -102,6 +102,33 @@ CREATE ROLE regress_rolecreator CREATEROLE;
CREATE ROLE regress_hasprivs CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
-- ok, we should be able to modify a role we created
COMMENT ON ROLE regress_hasprivs IS 'some comment';
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NOT NULL AS has_comment;
+ has_comment
+-------------
+ t
+(1 row)
+
+COMMENT ON ROLE regress_hasprivs IS NULL;
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NULL AS no_comment;
+ no_comment
+------------
+ t
+(1 row)
+
+COMMENT ON ROLE regress_hasprivs IS 'add the comment back';
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NOT NULL AS has_comment;
+ has_comment
+-------------
+ t
+(1 row)
+
+COMMENT ON ROLE regress_hasprivs IS ''; -- empty string removes the comment, same as NULL
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NULL AS no_comment;
+ no_comment
+------------
+ t
+(1 row)
+
ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7;
-- fail, we should be unable to modify a role we did not create
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index eabc9623b20..82e4062a215 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -45,6 +45,10 @@ CREATE INDEX six ON shighway USING btree (name text_ops);
COMMENT ON INDEX six_wrong IS 'bad index';
COMMENT ON INDEX six IS 'good index';
COMMENT ON INDEX six IS NULL;
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
+COMMENT ON INDEX six IS 'add the comment back';
+COMMENT ON INDEX six IS ''; -- empty string removes the comment, same as NULL
+SELECT obj_description('six'::regclass, 'pg_class') IS NULL AS six_comment_is_null;
--
-- BTREE partial indices
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 4491a28a8ae..b22f9c6f50a 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -92,6 +92,13 @@ CREATE ROLE regress_hasprivs CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
-- ok, we should be able to modify a role we created
COMMENT ON ROLE regress_hasprivs IS 'some comment';
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NOT NULL AS has_comment;
+COMMENT ON ROLE regress_hasprivs IS NULL;
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NULL AS no_comment;
+COMMENT ON ROLE regress_hasprivs IS 'add the comment back';
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NOT NULL AS has_comment;
+COMMENT ON ROLE regress_hasprivs IS ''; -- empty string removes the comment, same as NULL
+SELECT shobj_description('regress_hasprivs'::regrole, 'pg_authid') IS NULL AS no_comment;
ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7;
--
2.50.1 (Apple Git-155)
=
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 05:51 Fujii Masao <masao.fujii@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-03-03 05:51 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; zhang_qiang81@163.com
On Tue, Mar 3, 2026 at 12:17 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Mar 3, 2026, at 08:28, Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Tue, Mar 3, 2026 at 8:42 AM Chao Li <li.evan.chao@gmail.com> wrote:
> >>
> >>
> >>
> >>> On Feb 28, 2026, at 09:21, David G. Johnston <david.g.johnston@gmail.com> wrote:
> >>>
> >>> On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >>>
> >>> PFA v3.
> >
> > Thanks for updating the patch! LGTM
> >
> > One more comment: the handling of empty-string comments exists in both
> > CreateComments() and CreateSharedComments(). For better test coverage,
> > how about adding also the regression test to check that COMMENT with
> > empty-string works as expected for shared objects such as roles?
> >
>
> Sure. CreateSharedComments() handles database, tablespace and role, so as you suggested, I added tests for role in v4.
>
> > From: zhangqiang <zhang_qiang81@163.com>
> >
> > Hi hackers,I have verified that using an empty string does indeed delete comments. I agree this patch content enables users to better understand how the database program handles empty strings when using the COMMENT ON command in documentation, thereby improving the user experience.I think it's a good patch.
> >
>
> Thanks for your review. Sounds good.
>
> PFA v4:
> * Add tests that use NULL and ‘’ to remove comments from a role object.
Thanks for updating the patch! I've pushed it.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: doc: Clarify that empty COMMENT string removes the comment
@ 2026-03-03 07:34 Chao Li <li.evan.chao@gmail.com>
parent: Fujii Masao <masao.fujii@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Chao Li @ 2026-03-03 07:34 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; zhang_qiang81@163.com
> On Mar 3, 2026, at 13:51, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Mar 3, 2026 at 12:17 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Mar 3, 2026, at 08:28, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>
>>> On Tue, Mar 3, 2026 at 8:42 AM Chao Li <li.evan.chao@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Feb 28, 2026, at 09:21, David G. Johnston <david.g.johnston@gmail.com> wrote:
>>>>>
>>>>> On Fri, Feb 27, 2026 at 5:12 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>>>
>>>>> PFA v3.
>>>
>>> Thanks for updating the patch! LGTM
>>>
>>> One more comment: the handling of empty-string comments exists in both
>>> CreateComments() and CreateSharedComments(). For better test coverage,
>>> how about adding also the regression test to check that COMMENT with
>>> empty-string works as expected for shared objects such as roles?
>>>
>>
>> Sure. CreateSharedComments() handles database, tablespace and role, so as you suggested, I added tests for role in v4.
>>
>>> From: zhangqiang <zhang_qiang81@163.com>
>>>
>>> Hi hackers,I have verified that using an empty string does indeed delete comments. I agree this patch content enables users to better understand how the database program handles empty strings when using the COMMENT ON command in documentation, thereby improving the user experience.I think it's a good patch.
>>>
>>
>> Thanks for your review. Sounds good.
>>
>> PFA v4:
>> * Add tests that use NULL and ‘’ to remove comments from a role object.
>
> Thanks for updating the patch! I've pushed it.
>
> Regards,
>
>
> --
> Fujii Masao
Hi Fujii-san, thank you very much for pushing.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2026-03-03 07:34 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-26 06:07 doc: Clarify that empty COMMENT string removes the comment Chao Li <li.evan.chao@gmail.com>
2026-02-26 07:52 ` shengbin Zhao <zshengbin91@gmail.com>
2026-02-26 12:02 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-02-26 14:10 ` Fujii Masao <masao.fujii@gmail.com>
2026-02-26 14:40 ` Jim Jones <jim.jones@uni-muenster.de>
2026-02-26 15:56 ` David G. Johnston <david.g.johnston@gmail.com>
2026-02-27 03:19 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-02-27 03:37 ` David G. Johnston <david.g.johnston@gmail.com>
2026-02-27 03:44 ` Chao Li <li.evan.chao@gmail.com>
2026-02-27 16:44 ` David G. Johnston <david.g.johnston@gmail.com>
2026-02-27 17:00 ` Fujii Masao <masao.fujii@gmail.com>
2026-02-28 00:12 ` Chao Li <li.evan.chao@gmail.com>
2026-02-28 01:21 ` David G. Johnston <david.g.johnston@gmail.com>
2026-03-02 23:41 ` Chao Li <li.evan.chao@gmail.com>
2026-03-03 00:28 ` Fujii Masao <masao.fujii@gmail.com>
2026-03-03 03:16 ` Chao Li <li.evan.chao@gmail.com>
2026-03-03 05:51 ` Fujii Masao <masao.fujii@gmail.com>
2026-03-03 07:34 ` Chao Li <li.evan.chao@gmail.com>
2026-03-03 02:38 ` zhangqiang <zhang_qiang81@163.com>
2026-03-03 03:07 ` zhangqiang <zhang_qiang81@163.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox