Re: pg_dump --no-comments confusion

Someone emailed me privately saying they were confused because they
thought pg_dump --no-comments would remove SQL comments, not the SQL
COMMENT commands. Is this something worth clarifying in our docs? I am
not even sure how I would express it. It currently says:

--no-comments
Do not dump comments.

We could change it to:

--no-comments
Do not dump SQL COMMENT commands

I think this is someone with limited English ability, which could
explain the confusion.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

On Mon, 4 Nov 2024 at 15:41, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> Someone emailed me privately saying they were confused because they
> thought pg_dump --no-comments would remove SQL comments, not the SQL
> COMMENT commands. Is this something worth clarifying in our docs? I am
> not even sure how I would express it. It currently says:
>
> --no-comments
> Do not dump comments.
>
> We could change it to:
>
> --no-comments
> Do not dump SQL COMMENT commands

I think that'd be more confusing, as SQL comments are /* */. There is
no SQL standard-prescribed COMMENT command (if our current docs are to
be believed, I don't have a recent version of ISO 9075 to verify that
claim).

Maybe: "Do not dump database object comments", or "Do not dump COMMENT
ON ... -commands"?

Kind regards,

Matthias van de Meent
Neon (https://neon.tech/)

On 2024-11-04 16:13 +0100, Matthias van de Meent wrote:
> On Mon, 4 Nov 2024 at 15:41, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >
> > Someone emailed me privately saying they were confused because they
> > thought pg_dump --no-comments would remove SQL comments, not the SQL
> > COMMENT commands. Is this something worth clarifying in our docs? I am
> > not even sure how I would express it. It currently says:
> >
> > --no-comments
> > Do not dump comments.
> >
> > We could change it to:
> >
> > --no-comments
> > Do not dump SQL COMMENT commands
>
> I think that'd be more confusing, as SQL comments are /* */. There is
> no SQL standard-prescribed COMMENT command (if our current docs are to
> be believed, I don't have a recent version of ISO 9075 to verify that
> claim).

I think Bruce's suggestion is pretty clear that it does not mean line or
block comments, but rather the COMMENT command. But I also think that
"SQL" in front of the command name is unnecessary because the man page
uses the "FOOBAR command" form throughout, e.g.:

--inserts
Dump data as INSERT commands [...]

Also, it doesn't really matter whether COMMENT is standard SQL. I guess
the sole purpose of --no-comment is to make the dump more portable. But
we don't mention that use case at all right now which would also apply
to --no-{publications,security-labels,subscriptions}.

> Maybe: "Do not dump database object comments",

I think that would be confusing because --verbose already reads "output
detailed object comments" which are in fact line comments.

--
Erik

> On 4 Nov 2024, at 17:24, Erik Wienhold <ewie(at)ewie(dot)name> wrote:
>
> On 2024-11-04 16:13 +0100, Matthias van de Meent wrote:
>> On Mon, 4 Nov 2024 at 15:41, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>>
>>> Someone emailed me privately saying they were confused because they
>>> thought pg_dump --no-comments would remove SQL comments, not the SQL
>>> COMMENT commands. Is this something worth clarifying in our docs? I am
>>> not even sure how I would express it. It currently says:
>>>
>>> --no-comments
>>> Do not dump comments.
>>>
>>> We could change it to:
>>>
>>> --no-comments
>>> Do not dump SQL COMMENT commands
>>
>> I think that'd be more confusing, as SQL comments are /* */. There is
>> no SQL standard-prescribed COMMENT command (if our current docs are to
>> be believed, I don't have a recent version of ISO 9075 to verify that
>> claim).
>
> I think Bruce's suggestion is pretty clear that it does not mean line or
> block comments, but rather the COMMENT command.

I think so too, I think it would be a good change.

> But I also think that
> "SQL" in front of the command name is unnecessary because the man page
> uses the "FOOBAR command" form throughout

Agreed.

> --inserts
> Dump data as INSERT commands [...]
>
> Also, it doesn't really matter whether COMMENT is standard SQL.

AFAIK some flavor of COMMENT is present in MySQL, PostgreSQL and Oracle which
already makes it more "standardized" than many parts of the SQL standard =)

--
Daniel Gustafsson

On Mon, Nov 4, 2024 at 07:49:45PM +0100, Daniel Gustafsson wrote:
> > On 4 Nov 2024, at 17:24, Erik Wienhold <ewie(at)ewie(dot)name> wrote:
> > But I also think that
> > "SQL" in front of the command name is unnecessary because the man page
> > uses the "FOOBAR command" form throughout
>
> Agreed.
>
> > --inserts
> > Dump data as INSERT commands [...]
> >
> > Also, it doesn't really matter whether COMMENT is standard SQL.
>
> AFAIK some flavor of COMMENT is present in MySQL, PostgreSQL and Oracle which
> already makes it more "standardized" than many parts of the SQL standard =)

Proposed patch attached.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

> On 4 Nov 2024, at 21:00, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> Proposed patch attached.

LGTM.

--
Daniel Gustafsson

On 2024-11-04 21:03 +0100, Daniel Gustafsson wrote:
> > On 4 Nov 2024, at 21:00, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> > Proposed patch attached.
>
> LGTM.

Seconded.

--
Erik

On 2024-Nov-04, Erik Wienhold wrote:

> I think Bruce's suggestion is pretty clear that it does not mean line or
> block comments, but rather the COMMENT command. But I also think that
> "SQL" in front of the command name is unnecessary [...]

+1 for "Do not dump COMMENT commands", which is what I think you're
saying.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, C.S. Lewis)

On Mon, 4 Nov 2024 at 21:00, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Mon, Nov 4, 2024 at 07:49:45PM +0100, Daniel Gustafsson wrote:
> > > On 4 Nov 2024, at 17:24, Erik Wienhold <ewie(at)ewie(dot)name> wrote:
> > > But I also think that
> > > "SQL" in front of the command name is unnecessary because the man page
> > > uses the "FOOBAR command" form throughout
> >
> > Agreed.
> >
> > > --inserts
> > > Dump data as INSERT commands [...]
> > >
> > > Also, it doesn't really matter whether COMMENT is standard SQL.
> >
> > AFAIK some flavor of COMMENT is present in MySQL, PostgreSQL and Oracle which
> > already makes it more "standardized" than many parts of the SQL standard =)

Oh, I didn't know that, TIL.

> Proposed patch attached.

LGTM.

-Matthias

On Tue, Nov 5, 2024 at 10:12:20AM +0100, Álvaro Herrera wrote:
> On 2024-Nov-04, Erik Wienhold wrote:
>
> > I think Bruce's suggestion is pretty clear that it does not mean line or
> > block comments, but rather the COMMENT command. But I also think that
> > "SQL" in front of the command name is unnecessary [...]
>
> +1 for "Do not dump COMMENT commands", which is what I think you're
> saying.

Patch applied to master.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

But it would be good to have this patch applied to all supported versions,
as soon as nothing was changed on that pg_dump option, no ?

regards
Marcos

Em seg., 18 de nov. de 2024 às 18:30, Bruce Momjian <bruce(at)momjian(dot)us>
escreveu:

> On Tue, Nov 5, 2024 at 10:12:20AM +0100, Álvaro Herrera wrote:
> > On 2024-Nov-04, Erik Wienhold wrote:
> >
> > > I think Bruce's suggestion is pretty clear that it does not mean line
> or
> > > block comments, but rather the COMMENT command. But I also think that
> > > "SQL" in front of the command name is unnecessary [...]
> >
> > +1 for "Do not dump COMMENT commands", which is what I think you're
> > saying.
>
> Patch applied to master.
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
> EDB https://enterprisedb.com
>
> When a patient asks the doctor, "Am I going to die?", he means
> "Am I going to die soon?"
>
>
>

On Mon, Nov 18, 2024 at 06:36:45PM -0300, Marcos Pegoraro wrote:
> But it would be good to have this patch applied to all supported versions, as
> soon as nothing was changed on that pg_dump option, no ?
>
> Em seg., 18 de nov. de 2024 às 18:30, Bruce Momjian <bruce(at)momjian(dot)us>
> escreveu:
>
> On Tue, Nov  5, 2024 at 10:12:20AM +0100, Álvaro Herrera wrote:
> > On 2024-Nov-04, Erik Wienhold wrote:
> >
> > > I think Bruce's suggestion is pretty clear that it does not mean line
> or
> > > block comments, but rather the COMMENT command.  But I also think that
> > > "SQL" in front of the command name is unnecessary [...]
> >
> > +1 for "Do not dump COMMENT commands", which is what I think you're
> > saying.
>
> Patch applied to master.

Well, I have these notes about doc changes:

* inaccuracies, apply to all applicable supported versions
* missing docs, master and most recent major version
* unclear language or major edits, master only

and it seems this patch matches the last one, which is why I only did
master. Of course, these are just guidelines --- if people want me to,
I can apply it to all supported braches.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

Marcos Pegoraro <marcos(at)f10(dot)com(dot)br> writes:
> But it would be good to have this patch applied to all supported versions,
> as soon as nothing was changed on that pg_dump option, no ?

Even more to the point, should we change pg_dump's help output?

...
--load-via-partition-root load partitions via the root table
--no-comments do not dump comments
--no-publications do not dump publications
...

Also, the identical text appears in pg_dumpall's man page and help
output, while pg_restore has a differently worded version:

printf(_(" --no-comments do not restore comments\n"));

pg_restore's man page seems OK though:

Do not output commands to restore comments, even if the archive
contains them.

Note: I would not argue for back-patching changes in the help output,
as that creates translation issues. So probably back-patching the
SGML changes isn't appropriate either. But we should make all of this
consistent in master.

regards, tom lane

On Mon, Nov 18, 2024 at 05:14:53PM -0500, Tom Lane wrote:
> Marcos Pegoraro <marcos(at)f10(dot)com(dot)br> writes:
> > But it would be good to have this patch applied to all supported versions,
> > as soon as nothing was changed on that pg_dump option, no ?
>
> Even more to the point, should we change pg_dump's help output?
>
> ...
> --load-via-partition-root load partitions via the root table
> --no-comments do not dump comments
> --no-publications do not dump publications
> ...
>
> Also, the identical text appears in pg_dumpall's man page and help
> output, while pg_restore has a differently worded version:
>
> printf(_(" --no-comments do not restore comments\n"));
>
> pg_restore's man page seems OK though:
>
> Do not output commands to restore comments, even if the archive
> contains them.

Fixed in the attached applied patch.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

On 2024-Nov-04, Bruce Momjian wrote:

> Someone emailed me privately saying they were confused because [...]

> I think this is someone with limited English ability, which could
> explain the confusion.

I just realized you added "Reported-by: Tom Lane" to the commit message
of this change (f722dd32de49), which seems at odds with the idea of this
being someone with limited English ability, and frankly quite hilarious.

(I also noticed that you add blank lines between each of the trailer
lines in the commit message, which is at odds with what everybody else
does, both in Postgres and elsewhere.)

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/