add --sequence-data to pg_dumpall

Started by Nathan Bossart9 months ago5 messages
#1Nathan Bossart
nathandbossart@gmail.com
1 attachment(s)

I noticed that I forgot to add --sequence-data to pg_dumpall in commit
9c49f0e, which added the same option to pg_dump. This option is primarily
intended for use by pg_upgrade, so we will need it if/when we teach
pg_upgrade to use pg_dumpall to dump the databases. (Right now pg_upgrade
directly calls pg_dump on each database.)

Assuming we want this patch, should we apply it to v18? It's arguably an
oversight in the pg_dump --sequence-data commit, and pg_dumpall will just
pass the option through to pg_dump, but otherwise there's not a really
strong reason it can't wait.

--
nathan

Attachments:

v1-0001-pg_dumpall-Add-sequence-data.patchtext/plain; charset=us-asciiDownload
From 41a52a6d48a67f5cbdb51931a0f14c416f3d6588 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 29 Apr 2025 15:03:31 -0500
Subject: [PATCH v1 1/1] pg_dumpall: Add --sequence-data.

I recently added this option to pg_dump, but I forgot to add it to
pg_dumpall, too.  There's probably little use for it at the moment,
but we will need it if/when we teach pg_upgrade to use pg_dumpall
to dump the databases.

Oversight in commit 9c49f0e8cd.
---
 doc/src/sgml/ref/pg_dumpall.sgml | 11 +++++++++++
 src/bin/pg_dump/pg_dumpall.c     |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 43fdab2d77e..e54e84f51a6 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -722,6 +722,17 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--sequence-data</option></term>
+      <listitem>
+       <para>
+        Include sequence data in the dump.  This is the default behavior except
+        when <option>--no-data</option>, <option>--schema-only</option>, or
+        <option>--statistics-only</option> is specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--use-set-session-authorization</option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 946a6d0fafc..7f9c302b719 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -114,6 +114,7 @@ static int	server_version;
 static int	load_via_partition_root = 0;
 static int	on_conflict_do_nothing = 0;
 static int	statistics_only = 0;
+static int	sequence_data = 0;
 
 static char role_catalog[10];
 #define PG_AUTHID "pg_authid"
@@ -189,6 +190,7 @@ main(int argc, char *argv[])
 		{"rows-per-insert", required_argument, NULL, 7},
 		{"statistics-only", no_argument, &statistics_only, 1},
 		{"filter", required_argument, NULL, 8},
+		{"sequence-data", no_argument, &sequence_data, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -505,6 +507,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --on-conflict-do-nothing");
 	if (statistics_only)
 		appendPQExpBufferStr(pgdumpopts, " --statistics-only");
+	if (sequence_data)
+		appendPQExpBufferStr(pgdumpopts, " --sequence-data");
 
 	/*
 	 * Open the output file if required, otherwise use stdout.  If required,
@@ -745,6 +749,7 @@ help(void)
 	printf(_("  --on-conflict-do-nothing     add ON CONFLICT DO NOTHING to INSERT commands\n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --rows-per-insert=NROWS      number of rows per INSERT; implies --inserts\n"));
+	printf(_("  --sequence-data              include sequence data in dump\n"));
 	printf(_("  --statistics-only            dump only the statistics, not schema or data\n"));
 	printf(_("  --use-set-session-authorization\n"
 			 "                               use SET SESSION AUTHORIZATION commands instead of\n"
-- 
2.39.5 (Apple Git-154)

#2Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#1)
Re: add --sequence-data to pg_dumpall

On Tue, Apr 29, 2025 at 03:55:08PM -0500, Nathan Bossart wrote:

I noticed that I forgot to add --sequence-data to pg_dumpall in commit
9c49f0e, which added the same option to pg_dump. This option is primarily
intended for use by pg_upgrade, so we will need it if/when we teach
pg_upgrade to use pg_dumpall to dump the databases. (Right now pg_upgrade
directly calls pg_dump on each database.)

Assuming we want this patch, should we apply it to v18? It's arguably an
oversight in the pg_dump --sequence-data commit, and pg_dumpall will just
pass the option through to pg_dump, but otherwise there's not a really
strong reason it can't wait.

This reminds me of, that fixed a similar defect in pg_dumpall
following the addition of an option in pg_dump where the former was
forgotten:
/messages/by-id/YKHC+qCJvzCRVCpY@paquier.xyz

I agree with applying that to v18 now and treat it as a defect rather
than wait for v19 and treat this patch as a new feature. Bonus points
for the patch being straight-forward.
--
Michael

#3Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#2)
Re: add --sequence-data to pg_dumpall

On Wed, Apr 30, 2025 at 09:29:59AM +0900, Michael Paquier wrote:

On Tue, Apr 29, 2025 at 03:55:08PM -0500, Nathan Bossart wrote:

Assuming we want this patch, should we apply it to v18? It's arguably an
oversight in the pg_dump --sequence-data commit, and pg_dumpall will just
pass the option through to pg_dump, but otherwise there's not a really
strong reason it can't wait.

This reminds me of, that fixed a similar defect in pg_dumpall
following the addition of an option in pg_dump where the former was
forgotten:
/messages/by-id/YKHC+qCJvzCRVCpY@paquier.xyz

I agree with applying that to v18 now and treat it as a defect rather
than wait for v19 and treat this patch as a new feature. Bonus points
for the patch being straight-forward.

Since there's precedent, I'll plan on committing this in the next few days
unless someone objects. I've added the rest of the RMT to this thread,
too, just in case.

--
nathan

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#3)
Re: add --sequence-data to pg_dumpall

On Wed, Apr 30, 2025 at 02:52:27PM -0500, Nathan Bossart wrote:

Since there's precedent, I'll plan on committing this in the next few days
unless someone objects. I've added the rest of the RMT to this thread,
too, just in case.

I brought this up in the RMT meeting today, and everyone was fine with
committing it for v18. I'll plan on doing so later this week.

--
nathan

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#4)
Re: add --sequence-data to pg_dumpall

On Tue, May 06, 2025 at 11:39:33AM -0500, Nathan Bossart wrote:

I brought this up in the RMT meeting today, and everyone was fine with
committing it for v18. I'll plan on doing so later this week.

Committed.

--
nathan