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@momjian.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@momjian.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@momjian.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 commandsI 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@ewie.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@momjian.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 commandsI 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@ewie.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 throughoutAgreed.
--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@momjian.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?"
Attachments:
comment.difftext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index ffc29b04fb7..d66e901f51b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1075,7 +1075,7 @@ PostgreSQL documentation
<term><option>--no-comments</option></term>
<listitem>
<para>
- Do not dump comments.
+ Do not dump <command>COMMENT</command> commands.
</para>
</listitem>
</varlistentry>
On 4 Nov 2024, at 21:00, Bruce Momjian <bruce@momjian.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@momjian.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@momjian.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@ewie.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 throughoutAgreed.
--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@momjian.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@momjian.us>
escreveu:
Show quoted text
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@momjian.us> https://momjian.us
EDB https://enterprisedb.comWhen 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@momjian.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@momjian.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@f10.com.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@f10.com.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@momjian.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?"
Attachments:
master.difftext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 4d7c0464687..014f2792589 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -417,7 +417,7 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
<term><option>--no-comments</option></term>
<listitem>
<para>
- Do not dump comments.
+ Do not dump <command>COMMENT</command> commands.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a8c141b689d..c30aafbe70d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1212,7 +1212,7 @@ help(const char *progname)
" servers matching PATTERN\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
- printf(_(" --no-comments do not dump comments\n"));
+ printf(_(" --no-comments do not dump comment commands\n"));
printf(_(" --no-publications do not dump publications\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-subscriptions do not dump subscriptions\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index e3ad8fb2956..9a04e51c81a 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -662,7 +662,7 @@ help(void)
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
- printf(_(" --no-comments do not dump comments\n"));
+ printf(_(" --no-comments do not dump comment commands\n"));
printf(_(" --no-publications do not dump publications\n"));
printf(_(" --no-role-passwords do not dump passwords for roles\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index f2c1020d053..10da7d27da8 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -484,7 +484,7 @@ usage(const char *progname)
printf(_(" --filter=FILENAME restore or skip objects based on expressions\n"
" in FILENAME\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
- printf(_(" --no-comments do not restore comments\n"));
+ printf(_(" --no-comments do not restore comment commands\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-publications do not restore publications\n"));
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/