From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Euler Taveira <euler@eulerto.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: improve performance of pg_dump with many sequences
Date: Thu, 8 Jan 2026 13:34:28 -0600
Message-ID: <aWAGxA8igE1ZG_Xq@nathan> (raw)
In-Reply-To: <1980877.1767896379@sss.pgh.pa.us>
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>
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
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.cindex 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.cindex 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)
Attachments:
[text/plain] v2-0001-pg_dump-fix-use-of-pg_get_sequence_data.patch (3.7K, ../aWAGxA8igE1ZG_Xq@nathan/2-v2-0001-pg_dump-fix-use-of-pg_get_sequence_data.patch)
download | inline diff:
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.cindex 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.cindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, michael@paquier.xyz, euler@eulerto.com
Subject: Re: improve performance of pg_dump with many sequences
In-Reply-To: <aWAGxA8igE1ZG_Xq@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox