Обсуждение: doc: Clarify that empty COMMENT string removes the comment

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/

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
asNULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the
sameproblem. 
>
> 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
asNULL, which effectively removes the comment from the object. Today I also saw discussion [2], which mentioned the
sameproblem. 
> >
> > 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
itexplicitly? I don’t think so. For example, a similar command, SECURITY LABEL ON, treats an empty string as just an
emptystring. 
> >
> > 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 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
stringdoes directly. 

+1.

>
> The command Description should cover this if we are going to mention it in the Parameters section.  Otherwise a lone
commentin 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
writingit 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
makingthe whole thing more concise with no loss of content.  "the comment" is more precise than "a comment", as are the
effectsof executing the command. 

+1.


--
Best Wishes,
Ashutosh Bapat

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.

> 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
stringdoes 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 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
readsas 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
wherewe use this style of communication in the description (i.e., telling the user directly what they should be doing,
asopposed 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
pageconform to the standard established elsewhere.  I'm not seeing any particular virtue to leaving this page unique in
thatregard.  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
wherewe use this style of communication in the description (i.e., telling the user directly what they should be doing,
asopposed 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
pageconform to the standard established elsewhere.  I'm not seeing any particular virtue to leaving this page unique in
thatregard.  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
considerthe 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-specifictest files. I was hesitant to add empty-string cases everywhere, so for now I added a test case in
create_view.sqlto 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
letme know your preference. 

PFA v3.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

> 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

> 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
usersto 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
enablesusers 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
enablesusers 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/