doc: Clarify that empty COMMENT string removes the comment

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]
/messages/by-id/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
[2]
/messages/by-id/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

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

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

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

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.

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

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.

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/

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.

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

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/

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.

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/

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

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] /messages/by-id/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
[2] /messages/by-id/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.

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] /messages/by-id/e08cb97f-0364-4002-9cda-3c16b42e4136@uni-muenster.de
[2] /messages/by-id/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.

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/

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

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/