Date: Thu, 8 Jan 2026 13:34:28 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>,
	Euler Taveira <euler@eulerto.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: improve performance of pg_dump with many sequences
Message-ID: <aWAGxA8igE1ZG_Xq@nathan>
References: <Zpl7HywbF2qAkTVf@nathan>
 <ZqFJspSbieuTE4X0@nathan>
 <ZqpV5Sb7YWauwYnx@nathan>
 <2885944.1767029161@sss.pgh.pa.us>
 <aV7m7rTty4KAb-tF@nathan>
 <1862355.1767827628@sss.pgh.pa.us>
 <aV7r43VN-kBR6T8v@nathan>
 <1869482.1767831892@sss.pgh.pa.us>
 <aV_rJv-JnMh6WWQw@nathan>
 <1980877.1767896379@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="WE5vjb+B2g0raN9A"
Content-Disposition: inline
In-Reply-To: <1980877.1767896379@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/aWAGxA8igE1ZG_Xq%40nathan>
Precedence: bulk


--WE5vjb+B2g0raN9A
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Jan 08, 2026 at 01:19:39PM -0500, Tom Lane wrote:
> One nitpicky point is that try_sequence_open() will still error out
> if it is given an OID that is a non-sequence relation.  I think it'd
> be more desirable for it to close the relation again and return NULL.
> That's probably insignificant for pg_dump's usage, because we could
> only hit the case with very improbable OID wraparound timing.  But
> I think our experience with catalog-inspection functions similar to
> pg_get_sequence_data is that it's usually better to return NULL than
> throw an error.

Hm.  That makes sense, but both try_table_open and try_index_open error for
wrong relkinds.  I could change all of the try_*_open functions to return
NULL in that case, or I could just open-code the relkind check in
pg_get_sequence_data after try_relation_open (and have it return NULL for
non-sequences).  I'm leaning towards the latter, if for no other reason
than it might be slightly nicer for back-patching (e.g., smaller, no new
extern functions).

-- 
nathan

--WE5vjb+B2g0raN9A
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v2-0001-pg_dump-fix-use-of-pg_get_sequence_data.patch

From 68a9b7fb9714805819b040f19851601f1426c990 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Thu, 8 Jan 2026 11:29:59 -0600
Subject: [PATCH v2 1/1] pg_dump: fix use of pg_get_sequence_data

---
 src/backend/commands/sequence.c | 14 ++++++++------
 src/bin/pg_dump/pg_dump.c       | 10 ++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 904eeada5ab..e1b808bbb60 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1794,7 +1794,6 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
 {
 #define PG_GET_SEQUENCE_DATA_COLS	3
 	Oid			relid = PG_GETARG_OID(0);
-	SeqTable	elm;
 	Relation	seqrel;
 	Datum		values[PG_GET_SEQUENCE_DATA_COLS] = {0};
 	bool		isnull[PG_GET_SEQUENCE_DATA_COLS] = {0};
@@ -1811,13 +1810,15 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
 					   LSNOID, -1, 0);
 	resultTupleDesc = BlessTupleDesc(resultTupleDesc);
 
-	init_sequence(relid, &elm, &seqrel);
+	seqrel = try_relation_open(relid, AccessShareLock);
 
 	/*
-	 * Return all NULLs for sequences for which we lack privileges, other
-	 * sessions' temporary sequences, and unlogged sequences on standbys.
+	 * Return all NULLs for missing sequences, sequences for which we lack
+	 * privileges, other sessions' temporary sequences, and unlogged sequences
+	 * on standbys.
 	 */
-	if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT) == ACLCHECK_OK &&
+	if (seqrel && seqrel->rd_rel->relkind == RELKIND_SEQUENCE &&
+		pg_class_aclcheck(relid, GetUserId(), ACL_SELECT) == ACLCHECK_OK &&
 		!RELATION_IS_OTHER_TEMP(seqrel) &&
 		(RelationIsPermanent(seqrel) || !RecoveryInProgress()))
 	{
@@ -1838,7 +1839,8 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
 	else
 		memset(isnull, true, sizeof(isnull));
 
-	sequence_close(seqrel, NoLock);
+	if (seqrel)
+		relation_close(seqrel, AccessShareLock);
 
 	resultHeapTuple = heap_form_tuple(resultTupleDesc, values, isnull);
 	result = HeapTupleGetDatum(resultHeapTuple);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7df56d8b1b0..573fb0c06a1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -137,6 +137,7 @@ typedef struct
 	int64		cache;			/* cache size */
 	int64		last_value;		/* last value of sequence */
 	bool		is_called;		/* whether nextval advances before returning */
+	bool		null_seqtuple;	/* did pg_get_sequence_data return nulls? */
 } SequenceItem;
 
 typedef enum OidOptions
@@ -18959,6 +18960,7 @@ collectSequences(Archive *fout)
 		sequences[i].cycled = (strcmp(PQgetvalue(res, i, 7), "t") == 0);
 		sequences[i].last_value = strtoi64(PQgetvalue(res, i, 8), NULL, 10);
 		sequences[i].is_called = (strcmp(PQgetvalue(res, i, 9), "t") == 0);
+		sequences[i].null_seqtuple = (PQgetisnull(res, i, 8) || PQgetisnull(res, i, 9));
 	}
 
 	PQclear(res);
@@ -19230,6 +19232,10 @@ dumpSequenceData(Archive *fout, const TableDataInfo *tdinfo)
 	bool		called;
 	PQExpBuffer query = createPQExpBuffer();
 
+	/* needn't bother if not dumping sequence data */
+	if (!fout->dopt->dumpData && !fout->dopt->sequence_data)
+		return;
+
 	/*
 	 * For versions >= 18, the sequence information is gathered in the sorted
 	 * array before any calls to dumpSequenceData().  See collectSequences()
@@ -19271,6 +19277,10 @@ dumpSequenceData(Archive *fout, const TableDataInfo *tdinfo)
 		entry = bsearch(&key, sequences, nsequences,
 						sizeof(SequenceItem), SequenceItemCmp);
 
+		if (entry->null_seqtuple)
+			pg_fatal("failed to get data for sequence \"%s\"; user may lack privileges or sequence may have been dropped",
+					 tbinfo->dobj.name);
+
 		last = entry->last_value;
 		called = entry->is_called;
 	}
-- 
2.39.5 (Apple Git-154)


--WE5vjb+B2g0raN9A--