public inbox for [email protected]
help / color / mirror / Atom feedFix publisher-side sequence permission reporting
20+ messages / 5 participants
[nested] [flat]
* Fix publisher-side sequence permission reporting
@ 2026-06-18 15:36 Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-18 15:36 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi,
While testing logical replication sequence synchronization, I found
that a publisher-side permission problem can be reported as a
misleading "missing sequence on publisher" warning.
The issue can be reproduced as follows:
1. On the publisher:
CREATE SEQUENCE myseq;
CREATE PUBLICATION mypub FOR ALL SEQUENCES;
CREATE ROLE foo LOGIN REPLICATION NOSUPERUSER;
2. On the subscriber:
CREATE SEQUENCE myseq;
CREATE SUBSCRIPTION mysub
CONNECTION 'user=foo dbname=postgres ...'
PUBLICATION mypub;
The subscriber currently emits:
WARNING: missing sequence on publisher ("public.myseq")
even though the sequence still exists on the publisher. The real
problem is that the replication connection lacks SELECT privilege to
read the sequence data.
The cause is that; sequence synchronization obtains sequence data using
pg_get_sequence_data(). When the user lacks SELECT privilege on
the sequence, pg_get_sequence_data() returns a row containing all NULL
values. Sequence synchronization currently treats that the same as
a missing sequence, so publisher-side permission failures and
genuinely missing sequences are not distinguished, leading to
the misleading warning.
Patch 0001 fixes this by distinguishing the two cases during sequence
synchronization. It checks whether the replication connection has the
required privilege for each published sequence and reports
publisher-side permission failures separately.
While working on this, I also noticed that the documented privilege
requirement for pg_get_sequence_data() does not match the
implementation. The documentation says that USAGE or SELECT privilege
is sufficient, but the implementation requires SELECT.
Patch 0002 updates the documentation to match the current behavior.
I chose to update the documentation rather than broaden the
implementation for two reasons.
First, commit c8b06bb969b, which introduced the predecessor of
pg_get_sequence_data(), described it as a substitute for SELECT
from a sequence, and its implementation has always required
SELECT privilege.
Second, the logical replication documentation already states that
replicating sequence data requires SELECT privilege.
Patches attached.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v1-0001-Fix-misreporting-of-publisher-sequence-permission.patch (11.2K, ../../CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com/2-v1-0001-Fix-misreporting-of-publisher-sequence-permission.patch)
download | inline diff:
From 479131405fce33fb52c5b2a2ddea7127a876c221 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 18 Jun 2026 21:47:24 +0900
Subject: [PATCH v1 1/2] Fix misreporting of publisher sequence permissions
during sync
When synchronizing sequences for logical replication, a
publisher-side permission failure could be reported as if the sequence
were missing on the publisher, making the real cause harder to
identify.
This happened because pg_get_sequence_data() returns a row of NULL
values when the replication connection lacks permission to read a
sequence. Sequence synchronization treated that the same as a missing
sequence, causing it to emit a misleading "missing sequence on
publisher" warning.
Fix this by distinguishing permission failures from genuinely missing
sequences. The synchronization query now checks whether the
replication connection has the required privilege for each published
sequence, allowing the worker to report permission failures
separately.
---
.../replication/logical/sequencesync.c | 98 +++++++++++++------
src/test/subscription/t/036_sequences.pl | 20 +++-
2 files changed, 86 insertions(+), 32 deletions(-)
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index e2ff8d77b16..13e593649a4 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -72,13 +72,14 @@
#include "utils/syscache.h"
#include "utils/usercontext.h"
-#define REMOTE_SEQ_COL_COUNT 10
+#define REMOTE_SEQ_COL_COUNT 11
typedef enum CopySeqResult
{
COPYSEQ_SUCCESS,
COPYSEQ_MISMATCH,
- COPYSEQ_INSUFFICIENT_PERM,
+ COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM,
+ COPYSEQ_PUBLISHER_INSUFFICIENT_PERM,
COPYSEQ_SKIPPED
} CopySeqResult;
@@ -166,18 +167,22 @@ get_sequences_string(List *seqindexes, StringInfo buf)
* Report discrepancies found during sequence synchronization between
* the publisher and subscriber. Emits warnings for:
* a) mismatched definitions or concurrent rename
- * b) insufficient privileges
- * c) missing sequences on the subscriber
+ * b) insufficient privileges on the subscriber
+ * c) insufficient privileges on the publisher
+ * d) missing sequences on the publisher
* Then raises an ERROR to indicate synchronization failure.
*/
static void
-report_sequence_errors(List *mismatched_seqs_idx, List *insuffperm_seqs_idx,
+report_sequence_errors(List *mismatched_seqs_idx,
+ List *sub_insuffperm_seqs_idx,
+ List *pub_insuffperm_seqs_idx,
List *missing_seqs_idx)
{
StringInfoData seqstr;
/* Quick exit if there are no errors to report */
- if (!mismatched_seqs_idx && !insuffperm_seqs_idx && !missing_seqs_idx)
+ if (!mismatched_seqs_idx && !sub_insuffperm_seqs_idx &&
+ !pub_insuffperm_seqs_idx && !missing_seqs_idx)
return;
initStringInfo(&seqstr);
@@ -193,14 +198,25 @@ report_sequence_errors(List *mismatched_seqs_idx, List *insuffperm_seqs_idx,
seqstr.data));
}
- if (insuffperm_seqs_idx)
+ if (sub_insuffperm_seqs_idx)
{
- get_sequences_string(insuffperm_seqs_idx, &seqstr);
+ get_sequences_string(sub_insuffperm_seqs_idx, &seqstr);
ereport(WARNING,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg_plural("insufficient privileges on sequence (%s)",
- "insufficient privileges on sequences (%s)",
- list_length(insuffperm_seqs_idx),
+ errmsg_plural("insufficient privileges on subscriber sequence (%s)",
+ "insufficient privileges on subscriber sequences (%s)",
+ list_length(sub_insuffperm_seqs_idx),
+ seqstr.data));
+ }
+
+ if (pub_insuffperm_seqs_idx)
+ {
+ get_sequences_string(pub_insuffperm_seqs_idx, &seqstr);
+ ereport(WARNING,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg_plural("insufficient privileges on publisher sequence (%s)",
+ "insufficient privileges on publisher sequences (%s)",
+ list_length(pub_insuffperm_seqs_idx),
seqstr.data));
}
@@ -235,6 +251,7 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
bool isnull;
int col = 0;
Datum datum;
+ bool remote_has_select_priv;
Oid remote_typid;
int64 remote_start;
int64 remote_increment;
@@ -253,13 +270,20 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
*seqinfo = seqinfo_local =
(LogicalRepSequenceInfo *) list_nth(seqinfos, *seqidx);
+ seqinfo_local->found_on_pub = true;
+
/*
- * The sequence data can be NULL due to insufficient privileges or if the
- * sequence was dropped concurrently (see pg_get_sequence_data()).
+ * The remote sequence state can be NULL if the publisher lacks the
+ * required privileges or if the sequence was dropped concurrently after
+ * it was identified in the catalog snapshot (see pg_get_sequence_data()).
*/
+ remote_has_select_priv = DatumGetBool(slot_getattr(slot, ++col, &isnull));
+ Assert(!isnull);
+
datum = slot_getattr(slot, ++col, &isnull);
if (isnull)
- return COPYSEQ_SKIPPED;
+ return remote_has_select_priv ? COPYSEQ_SKIPPED :
+ COPYSEQ_PUBLISHER_INSUFFICIENT_PERM;
seqinfo_local->last_value = DatumGetInt64(datum);
seqinfo_local->is_called = DatumGetBool(slot_getattr(slot, ++col, &isnull));
@@ -289,8 +313,6 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
/* Sanity check */
Assert(col == REMOTE_SEQ_COL_COUNT);
- seqinfo_local->found_on_pub = true;
-
*sequence_rel = try_table_open(seqinfo_local->localrelid, RowExclusiveLock);
/* Sequence was concurrently dropped? */
@@ -351,7 +373,7 @@ copy_sequence(LogicalRepSequenceInfo *seqinfo, Oid seqowner)
if (!run_as_owner)
RestoreUserContext(&ucxt);
- return COPYSEQ_INSUFFICIENT_PERM;
+ return COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM;
}
/*
@@ -387,7 +409,8 @@ copy_sequences(WalReceiverConn *conn)
int n_seqinfos = list_length(seqinfos);
List *mismatched_seqs_idx = NIL;
List *missing_seqs_idx = NIL;
- List *insuffperm_seqs_idx = NIL;
+ List *sub_insuffperm_seqs_idx = NIL;
+ List *pub_insuffperm_seqs_idx = NIL;
StringInfoData seqstr;
StringInfoData cmd;
MemoryContext oldctx;
@@ -403,13 +426,14 @@ copy_sequences(WalReceiverConn *conn)
while (cur_batch_base_index < n_seqinfos)
{
- Oid seqRow[REMOTE_SEQ_COL_COUNT] = {INT8OID, INT8OID,
+ Oid seqRow[REMOTE_SEQ_COL_COUNT] = {INT8OID, BOOLOID, INT8OID,
BOOLOID, LSNOID, OIDOID, INT8OID, INT8OID, INT8OID, INT8OID, BOOLOID};
int batch_size = 0;
int batch_succeeded_count = 0;
int batch_mismatched_count = 0;
int batch_skipped_count = 0;
- int batch_insuffperm_count = 0;
+ int batch_sub_insuffperm_count = 0;
+ int batch_pub_insuffperm_count = 0;
int batch_missing_count;
WalRcvExecResult *res;
@@ -471,7 +495,8 @@ copy_sequences(WalReceiverConn *conn)
* matching.
*/
appendStringInfo(&cmd,
- "SELECT s.seqidx, ps.*, seq.seqtypid,\n"
+ "SELECT s.seqidx, has_sequence_privilege(c.oid, 'SELECT'),\n"
+ " ps.*, seq.seqtypid,\n"
" seq.seqstart, seq.seqincrement, seq.seqmin,\n"
" seq.seqmax, seq.seqcycle\n"
"FROM ( VALUES %s ) AS s (schname, seqname, seqidx)\n"
@@ -532,7 +557,7 @@ copy_sequences(WalReceiverConn *conn)
MemoryContextSwitchTo(oldctx);
batch_mismatched_count++;
break;
- case COPYSEQ_INSUFFICIENT_PERM:
+ case COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM:
/*
* Remember sequences with insufficient privileges in a
@@ -540,10 +565,22 @@ copy_sequences(WalReceiverConn *conn)
* after the transaction is committed.
*/
oldctx = MemoryContextSwitchTo(ApplyContext);
- insuffperm_seqs_idx = lappend_int(insuffperm_seqs_idx,
- seqidx);
+ sub_insuffperm_seqs_idx = lappend_int(sub_insuffperm_seqs_idx,
+ seqidx);
+ MemoryContextSwitchTo(oldctx);
+ batch_sub_insuffperm_count++;
+ break;
+ case COPYSEQ_PUBLISHER_INSUFFICIENT_PERM:
+
+ /*
+ * Remember sequences for which the publisher lacks the
+ * privileges required by pg_get_sequence_data().
+ */
+ oldctx = MemoryContextSwitchTo(ApplyContext);
+ pub_insuffperm_seqs_idx = lappend_int(pub_insuffperm_seqs_idx,
+ seqidx);
MemoryContextSwitchTo(oldctx);
- batch_insuffperm_count++;
+ batch_pub_insuffperm_count++;
break;
case COPYSEQ_SKIPPED:
@@ -575,15 +612,16 @@ copy_sequences(WalReceiverConn *conn)
batch_missing_count = batch_size - (batch_succeeded_count +
batch_mismatched_count +
- batch_insuffperm_count +
+ batch_sub_insuffperm_count +
+ batch_pub_insuffperm_count +
batch_skipped_count);
elog(DEBUG1,
- "logical replication sequence synchronization for subscription \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d insufficient permission, %d missing from publisher, %d skipped",
+ "logical replication sequence synchronization for subscription \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d subscriber insufficient permission, %d publisher insufficient permission, %d missing from publisher, %d skipped",
MySubscription->name,
(cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
batch_size, batch_succeeded_count, batch_mismatched_count,
- batch_insuffperm_count, batch_missing_count, batch_skipped_count);
+ batch_sub_insuffperm_count, batch_pub_insuffperm_count, batch_missing_count, batch_skipped_count);
/* Commit this batch, and prepare for next batch */
CommitTransactionCommand();
@@ -610,8 +648,8 @@ copy_sequences(WalReceiverConn *conn)
}
/* Report mismatches, permission issues, or missing sequences */
- report_sequence_errors(mismatched_seqs_idx, insuffperm_seqs_idx,
- missing_seqs_idx);
+ report_sequence_errors(mismatched_seqs_idx, sub_insuffperm_seqs_idx,
+ pub_insuffperm_seqs_idx, missing_seqs_idx);
}
/*
diff --git a/src/test/subscription/t/036_sequences.pl b/src/test/subscription/t/036_sequences.pl
index 8e293871efb..185a563b239 100644
--- a/src/test/subscription/t/036_sequences.pl
+++ b/src/test/subscription/t/036_sequences.pl
@@ -234,8 +234,8 @@ $node_publisher->safe_psql(
##########
# Ensure that insufficient privileges on the publisher for a sequence do not
-# disrupt the subscriber. The subscriber should log a warning and continue
-# retrying.
+# get misreported as a missing sequence. The subscriber should log a warning
+# and continue retrying.
##########
$node_publisher->safe_psql(
@@ -254,6 +254,22 @@ $node_subscriber->safe_psql('postgres',
"ALTER SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_limited_connstr'"
);
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+$node_subscriber->wait_for_log(
+ qr/WARNING: ( [A-Z0-9]+:)? insufficient privileges on publisher sequence \("public.regress_s2"\)/,
+ $log_offset);
+
+##########
+# Ensure that a sequence that is actually removed on the publisher is still
+# reported as missing.
+##########
+
+$node_publisher->safe_psql('postgres', qq(DROP SEQUENCE regress_s2;));
+
+$log_offset = -s $node_subscriber->logfile;
+
$node_subscriber->safe_psql('postgres',
"ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
--
2.53.0
[application/octet-stream] v1-0002-doc-Clarify-pg_get_sequence_data-privileges-and-N.patch (2.0K, ../../CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com/3-v1-0002-doc-Clarify-pg_get_sequence_data-privileges-and-N.patch)
download | inline diff:
From 9a196d9e82fb20c0df038ce486f153bfea64e3ce Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 18 Jun 2026 21:46:27 +0900
Subject: [PATCH v1 2/2] doc: Clarify pg_get_sequence_data() privileges and
NULL results
The documentation for pg_get_sequence_data() did not match the
function's behavior. It stated that either USAGE or SELECT privilege
was sufficient, but the function returns sequence data only when the
caller has SELECT privilege.
The documentation also did not explain that the function returns a row
containing all NULL values when sequence data cannot be returned, such
as when the sequence does not exist or the caller lacks the required
privilege.
Update the documentation to reflect the actual behavior, including the
required privilege and the result returned when sequence data is
unavailable.
---
doc/src/sgml/func/func-sequence.sgml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func/func-sequence.sgml b/doc/src/sgml/func/func-sequence.sgml
index 4a2a6dc9369..de266c36296 100644
--- a/doc/src/sgml/func/func-sequence.sgml
+++ b/doc/src/sgml/func/func-sequence.sgml
@@ -163,12 +163,13 @@ SELECT setval('myseq', 42, false); <lineannotation>Next <function>nextval</fu
<structfield>is_called</structfield> indicates whether the sequence has
been used. <structfield>page_lsn</structfield> is the LSN corresponding
to the most recent WAL record that modified this sequence relation.
+ This function returns a row of NULL values if the sequence does not
+ exist or if the current user lacks privileges on it.
</para>
<para>
This function is primarily intended for internal use by pg_dump and by
logical replication to synchronize sequences. It requires
- <literal>USAGE</literal> or <literal>SELECT</literal> privilege on the
- sequence.
+ <literal>SELECT</literal> privilege on the sequence.
</para></entry>
</row>
</tbody>
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-18 23:11 Tristan Partin <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Tristan Partin @ 2026-06-18 23:11 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi Masao-san,
On Thu Jun 18, 2026 at 10:36 AM CDT, Fujii Masao wrote:
> Hi,
>
> While testing logical replication sequence synchronization, I found
> that a publisher-side permission problem can be reported as a
> misleading "missing sequence on publisher" warning.
>
> The issue can be reproduced as follows:
>
> 1. On the publisher:
>
> CREATE SEQUENCE myseq;
> CREATE PUBLICATION mypub FOR ALL SEQUENCES;
> CREATE ROLE foo LOGIN REPLICATION NOSUPERUSER;
>
> 2. On the subscriber:
>
> CREATE SEQUENCE myseq;
> CREATE SUBSCRIPTION mysub
> CONNECTION 'user=foo dbname=postgres ...'
> PUBLICATION mypub;
>
> The subscriber currently emits:
>
> WARNING: missing sequence on publisher ("public.myseq")
>
> even though the sequence still exists on the publisher. The real
> problem is that the replication connection lacks SELECT privilege to
> read the sequence data.
>
> The cause is that; sequence synchronization obtains sequence data using
> pg_get_sequence_data(). When the user lacks SELECT privilege on
> the sequence, pg_get_sequence_data() returns a row containing all NULL
> values. Sequence synchronization currently treats that the same as
> a missing sequence, so publisher-side permission failures and
> genuinely missing sequences are not distinguished, leading to
> the misleading warning.
>
> Patch 0001 fixes this by distinguishing the two cases during sequence
> synchronization. It checks whether the replication connection has the
> required privilege for each published sequence and reports
> publisher-side permission failures separately.
The patch looks good to me! I had one suggestion:
> ##########
> # Ensure that insufficient privileges on the publisher for a sequence do not
> -# disrupt the subscriber. The subscriber should log a warning and continue
> -# retrying.
> +# get misreported as a missing sequence. The subscriber should log a warning
> +# and continue retrying.
> ##########
I think a better comment might be:
Ensure that insufficient privileges on the publisher for a sequence
are reported correctly...
My reasoning for suggesting that is because your comment would to
indicate that any warning is accurate as long as it isn't related to
a missing sequence.
The API for pg_get_sequence_data() is very _interesting_. Overloading
the NULLs row to mean many things is a bit strange. I'm not sure what
a better solution would be. Just thought I would mention that.
> While working on this, I also noticed that the documented privilege
> requirement for pg_get_sequence_data() does not match the
> implementation. The documentation says that USAGE or SELECT privilege
> is sufficient, but the implementation requires SELECT.
>
> Patch 0002 updates the documentation to match the current behavior.
>
> I chose to update the documentation rather than broaden the
> implementation for two reasons.
>
> First, commit c8b06bb969b, which introduced the predecessor of
> pg_get_sequence_data(), described it as a substitute for SELECT
> from a sequence, and its implementation has always required
> SELECT privilege.
>
> Second, the logical replication documentation already states that
> replicating sequence data requires SELECT privilege.
Your reasoning for updating the documentation makes sense, and the patch
you submitted achieves the stated goal.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-19 14:15 Fujii Masao <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-19 14:15 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Fri, Jun 19, 2026 at 8:11 AM Tristan Partin <[email protected]> wrote:
> The patch looks good to me! I had one suggestion:
>
> > ##########
> > # Ensure that insufficient privileges on the publisher for a sequence do not
> > -# disrupt the subscriber. The subscriber should log a warning and continue
> > -# retrying.
> > +# get misreported as a missing sequence. The subscriber should log a warning
> > +# and continue retrying.
> > ##########
>
> I think a better comment might be:
>
> Ensure that insufficient privileges on the publisher for a sequence
> are reported correctly...
>
> My reasoning for suggesting that is because your comment would to
> indicate that any warning is accurate as long as it isn't related to
> a missing sequence.
Thanks for the review! I've updated the patch as suggested.
Updated patches attached.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v2-0002-doc-Clarify-pg_get_sequence_data-privileges-and-N.patch (2.2K, ../../CAHGQGwE95GLD7RZr2g=zTHKJRafKACmKouhb4TTVH155Hy8zKA@mail.gmail.com/2-v2-0002-doc-Clarify-pg_get_sequence_data-privileges-and-N.patch)
download | inline diff:
From 02bcb728a8c6bfe21aab54ce15cb75266c42dfe3 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 18 Jun 2026 21:46:27 +0900
Subject: [PATCH v2 2/2] doc: Clarify pg_get_sequence_data() privileges and
NULL results
The documentation for pg_get_sequence_data() did not match the
function's behavior. It stated that either USAGE or SELECT privilege
was sufficient, but the function returns sequence data only when the
caller has SELECT privilege.
The documentation also did not explain that the function returns a row
containing all NULL values when sequence data cannot be returned, such
as when the sequence does not exist or the caller lacks the required
privilege.
Update the documentation to reflect the actual behavior, including the
required privilege and the result returned when sequence data is
unavailable.
Author: Fujii Masao <[email protected]>
Reviewed-by: Tristan Partin <[email protected]>
Discussion: https://postgr.es/m/CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com
---
doc/src/sgml/func/func-sequence.sgml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func/func-sequence.sgml b/doc/src/sgml/func/func-sequence.sgml
index 4a2a6dc9369..de266c36296 100644
--- a/doc/src/sgml/func/func-sequence.sgml
+++ b/doc/src/sgml/func/func-sequence.sgml
@@ -163,12 +163,13 @@ SELECT setval('myseq', 42, false); <lineannotation>Next <function>nextval</fu
<structfield>is_called</structfield> indicates whether the sequence has
been used. <structfield>page_lsn</structfield> is the LSN corresponding
to the most recent WAL record that modified this sequence relation.
+ This function returns a row of NULL values if the sequence does not
+ exist or if the current user lacks privileges on it.
</para>
<para>
This function is primarily intended for internal use by pg_dump and by
logical replication to synchronize sequences. It requires
- <literal>USAGE</literal> or <literal>SELECT</literal> privilege on the
- sequence.
+ <literal>SELECT</literal> privilege on the sequence.
</para></entry>
</row>
</tbody>
--
2.53.0
[application/octet-stream] v2-0001-Fix-misreporting-of-publisher-sequence-permission.patch (11.1K, ../../CAHGQGwE95GLD7RZr2g=zTHKJRafKACmKouhb4TTVH155Hy8zKA@mail.gmail.com/3-v2-0001-Fix-misreporting-of-publisher-sequence-permission.patch)
download | inline diff:
From 33dc5fd0c793d8b03abbd44a936aeca2b6c3456a Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 18 Jun 2026 21:47:24 +0900
Subject: [PATCH v2 1/2] Fix misreporting of publisher sequence permissions
during sync
When synchronizing sequences for logical replication, a
publisher-side permission failure could be reported as if the sequence
were missing on the publisher, making the real cause harder to
identify.
This happened because pg_get_sequence_data() returns a row of NULL
values when the replication connection lacks permission to read a
sequence. Sequence synchronization treated that the same as a missing
sequence, causing it to emit a misleading "missing sequence on
publisher" warning.
Fix this by distinguishing permission failures from genuinely missing
sequences. The synchronization query now checks whether the
replication connection has the required privilege for each published
sequence, allowing the worker to report permission failures
separately.
Author: Fujii Masao <[email protected]>
Reviewed-by: Tristan Partin <[email protected]>
Discussion: https://postgr.es/m/CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com
---
.../replication/logical/sequencesync.c | 95 +++++++++++++------
src/test/subscription/t/036_sequences.pl | 22 ++++-
2 files changed, 86 insertions(+), 31 deletions(-)
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index e2ff8d77b16..f47f962c7db 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -72,13 +72,14 @@
#include "utils/syscache.h"
#include "utils/usercontext.h"
-#define REMOTE_SEQ_COL_COUNT 10
+#define REMOTE_SEQ_COL_COUNT 11
typedef enum CopySeqResult
{
COPYSEQ_SUCCESS,
COPYSEQ_MISMATCH,
- COPYSEQ_INSUFFICIENT_PERM,
+ COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM,
+ COPYSEQ_PUBLISHER_INSUFFICIENT_PERM,
COPYSEQ_SKIPPED
} CopySeqResult;
@@ -166,18 +167,22 @@ get_sequences_string(List *seqindexes, StringInfo buf)
* Report discrepancies found during sequence synchronization between
* the publisher and subscriber. Emits warnings for:
* a) mismatched definitions or concurrent rename
- * b) insufficient privileges
- * c) missing sequences on the subscriber
+ * b) insufficient privileges on the subscriber
+ * c) insufficient privileges on the publisher
+ * d) missing sequences on the publisher
* Then raises an ERROR to indicate synchronization failure.
*/
static void
-report_sequence_errors(List *mismatched_seqs_idx, List *insuffperm_seqs_idx,
+report_sequence_errors(List *mismatched_seqs_idx,
+ List *sub_insuffperm_seqs_idx,
+ List *pub_insuffperm_seqs_idx,
List *missing_seqs_idx)
{
StringInfoData seqstr;
/* Quick exit if there are no errors to report */
- if (!mismatched_seqs_idx && !insuffperm_seqs_idx && !missing_seqs_idx)
+ if (!mismatched_seqs_idx && !sub_insuffperm_seqs_idx &&
+ !pub_insuffperm_seqs_idx && !missing_seqs_idx)
return;
initStringInfo(&seqstr);
@@ -193,14 +198,25 @@ report_sequence_errors(List *mismatched_seqs_idx, List *insuffperm_seqs_idx,
seqstr.data));
}
- if (insuffperm_seqs_idx)
+ if (sub_insuffperm_seqs_idx)
{
- get_sequences_string(insuffperm_seqs_idx, &seqstr);
+ get_sequences_string(sub_insuffperm_seqs_idx, &seqstr);
ereport(WARNING,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg_plural("insufficient privileges on sequence (%s)",
- "insufficient privileges on sequences (%s)",
- list_length(insuffperm_seqs_idx),
+ errmsg_plural("insufficient privileges on subscriber sequence (%s)",
+ "insufficient privileges on subscriber sequences (%s)",
+ list_length(sub_insuffperm_seqs_idx),
+ seqstr.data));
+ }
+
+ if (pub_insuffperm_seqs_idx)
+ {
+ get_sequences_string(pub_insuffperm_seqs_idx, &seqstr);
+ ereport(WARNING,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg_plural("insufficient privileges on publisher sequence (%s)",
+ "insufficient privileges on publisher sequences (%s)",
+ list_length(pub_insuffperm_seqs_idx),
seqstr.data));
}
@@ -235,6 +251,7 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
bool isnull;
int col = 0;
Datum datum;
+ bool remote_has_select_priv;
Oid remote_typid;
int64 remote_start;
int64 remote_increment;
@@ -254,12 +271,18 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
(LogicalRepSequenceInfo *) list_nth(seqinfos, *seqidx);
/*
- * The sequence data can be NULL due to insufficient privileges or if the
- * sequence was dropped concurrently (see pg_get_sequence_data()).
+ * The remote sequence state can be NULL if the publisher lacks the
+ * required privileges or if the sequence was dropped concurrently after
+ * it was identified in the catalog snapshot (see pg_get_sequence_data()).
*/
+ remote_has_select_priv = DatumGetBool(slot_getattr(slot, ++col, &isnull));
+ Assert(!isnull);
+
datum = slot_getattr(slot, ++col, &isnull);
if (isnull)
- return COPYSEQ_SKIPPED;
+ return remote_has_select_priv ? COPYSEQ_SKIPPED :
+ COPYSEQ_PUBLISHER_INSUFFICIENT_PERM;
+
seqinfo_local->last_value = DatumGetInt64(datum);
seqinfo_local->is_called = DatumGetBool(slot_getattr(slot, ++col, &isnull));
@@ -351,7 +374,7 @@ copy_sequence(LogicalRepSequenceInfo *seqinfo, Oid seqowner)
if (!run_as_owner)
RestoreUserContext(&ucxt);
- return COPYSEQ_INSUFFICIENT_PERM;
+ return COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM;
}
/*
@@ -387,7 +410,8 @@ copy_sequences(WalReceiverConn *conn)
int n_seqinfos = list_length(seqinfos);
List *mismatched_seqs_idx = NIL;
List *missing_seqs_idx = NIL;
- List *insuffperm_seqs_idx = NIL;
+ List *sub_insuffperm_seqs_idx = NIL;
+ List *pub_insuffperm_seqs_idx = NIL;
StringInfoData seqstr;
StringInfoData cmd;
MemoryContext oldctx;
@@ -403,13 +427,14 @@ copy_sequences(WalReceiverConn *conn)
while (cur_batch_base_index < n_seqinfos)
{
- Oid seqRow[REMOTE_SEQ_COL_COUNT] = {INT8OID, INT8OID,
+ Oid seqRow[REMOTE_SEQ_COL_COUNT] = {INT8OID, BOOLOID, INT8OID,
BOOLOID, LSNOID, OIDOID, INT8OID, INT8OID, INT8OID, INT8OID, BOOLOID};
int batch_size = 0;
int batch_succeeded_count = 0;
int batch_mismatched_count = 0;
int batch_skipped_count = 0;
- int batch_insuffperm_count = 0;
+ int batch_sub_insuffperm_count = 0;
+ int batch_pub_insuffperm_count = 0;
int batch_missing_count;
WalRcvExecResult *res;
@@ -471,7 +496,8 @@ copy_sequences(WalReceiverConn *conn)
* matching.
*/
appendStringInfo(&cmd,
- "SELECT s.seqidx, ps.*, seq.seqtypid,\n"
+ "SELECT s.seqidx, has_sequence_privilege(c.oid, 'SELECT'),\n"
+ " ps.*, seq.seqtypid,\n"
" seq.seqstart, seq.seqincrement, seq.seqmin,\n"
" seq.seqmax, seq.seqcycle\n"
"FROM ( VALUES %s ) AS s (schname, seqname, seqidx)\n"
@@ -532,7 +558,7 @@ copy_sequences(WalReceiverConn *conn)
MemoryContextSwitchTo(oldctx);
batch_mismatched_count++;
break;
- case COPYSEQ_INSUFFICIENT_PERM:
+ case COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM:
/*
* Remember sequences with insufficient privileges in a
@@ -540,10 +566,22 @@ copy_sequences(WalReceiverConn *conn)
* after the transaction is committed.
*/
oldctx = MemoryContextSwitchTo(ApplyContext);
- insuffperm_seqs_idx = lappend_int(insuffperm_seqs_idx,
- seqidx);
+ sub_insuffperm_seqs_idx = lappend_int(sub_insuffperm_seqs_idx,
+ seqidx);
+ MemoryContextSwitchTo(oldctx);
+ batch_sub_insuffperm_count++;
+ break;
+ case COPYSEQ_PUBLISHER_INSUFFICIENT_PERM:
+
+ /*
+ * Remember sequences for which the publisher lacks the
+ * privileges required by pg_get_sequence_data().
+ */
+ oldctx = MemoryContextSwitchTo(ApplyContext);
+ pub_insuffperm_seqs_idx = lappend_int(pub_insuffperm_seqs_idx,
+ seqidx);
MemoryContextSwitchTo(oldctx);
- batch_insuffperm_count++;
+ batch_pub_insuffperm_count++;
break;
case COPYSEQ_SKIPPED:
@@ -575,15 +613,16 @@ copy_sequences(WalReceiverConn *conn)
batch_missing_count = batch_size - (batch_succeeded_count +
batch_mismatched_count +
- batch_insuffperm_count +
+ batch_sub_insuffperm_count +
+ batch_pub_insuffperm_count +
batch_skipped_count);
elog(DEBUG1,
- "logical replication sequence synchronization for subscription \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d insufficient permission, %d missing from publisher, %d skipped",
+ "logical replication sequence synchronization for subscription \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d subscriber insufficient permission, %d publisher insufficient permission, %d missing from publisher, %d skipped",
MySubscription->name,
(cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
batch_size, batch_succeeded_count, batch_mismatched_count,
- batch_insuffperm_count, batch_missing_count, batch_skipped_count);
+ batch_sub_insuffperm_count, batch_pub_insuffperm_count, batch_missing_count, batch_skipped_count);
/* Commit this batch, and prepare for next batch */
CommitTransactionCommand();
@@ -610,8 +649,8 @@ copy_sequences(WalReceiverConn *conn)
}
/* Report mismatches, permission issues, or missing sequences */
- report_sequence_errors(mismatched_seqs_idx, insuffperm_seqs_idx,
- missing_seqs_idx);
+ report_sequence_errors(mismatched_seqs_idx, sub_insuffperm_seqs_idx,
+ pub_insuffperm_seqs_idx, missing_seqs_idx);
}
/*
diff --git a/src/test/subscription/t/036_sequences.pl b/src/test/subscription/t/036_sequences.pl
index 8e293871efb..2a0819aaf01 100644
--- a/src/test/subscription/t/036_sequences.pl
+++ b/src/test/subscription/t/036_sequences.pl
@@ -233,9 +233,9 @@ $node_publisher->safe_psql(
));
##########
-# Ensure that insufficient privileges on the publisher for a sequence do not
-# disrupt the subscriber. The subscriber should log a warning and continue
-# retrying.
+# Ensure that insufficient privileges on the publisher for a sequence
+# are reported correctly as a permission issue, not as a missing sequence.
+# The subscriber should log a warning and continue retrying.
##########
$node_publisher->safe_psql(
@@ -254,6 +254,22 @@ $node_subscriber->safe_psql('postgres',
"ALTER SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_limited_connstr'"
);
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+$node_subscriber->wait_for_log(
+ qr/WARNING: ( [A-Z0-9]+:)? insufficient privileges on publisher sequence \("public.regress_s2"\)/,
+ $log_offset);
+
+##########
+# Ensure that a sequence that is actually removed on the publisher is still
+# reported as missing.
+##########
+
+$node_publisher->safe_psql('postgres', qq(DROP SEQUENCE regress_s2;));
+
+$log_offset = -s $node_subscriber->logfile;
+
$node_subscriber->safe_psql('postgres',
"ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-19 16:38 Tristan Partin <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Tristan Partin @ 2026-06-19 16:38 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Fri Jun 19, 2026 at 9:16 AM CDT, Fujii Masao wrote:
> On Fri, Jun 19, 2026 at 8:11 AM Tristan Partin <[email protected]> wrote:
>> The patch looks good to me! I had one suggestion:
>>
>> > ##########
>> > # Ensure that insufficient privileges on the publisher for a sequence do not
>> > -# disrupt the subscriber. The subscriber should log a warning and continue
>> > -# retrying.
>> > +# get misreported as a missing sequence. The subscriber should log a warning
>> > +# and continue retrying.
>> > ##########
>>
>> I think a better comment might be:
>>
>> Ensure that insufficient privileges on the publisher for a sequence
>> are reported correctly...
>>
>> My reasoning for suggesting that is because your comment would to
>> indicate that any warning is accurate as long as it isn't related to
>> a missing sequence.
>
> Thanks for the review! I've updated the patch as suggested.
>
> Updated patches attached.
Looks great! Thanks for fixing this.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-20 09:24 Fujii Masao <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-20 09:24 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Sat, Jun 20, 2026 at 1:38 AM Tristan Partin <[email protected]> wrote:
> > Updated patches attached.
>
> Looks great! Thanks for fixing this.
Thanks for the review! I've pushed the patches.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-20 10:20 Amit Kapila <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Amit Kapila @ 2026-06-20 10:20 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, Jun 20, 2026 at 2:54 PM Fujii Masao <[email protected]> wrote:
>
> >
> > Looks great! Thanks for fixing this.
>
> Thanks for the review! I've pushed the patches.
>
You seem to have forgotten to update the following comment in
pg_get_sequence_data(): "Return all NULLs for missing sequences,
sequences for which we lack
privileges, other sessions' temporary sequences, ...". BTW, if we go
by the logic of your proposal, shouldn't one also distinguish other
cases as mentioned in the comment quoted by me?
Also, we might want to consider additional errhint as follows:
errhint("Grant UPDATE on the sequence to the subscription/sequence
owner on the subscriber.")
errhint("Grant SELECT on the sequence to the replication role on the
publisher.")
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-20 12:43 Fujii Masao <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-20 12:43 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, Jun 20, 2026 at 7:21 PM Amit Kapila <[email protected]> wrote:
>
> On Sat, Jun 20, 2026 at 2:54 PM Fujii Masao <[email protected]> wrote:
> >
> > >
> > > Looks great! Thanks for fixing this.
> >
> > Thanks for the review! I've pushed the patches.
> >
>
> You seem to have forgotten to update the following comment in
> pg_get_sequence_data(): "Return all NULLs for missing sequences,
> sequences for which we lack
> privileges, other sessions' temporary sequences, ...".
Do you mean that the documentation for pg_get_sequence_data() should
also mention other sessions' temporary sequences and unlogged sequences
on standbys, as the comment does? If I've misunderstood your point,
could you clarify?
> BTW, if we go
> by the logic of your proposal, shouldn't one also distinguish other
> cases as mentioned in the comment quoted by me?
Do you mean that sequencesync.c should also distinguish other
sessions' temporary sequences and unlogged sequences on standbys, and
report separate warnings for those cases?
> Also, we might want to consider additional errhint as follows:
Sounds good.
> errhint("Grant UPDATE on the sequence to the subscription/sequence
> owner on the subscriber.")
Wouldn't it be better to drop "sequence" from "subscription/sequence
owner"? The sequence owner should already have UPDATE privilege on
the sequence. How about:
errhint("Grant UPDATE on the sequence to the subscription owner on
the subscriber.")
> errhint("Grant SELECT on the sequence to the replication role on the
> publisher.")
How about making this a bit more precise?
errhint("Grant SELECT on the sequence to the role used for the
replication connection on the publisher.")
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-22 04:03 Amit Kapila <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Amit Kapila @ 2026-06-22 04:03 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, Jun 20, 2026 at 6:14 PM Fujii Masao <[email protected]> wrote:
>
> On Sat, Jun 20, 2026 at 7:21 PM Amit Kapila <[email protected]> wrote:
> >
> > On Sat, Jun 20, 2026 at 2:54 PM Fujii Masao <[email protected]> wrote:
> > >
> > > >
> > > > Looks great! Thanks for fixing this.
> > >
> > > Thanks for the review! I've pushed the patches.
> > >
> >
> > You seem to have forgotten to update the following comment in
> > pg_get_sequence_data(): "Return all NULLs for missing sequences,
> > sequences for which we lack
> > privileges, other sessions' temporary sequences, ...".
>
> Do you mean that the documentation for pg_get_sequence_data() should
> also mention other sessions' temporary sequences and unlogged sequences
> on standbys, as the comment does? If I've misunderstood your point,
> could you clarify?
>
It is better to update docs for all cases. Sorry, I was wrong in
saying that code comments need an update.
>
> > BTW, if we go
> > by the logic of your proposal, shouldn't one also distinguish other
> > cases as mentioned in the comment quoted by me?
>
> Do you mean that sequencesync.c should also distinguish other
> sessions' temporary sequences and unlogged sequences on standbys, and
> report separate warnings for those cases?
>
Right. I mean to ask if we want to distinguish the lack of privilege
as a separate case then why not others? My opinion on this point is
that improving all these cases (including lack of privileges) together
could be considered as an enhancement for the next version but if you
think this is sort of a must to distinguish one or more cases then we
can do it now as well. However, the reason for doing it now is not
clear to me.
>
> > Also, we might want to consider additional errhint as follows:
>
> Sounds good.
>
>
> > errhint("Grant UPDATE on the sequence to the subscription/sequence
> > owner on the subscriber.")
>
> Wouldn't it be better to drop "sequence" from "subscription/sequence
> owner"? The sequence owner should already have UPDATE privilege on
> the sequence. How about:
>
> errhint("Grant UPDATE on the sequence to the subscription owner on
> the subscriber.")
>
makes sense.
>
> > errhint("Grant SELECT on the sequence to the replication role on the
> > publisher.")
>
> How about making this a bit more precise?
>
> errhint("Grant SELECT on the sequence to the role used for the
> replication connection on the publisher.")
>
makes sense.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 00:41 Fujii Masao <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 2 replies; 20+ messages in thread
From: Fujii Masao @ 2026-06-24 00:41 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Jun 22, 2026 at 1:03 PM Amit Kapila <[email protected]> wrote:
> > Do you mean that the documentation for pg_get_sequence_data() should
> > also mention other sessions' temporary sequences and unlogged sequences
> > on standbys, as the comment does? If I've misunderstood your point,
> > could you clarify?
> >
>
> It is better to update docs for all cases. Sorry, I was wrong in
> saying that code comments need an update.
Understood.
BTW, isn't the current documentation a bit misleading? It says:
This function returns a row of NULL values if the sequence does not exist.
But if the specified object does not exist, pg_get_sequence_data()
raises an error rather than returning a row of NULL values. On the
other hand, it does return a row of NULL values if the specified
object exists but is not a sequence. Is my understanding correct? If
so, how about something like:
---------------------
This function returns a row of NULL values if the specified object
exists but is not a sequence, if the current user lacks privileges on
the sequence, if the sequence is another session's temporary
sequence, or if it is an unlogged sequence on a standby server.
---------------------
> > Do you mean that sequencesync.c should also distinguish other
> > sessions' temporary sequences and unlogged sequences on standbys, and
> > report separate warnings for those cases?
> >
>
> Right. I mean to ask if we want to distinguish the lack of privilege
> as a separate case then why not others? My opinion on this point is
> that improving all these cases (including lack of privileges) together
> could be considered as an enhancement for the next version but if you
> think this is sort of a must to distinguish one or more cases then we
> can do it now as well. However, the reason for doing it now is not
> clear to me.
I think the lack-of-privilege case should be checked first, since it is
likely to be the most common one.
As for another session's temporary sequences, I don't think a sequence
sync worker can actually encounter one. To do so, it would have to
specify another session's temporary namespace when executing the query
below. However, the namespace comes from the publication, and temporary
sequences are never published, so that doesn't seem possible.
appendStringInfo(&cmd,
"SELECT s.seqidx, has_sequence_privilege(c.oid, 'SELECT'),\n"
" ps.*, seq.seqtypid,\n"
" seq.seqstart, seq.seqincrement, seq.seqmin,\n"
" seq.seqmax, seq.seqcycle\n"
"FROM ( VALUES %s ) AS s (schname, seqname, seqidx)\n"
"JOIN pg_namespace n ON n.nspname = s.schname\n"
"JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = s.seqname\n"
"JOIN pg_sequence seq ON seq.seqrelid = c.oid\n"
"JOIN LATERAL pg_get_sequence_data(seq.seqrelid) AS ps ON true\n",
seqstr.data);
An unlogged sequence on a standby seems theoretically possible, but
only under unlikely sequence of events:
1. The sequence sync worker fetches the sequence information from the
publication.
2. The sequence is dropped on the primary.
3. An unlogged sequence with the same name is created.
4. The schema change is replicated to the standby.
5. The sequence sync worker executes the above query with the fetched info.
So I think this case is possible, but unlikely.
> > > errhint("Grant SELECT on the sequence to the replication role on the
> > > publisher.")
> >
> > How about making this a bit more precise?
> >
> > errhint("Grant SELECT on the sequence to the role used for the
> > replication connection on the publisher.")
> >
>
> makes sense.
The attached patch adds those HINT messages. It also updates the
related documentation.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v1-0001-Add-hints-for-sequence-sync-permission-warnings.patch (3.3K, ../../CAHGQGwHuuffj_js4n2g2M07vpkbun8Aj_4+o4ALK=rQpBfwk7w@mail.gmail.com/2-v1-0001-Add-hints-for-sequence-sync-permission-warnings.patch)
download | inline diff:
From 9910860770045cf8ee365cfeaaa90561f5b1e43f Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 24 Jun 2026 08:14:36 +0900
Subject: [PATCH v1] Add hints for sequence sync permission warnings
---
doc/src/sgml/logical-replication.sgml | 12 ++++++++----
src/backend/replication/logical/sequencesync.c | 7 +++++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 9e7868487de..f0c25a75460 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2542,9 +2542,10 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
</para>
<para>
- In order to be able to copy the initial table or sequence data, the role
- used for the replication connection must have the <literal>SELECT</literal>
- privilege on a published table or sequence (or be a superuser).
+ In order to be able to copy the initial table data or synchronize
+ sequences, the role used for the replication connection must have the
+ <literal>SELECT</literal> privilege on a published table or sequence (or be
+ a superuser).
</para>
<para>
@@ -2602,7 +2603,10 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
needs privileges to <literal>SELECT</literal>, <literal>INSERT</literal>,
<literal>UPDATE</literal>, and <literal>DELETE</literal> from the
target table, and does not need privileges to <literal>SET ROLE</literal>
- to the table owner. However, this also means that any user who owns
+ to the table owner. When synchronizing sequences, the subscription owner
+ similarly needs <literal>UPDATE</literal> privilege on the target sequence
+ and does not need privileges to <literal>SET ROLE</literal> to the sequence
+ owner. However, this also means that any user who owns
a table into which replication is happening can execute arbitrary code with
the privileges of the subscription owner. For example, they could do this
by simply attaching a trigger to one of the tables which they own.
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index f47f962c7db..7910d610c60 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -206,7 +206,9 @@ report_sequence_errors(List *mismatched_seqs_idx,
errmsg_plural("insufficient privileges on subscriber sequence (%s)",
"insufficient privileges on subscriber sequences (%s)",
list_length(sub_insuffperm_seqs_idx),
- seqstr.data));
+ seqstr.data),
+ MySubscription->runasowner ?
+ errhint("Grant UPDATE on the sequence to the subscription owner on the subscriber.") : 0);
}
if (pub_insuffperm_seqs_idx)
@@ -217,7 +219,8 @@ report_sequence_errors(List *mismatched_seqs_idx,
errmsg_plural("insufficient privileges on publisher sequence (%s)",
"insufficient privileges on publisher sequences (%s)",
list_length(pub_insuffperm_seqs_idx),
- seqstr.data));
+ seqstr.data),
+ errhint("Grant SELECT on the sequence to the role used for the replication connection on the publisher."));
}
if (missing_seqs_idx)
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 05:40 Bharath Rupireddy <[email protected]>
parent: Fujii Masao <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Bharath Rupireddy @ 2026-06-24 05:40 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Tue, Jun 23, 2026 at 5:41 PM Fujii Masao <[email protected]> wrote:
>
> BTW, isn't the current documentation a bit misleading? It says:
>
> This function returns a row of NULL values if the sequence does not exist.
>
> But if the specified object does not exist, pg_get_sequence_data()
> raises an error rather than returning a row of NULL values. On the
> other hand, it does return a row of NULL values if the specified
> object exists but is not a sequence. Is my understanding correct? If
> so, how about something like:
When the provided relation oid doesn't exist, returns NULL:
postgres=# select pg_get_sequence_data(99999999);
pg_get_sequence_data
----------------------
(,,)
(1 row)
When the provided relation oid (pg_statistic) exists but not a
sequence, returns NULL:
postgres=# select pg_get_sequence_data(2619);
pg_get_sequence_data
----------------------
(,,)
(1 row)
When the provided relation oid exists and is a sequence, returns sequence data:
postgres=# select pg_get_sequence_data(16388);
pg_get_sequence_data
----------------------
(1,f,0/017DD9A0)
(1 row)
> > > Do you mean that sequencesync.c should also distinguish other
> > > sessions' temporary sequences and unlogged sequences on standbys, and
> > > report separate warnings for those cases?
> > >
> >
> > Right. I mean to ask if we want to distinguish the lack of privilege
> > as a separate case then why not others? My opinion on this point is
> > that improving all these cases (including lack of privileges) together
> > could be considered as an enhancement for the next version but if you
> > think this is sort of a must to distinguish one or more cases then we
> > can do it now as well. However, the reason for doing it now is not
> > clear to me.
>
> I think the lack-of-privilege case should be checked first, since it is
> likely to be the most common one.
>
> As for another session's temporary sequences, I don't think a sequence
> sync worker can actually encounter one. To do so, it would have to
> specify another session's temporary namespace when executing the query
> below. However, the namespace comes from the publication, and temporary
> sequences are never published, so that doesn't seem possible.
Ideally, all these checks (except seqrel being NULL when
try_relation_open detects a concurrent drop) should report an ERROR if
not met, instead of silently emitting NULLs, so the subscriber catches
these early and reports "could not fetch sequence information from the
publisher:" But I understand that on the subscriber side it would
require error message parsing to distinguish the exact cause, which is
not ideal.
However, instead of pg_get_sequence_data emitting just 3 columns, it
could emit additional columns such as:
"last_value", "is_called", "page_lsn", "has_privileges",
"is_sequence", "is_temp_relation", "is_recovery_in_progress"
When any of the new flag columns are true, the first three values
would be NULLs.
This keeps the subscriber-side query simple and leaves the existing
error-handling code as-is.
> > > > errhint("Grant SELECT on the sequence to the replication role on the
> > > > publisher.")
> > >
> > > How about making this a bit more precise?
> > >
> > > errhint("Grant SELECT on the sequence to the role used for the
> > > replication connection on the publisher.")
> > >
> >
> > makes sense.
>
> The attached patch adds those HINT messages. It also updates the
> related documentation.
Nice! This looks more explicit and clarifying. The patch LGTM.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 07:40 Amit Kapila <[email protected]>
parent: Fujii Masao <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Amit Kapila @ 2026-06-24 07:40 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 24, 2026 at 6:11 AM Fujii Masao <[email protected]> wrote:
>
> On Mon, Jun 22, 2026 at 1:03 PM Amit Kapila <[email protected]> wrote:
> > > Do you mean that the documentation for pg_get_sequence_data() should
> > > also mention other sessions' temporary sequences and unlogged sequences
> > > on standbys, as the comment does? If I've misunderstood your point,
> > > could you clarify?
> > >
> >
> > It is better to update docs for all cases. Sorry, I was wrong in
> > saying that code comments need an update.
>
> Understood.
>
> BTW, isn't the current documentation a bit misleading? It says:
>
> This function returns a row of NULL values if the sequence does not exist.
>
> But if the specified object does not exist, pg_get_sequence_data()
> raises an error rather than returning a row of NULL values. On the
> other hand, it does return a row of NULL values if the specified
> object exists but is not a sequence. Is my understanding correct? If
> so, how about something like:
>
> ---------------------
> This function returns a row of NULL values if the specified object
> exists but is not a sequence, if the current user lacks privileges on
> the sequence, if the sequence is another session's temporary
> sequence, or if it is an unlogged sequence on a standby server.
> ---------------------
>
>
> > > Do you mean that sequencesync.c should also distinguish other
> > > sessions' temporary sequences and unlogged sequences on standbys, and
> > > report separate warnings for those cases?
> > >
> >
> > Right. I mean to ask if we want to distinguish the lack of privilege
> > as a separate case then why not others? My opinion on this point is
> > that improving all these cases (including lack of privileges) together
> > could be considered as an enhancement for the next version but if you
> > think this is sort of a must to distinguish one or more cases then we
> > can do it now as well. However, the reason for doing it now is not
> > clear to me.
>
> I think the lack-of-privilege case should be checked first, since it is
> likely to be the most common one.
>
> As for another session's temporary sequences, I don't think a sequence
> sync worker can actually encounter one. To do so, it would have to
> specify another session's temporary namespace when executing the query
> below. However, the namespace comes from the publication, and temporary
> sequences are never published, so that doesn't seem possible.
>
> appendStringInfo(&cmd,
> "SELECT s.seqidx, has_sequence_privilege(c.oid, 'SELECT'),\n"
> " ps.*, seq.seqtypid,\n"
> " seq.seqstart, seq.seqincrement, seq.seqmin,\n"
> " seq.seqmax, seq.seqcycle\n"
> "FROM ( VALUES %s ) AS s (schname, seqname, seqidx)\n"
> "JOIN pg_namespace n ON n.nspname = s.schname\n"
> "JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = s.seqname\n"
> "JOIN pg_sequence seq ON seq.seqrelid = c.oid\n"
> "JOIN LATERAL pg_get_sequence_data(seq.seqrelid) AS ps ON true\n",
> seqstr.data);
>
> An unlogged sequence on a standby seems theoretically possible, but
> only under unlikely sequence of events:
>
> 1. The sequence sync worker fetches the sequence information from the
> publication.
>
Assume a case where the primary fails and the system promotes standby
as a new primary. Then the subscriber starts sync from the new
primary, there it can lead to an unlogged sequence sync scenario?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 08:50 Fujii Masao <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-24 08:50 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 24, 2026 at 4:40 PM Amit Kapila <[email protected]> wrote:
> Assume a case where the primary fails and the system promotes standby
> as a new primary. Then the subscriber starts sync from the new
> primary, there it can lead to an unlogged sequence sync scenario?
When I tested pg_get_sequence_data() with an unlogged sequence on
new primary after promotion, I hit an assertion failure...
---------------------
1. Create an unlogged sequence on the primary:
=# CREATE UNLOGGED SEQUENCE myseq;
CREATE SEQUENCE
2. Confirm that pg_get_sequence_data() returns NULL values on the standby:
=# SELECT * FROM pg_get_sequence_data('myseq');
last_value | is_called | page_lsn
------------+-----------+----------
(null) | (null) | (null)
(1 row)
3. Promote the standby, then call pg_get_sequence_data() on the new primary:
=# SELECT * FROM pg_get_sequence_data('myseq');
This results in the following assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >=
SizeOfPageHeaderData"), File:
"../../../src/include/storage/bufpage.h", Line: 357, PID: 96253
0 postgres 0x000000010d8d1412
ExceptionalCondition + 178
1 postgres 0x000000010d316970
PageValidateSpecialPointer + 128
2 postgres 0x000000010d3142df read_seq_tuple + 79
3 postgres 0x000000010d31635e
pg_get_sequence_data + 382
4 postgres 0x000000010d3bc42e
ExecMakeTableFunctionResult + 702
5 postgres 0x000000010d3da6cf FunctionNext + 175
6 postgres 0x000000010d3bde92 ExecScanFetch + 626
7 postgres 0x000000010d3bd88f ExecScanExtended + 95
8 postgres 0x000000010d3bd81f ExecScan + 95
9 postgres 0x000000010d3da2c5 ExecFunctionScan + 53
10 postgres 0x000000010d3b8a2b
ExecProcNodeFirst + 75
11 postgres 0x000000010d3affee ExecProcNode + 46
12 postgres 0x000000010d3ab507 ExecutePlan + 199
13 postgres 0x000000010d3ab3e0
standard_ExecutorRun + 368
14 postgres 0x000000010d3ab263 ExecutorRun + 67
15 postgres 0x000000010d6c2710 PortalRunSelect + 256
16 postgres 0x000000010d6c2170 PortalRun + 672
17 postgres 0x000000010d6bd53c
exec_simple_query + 1292
18 postgres 0x000000010d6bc6e5 PostgresMain + 2981
19 postgres 0x000000010d6b5228 BackendMain + 168
20 postgres 0x000000010d5a2269
postmaster_child_launch + 377
21 postgres 0x000000010d5a8775 BackendStartup + 277
22 postgres 0x000000010d5a6d85 ServerLoop + 341
23 postgres 0x000000010d5a5b84 PostmasterMain + 5748
24 postgres 0x000000010d43e533 main + 771
25 dyld 0x00007ff80fc71530 start + 3056
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 10:30 Fujii Masao <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-24 10:30 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 24, 2026 at 5:50 PM Fujii Masao <[email protected]> wrote:
>
> On Wed, Jun 24, 2026 at 4:40 PM Amit Kapila <[email protected]> wrote:
> > Assume a case where the primary fails and the system promotes standby
> > as a new primary. Then the subscriber starts sync from the new
> > primary, there it can lead to an unlogged sequence sync scenario?
>
> When I tested pg_get_sequence_data() with an unlogged sequence on
> new primary after promotion, I hit an assertion failure...
The assertion failure seems to be caused by seq_redo() not flushing
the init fork buffer from shared buffers. As a result, the init fork of
an unlogged sequence can remain invalid. During promotion,
ResetUnloggedRelations() creates the main fork by copying the init
fork from disk, so the main fork also becomes invalid. When
pg_get_sequence_data() later reads the invalid page, it hits the
assertion failure.
The attached patch adds a common function to flush an init fork buffer
and updates seq_redo() to use it. It also updates hash_xlog.c to
reuse the same function to simplify the code.
Thought?
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v1-0001-Fix-promoted-standby-reads-of-unlogged-sequences.patch (5.7K, ../../CAHGQGwEcGpnHWw1uwsWkgVwKv5cwysKfir2yXDQnN1aVOAhCdQ@mail.gmail.com/2-v1-0001-Fix-promoted-standby-reads-of-unlogged-sequences.patch)
download | inline diff:
From b91ca1dfef3b6b7f4fbbe87ddd86d1f2939ef7f8 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 24 Jun 2026 18:42:43 +0900
Subject: [PATCH v1] Fix promoted-standby reads of unlogged sequences
---
src/backend/access/hash/hash_xlog.c | 29 +++-----------------------
src/backend/access/transam/xlogutils.c | 26 ++++++++++++++++++++++-
src/backend/commands/sequence_xlog.c | 1 +
src/include/access/xlogutils.h | 2 ++
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 2060620c7de..e9a2b9aa9a7 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -29,7 +29,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -41,16 +40,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -68,7 +58,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -79,16 +68,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -109,10 +89,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fdc341d8fa4..d8c179c5dcc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -321,6 +321,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -398,7 +420,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence_xlog.c b/src/backend/commands/sequence_xlog.c
index d0aed48e268..fcb3230cf3b 100644
--- a/src/backend/commands/sequence_xlog.c
+++ b/src/backend/commands/sequence_xlog.c
@@ -63,6 +63,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index b97387c6d4c..0c6c7410069 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -87,6 +87,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-24 10:49 vignesh C <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: vignesh C @ 2026-06-24 10:49 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, 24 Jun 2026 at 16:00, Fujii Masao <[email protected]> wrote:
>
> On Wed, Jun 24, 2026 at 5:50 PM Fujii Masao <[email protected]> wrote:
> >
> > On Wed, Jun 24, 2026 at 4:40 PM Amit Kapila <[email protected]> wrote:
> > > Assume a case where the primary fails and the system promotes standby
> > > as a new primary. Then the subscriber starts sync from the new
> > > primary, there it can lead to an unlogged sequence sync scenario?
> >
> > When I tested pg_get_sequence_data() with an unlogged sequence on
> > new primary after promotion, I hit an assertion failure...
>
> The assertion failure seems to be caused by seq_redo() not flushing
> the init fork buffer from shared buffers. As a result, the init fork of
> an unlogged sequence can remain invalid. During promotion,
> ResetUnloggedRelations() creates the main fork by copying the init
> fork from disk, so the main fork also becomes invalid. When
> pg_get_sequence_data() later reads the invalid page, it hits the
> assertion failure.
>
> The attached patch adds a common function to flush an init fork buffer
> and updates seq_redo() to use it. It also updates hash_xlog.c to
> reuse the same function to simplify the code.
>
> Thought?
Thanks for the patch. I verified that it fixes the issue with reading
unlogged sequences on a promoted standby.
Do you think it would be worthwhile to add a test for this scenario,
or do you feel the additional test is not necessary in this case?
Regards,
Vignesh
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-26 07:04 Fujii Masao <[email protected]>
parent: vignesh C <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-06-26 07:04 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 24, 2026 at 7:49 PM vignesh C <[email protected]> wrote:
> Thanks for the patch. I verified that it fixes the issue with reading
> unlogged sequences on a promoted standby.
Thanks for testing!
> Do you think it would be worthwhile to add a test for this scenario,
> or do you feel the additional test is not necessary in this case?
I think it's worth adding a test for this scenario, so I've added one to
the patch. The test uses nextval() to read the unlogged sequence
instead of pg_get_sequence_data(), since this patch needs to be
backpatched to v15, while pg_get_sequence_data() was introduced in
v19.
Attached are updated patches for master and the stable branches.
Regards,
--
Fujii Masao
From 653b98b2418abaf015168955392a550401f519cf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++-------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
.../t/054_unlogged_sequence_promotion.pl | 36 +++++++++++++++++++
5 files changed, 67 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 6fe1c2f7951..e541ce81ce6 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -32,7 +32,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -44,16 +43,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -71,7 +61,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -82,16 +71,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -112,10 +92,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 702c8c14e12..72844504b64 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -334,6 +334,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -411,7 +433,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index fbcfcddb59e..9ef47b64274 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1908,6 +1908,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index c9d0b75a01b..e91d1d99ec5 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -84,6 +84,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 buffer_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 buffer_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..7e030c60eee
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,36 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_catchup($node_standby);
+
+$node_standby->promote;
+$node_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "Timed out waiting for promotion";
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
From dc7b84386209c745427d98586b7f100b6a8e122c Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++--------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
src/test/recovery/meson.build | 1 +
.../t/054_unlogged_sequence_promotion.pl | 34 +++++++++++++++++++
6 files changed, 66 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 8d97067fe54..d4cb6246b48 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -29,7 +29,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -41,16 +40,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -68,7 +58,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -79,16 +68,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -109,10 +89,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index db5a314edf8..0d67f256afe 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -321,6 +321,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -398,7 +420,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index a79ef0651a9..c1ad656397a 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1933,6 +1933,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index a1870d8e5aa..7639bd523e1 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -87,6 +87,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 5245fdde43c..38e1e43e041 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -58,6 +58,7 @@ tests += {
't/047_checkpoint_physical_slot.pl',
't/048_vacuum_horizon_floor.pl',
't/053_standby_login_event_trigger.pl',
+ 't/054_unlogged_sequence_promotion.pl',
],
},
}
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..96d1e4bf18b
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_replay_catchup($node_standby);
+
+$node_standby->promote;
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
From 90b5a189df90c263648f66bbc3edc7ce5dc12f9a Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++--------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
src/test/recovery/meson.build | 1 +
.../t/054_unlogged_sequence_promotion.pl | 34 +++++++++++++++++++
6 files changed, 66 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index e8e06c62a95..cd9617533c2 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -32,7 +32,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -44,16 +43,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -71,7 +61,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -82,16 +71,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -112,10 +92,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index d63364fd506..2352dcd607e 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -335,6 +335,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -412,7 +434,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index e0af32075d1..5eda26df455 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1893,6 +1893,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5b77b11f508..d0545248abf 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -84,6 +84,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 42059801ce2..5453e1aa6fa 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -47,6 +47,7 @@ tests += {
't/043_vacuum_horizon_floor.pl',
't/043_no_contrecord_switch.pl',
't/045_archive_restartpoint.pl',
+ 't/054_unlogged_sequence_promotion.pl',
],
},
}
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..96d1e4bf18b
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_replay_catchup($node_standby);
+
+$node_standby->promote;
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
Attachments:
[text/plain] v2-v15-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt (8.3K, ../../CAHGQGwE9AOvhqPOx7J=zPug5s90FhNVPH8m0m3AVJVzjG8npBQ@mail.gmail.com/2-v2-v15-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt)
download | inline diff:
From 653b98b2418abaf015168955392a550401f519cf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++-------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
.../t/054_unlogged_sequence_promotion.pl | 36 +++++++++++++++++++
5 files changed, 67 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 6fe1c2f7951..e541ce81ce6 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -32,7 +32,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -44,16 +43,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -71,7 +61,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -82,16 +71,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -112,10 +92,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 702c8c14e12..72844504b64 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -334,6 +334,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -411,7 +433,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index fbcfcddb59e..9ef47b64274 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1908,6 +1908,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index c9d0b75a01b..e91d1d99ec5 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -84,6 +84,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 buffer_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 buffer_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..7e030c60eee
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,36 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_catchup($node_standby);
+
+$node_standby->promote;
+$node_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "Timed out waiting for promotion";
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
[application/octet-stream] v2-0001-Fix-unlogged-sequence-corruption-after-standby-pr.patch (8.7K, ../../CAHGQGwE9AOvhqPOx7J=zPug5s90FhNVPH8m0m3AVJVzjG8npBQ@mail.gmail.com/3-v2-0001-Fix-unlogged-sequence-corruption-after-standby-pr.patch)
download | inline diff:
From b1020bfead7a84c6f84ef0bbb220921720dafa75 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++--------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence_xlog.c | 1 +
src/include/access/xlogutils.h | 2 ++
src/test/recovery/meson.build | 1 +
.../t/054_unlogged_sequence_promotion.pl | 34 +++++++++++++++++++
6 files changed, 66 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 2060620c7de..e9a2b9aa9a7 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -29,7 +29,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -41,16 +40,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -68,7 +58,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -79,16 +68,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -109,10 +89,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fdc341d8fa4..d8c179c5dcc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -321,6 +321,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -398,7 +420,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence_xlog.c b/src/backend/commands/sequence_xlog.c
index d0aed48e268..fcb3230cf3b 100644
--- a/src/backend/commands/sequence_xlog.c
+++ b/src/backend/commands/sequence_xlog.c
@@ -63,6 +63,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index b97387c6d4c..0c6c7410069 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -87,6 +87,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 9eb8ed11425..ad0d85f4189 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -62,6 +62,7 @@ tests += {
't/051_effective_wal_level.pl',
't/052_checkpoint_segment_missing.pl',
't/053_standby_login_event_trigger.pl',
+ 't/054_unlogged_sequence_promotion.pl',
],
},
}
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..96d1e4bf18b
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_replay_catchup($node_standby);
+
+$node_standby->promote;
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
[text/plain] v2-v18-v17-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt (8.7K, ../../CAHGQGwE9AOvhqPOx7J=zPug5s90FhNVPH8m0m3AVJVzjG8npBQ@mail.gmail.com/4-v2-v18-v17-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt)
download | inline diff:
From dc7b84386209c745427d98586b7f100b6a8e122c Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++--------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
src/test/recovery/meson.build | 1 +
.../t/054_unlogged_sequence_promotion.pl | 34 +++++++++++++++++++
6 files changed, 66 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 8d97067fe54..d4cb6246b48 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -29,7 +29,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -41,16 +40,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -68,7 +58,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -79,16 +68,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -109,10 +89,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index db5a314edf8..0d67f256afe 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -321,6 +321,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -398,7 +420,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index a79ef0651a9..c1ad656397a 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1933,6 +1933,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index a1870d8e5aa..7639bd523e1 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -87,6 +87,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 5245fdde43c..38e1e43e041 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -58,6 +58,7 @@ tests += {
't/047_checkpoint_physical_slot.pl',
't/048_vacuum_horizon_floor.pl',
't/053_standby_login_event_trigger.pl',
+ 't/054_unlogged_sequence_promotion.pl',
],
},
}
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..96d1e4bf18b
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_replay_catchup($node_standby);
+
+$node_standby->promote;
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
[text/plain] v2-v16-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt (8.7K, ../../CAHGQGwE9AOvhqPOx7J=zPug5s90FhNVPH8m0m3AVJVzjG8npBQ@mail.gmail.com/5-v2-v16-0001-Fix-unlogged-sequence-corruption-after-standby-pr.txt)
download | inline diff:
From 90b5a189df90c263648f66bbc3edc7ce5dc12f9a Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 26 Jun 2026 11:53:24 +0900
Subject: [PATCH v2] Fix unlogged sequence corruption after standby promotion
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
---
src/backend/access/hash/hash_xlog.c | 29 ++--------------
src/backend/access/transam/xlogutils.c | 26 +++++++++++++-
src/backend/commands/sequence.c | 1 +
src/include/access/xlogutils.h | 2 ++
src/test/recovery/meson.build | 1 +
.../t/054_unlogged_sequence_promotion.pl | 34 +++++++++++++++++++
6 files changed, 66 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/054_unlogged_sequence_promotion.pl
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index e8e06c62a95..cd9617533c2 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -32,7 +32,6 @@ hash_xlog_init_meta_page(XLogReaderState *record)
XLogRecPtr lsn = record->EndRecPtr;
Page page;
Buffer metabuf;
- ForkNumber forknum;
xl_hash_init_meta_page *xlrec = (xl_hash_init_meta_page *) XLogRecGetData(record);
@@ -44,16 +43,7 @@ hash_xlog_init_meta_page(XLogReaderState *record)
page = (Page) BufferGetPage(metabuf);
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 0, metabuf);
/* all done */
UnlockReleaseBuffer(metabuf);
@@ -71,7 +61,6 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
Page page;
HashMetaPage metap;
uint32 num_buckets;
- ForkNumber forknum;
xl_hash_init_bitmap_page *xlrec = (xl_hash_init_bitmap_page *) XLogRecGetData(record);
@@ -82,16 +71,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
_hash_initbitmapbuffer(bitmapbuf, xlrec->bmsize, true);
PageSetLSN(BufferGetPage(bitmapbuf), lsn);
MarkBufferDirty(bitmapbuf);
-
- /*
- * Force the on-disk state of init forks to always be in sync with the
- * state in shared buffers. See XLogReadBufferForRedoExtended. We need
- * special handling for init forks as create index operations don't log a
- * full page image of the metapage.
- */
- XLogRecGetBlockTag(record, 0, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(bitmapbuf);
+ XLogFlushBufferForRedoIfInit(record, 0, bitmapbuf);
UnlockReleaseBuffer(bitmapbuf);
/* add the new bitmap page to the metapage's list of bitmaps */
@@ -112,10 +92,7 @@ hash_xlog_init_bitmap_page(XLogReaderState *record)
PageSetLSN(page, lsn);
MarkBufferDirty(metabuf);
-
- XLogRecGetBlockTag(record, 1, NULL, &forknum, NULL);
- if (forknum == INIT_FORKNUM)
- FlushOneBuffer(metabuf);
+ XLogFlushBufferForRedoIfInit(record, 1, metabuf);
}
if (BufferIsValid(metabuf))
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index d63364fd506..2352dcd607e 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -335,6 +335,28 @@ XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id)
return buf;
}
+/*
+ * If a redo routine modified an init fork, flush the buffer immediately.
+ *
+ * At the end of crash recovery the init forks of unlogged relations are
+ * copied to the main fork directly from disk, without going through shared
+ * buffers. Therefore, redo routines that update init forks without
+ * restoring a full-page image must call this after setting the page LSN and
+ * marking the buffer dirty.
+ */
+void
+XLogFlushBufferForRedoIfInit(XLogReaderState *record, uint8 block_id,
+ Buffer buffer)
+{
+ ForkNumber forknum;
+
+ Assert(BufferIsValid(buffer));
+
+ XLogRecGetBlockTag(record, block_id, NULL, &forknum, NULL);
+ if (forknum == INIT_FORKNUM)
+ FlushOneBuffer(buffer);
+}
+
/*
* XLogReadBufferForRedoExtended
* Like XLogReadBufferForRedo, but with extra options.
@@ -412,7 +434,9 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
* At the end of crash recovery the init forks of unlogged relations
* are copied, without going through shared buffers. So we need to
* force the on-disk state of init forks to always be in sync with the
- * state in shared buffers.
+ * state in shared buffers. Use XLogFlushBufferForRedoIfInit() for
+ * redo routines that dirty init-fork buffers without restoring a
+ * full-page image.
*/
if (forknum == INIT_FORKNUM)
FlushOneBuffer(*buf);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index e0af32075d1..5eda26df455 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1893,6 +1893,7 @@ seq_redo(XLogReaderState *record)
memcpy(page, localpage, BufferGetPageSize(buffer));
MarkBufferDirty(buffer);
+ XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
pfree(localpage);
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5b77b11f508..d0545248abf 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -84,6 +84,8 @@ typedef struct ReadLocalXLogPageNoWaitPrivate
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
uint8 block_id, Buffer *buf);
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);
+extern void XLogFlushBufferForRedoIfInit(XLogReaderState *record,
+ uint8 block_id, Buffer buffer);
extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
uint8 block_id,
ReadBufferMode mode, bool get_cleanup_lock,
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 42059801ce2..5453e1aa6fa 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -47,6 +47,7 @@ tests += {
't/043_vacuum_horizon_floor.pl',
't/043_no_contrecord_switch.pl',
't/045_archive_restartpoint.pl',
+ 't/054_unlogged_sequence_promotion.pl',
],
},
}
diff --git a/src/test/recovery/t/054_unlogged_sequence_promotion.pl b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
new file mode 100644
index 00000000000..96d1e4bf18b
--- /dev/null
+++ b/src/test/recovery/t/054_unlogged_sequence_promotion.pl
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that unlogged sequences created on a primary can be read after
+# promotion of a standby that replayed their init fork.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create the unlogged sequence after the standby has started, so its init fork
+# is generated by WAL replay on the standby.
+$node_primary->safe_psql('postgres', "CREATE UNLOGGED SEQUENCE ulseq");
+$node_primary->wait_for_replay_catchup($node_standby);
+
+$node_standby->promote;
+
+is($node_standby->safe_psql('postgres', "SELECT nextval('ulseq')"),
+ 1, 'unlogged sequence can be read after standby promotion');
+
+done_testing();
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-29 16:01 vignesh C <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: vignesh C @ 2026-06-29 16:01 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 26 Jun 2026 at 12:34, Fujii Masao <[email protected]> wrote:
>
> On Wed, Jun 24, 2026 at 7:49 PM vignesh C <[email protected]> wrote:
> > Thanks for the patch. I verified that it fixes the issue with reading
> > unlogged sequences on a promoted standby.
>
> Thanks for testing!
>
>
> > Do you think it would be worthwhile to add a test for this scenario,
> > or do you feel the additional test is not necessary in this case?
>
> I think it's worth adding a test for this scenario, so I've added one to
> the patch. The test uses nextval() to read the unlogged sequence
> instead of pg_get_sequence_data(), since this patch needs to be
> backpatched to v15, while pg_get_sequence_data() was introduced in
> v19.
Thanks for the updated patches. I verified that the issue occurs with
nextval() on all supported branches up to v15, where unlogged
sequences are available. The corresponding back patches apply cleanly
to their respective branches and fix the issue. I also checked other
unlogged objects but couldn't reproduce a similar issue with them.
Overall, the patches look good to me.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-06-29 23:55 Fujii Masao <[email protected]>
parent: vignesh C <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Fujii Masao @ 2026-06-29 23:55 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 30, 2026 at 1:01 AM vignesh C <[email protected]> wrote:
> Thanks for the updated patches. I verified that the issue occurs with
> nextval() on all supported branches up to v15, where unlogged
> sequences are available. The corresponding back patches apply cleanly
> to their respective branches and fix the issue. I also checked other
> unlogged objects but couldn't reproduce a similar issue with them.
>
> Overall, the patches look good to me.
Thanks for the review! I've pushed the patch.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-07-01 16:08 Fujii Masao <[email protected]>
parent: Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2026-07-01 16:08 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Amit Kapila <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 24, 2026 at 2:40 PM Bharath Rupireddy
<[email protected]> wrote:
>
> Hi,
>
> On Tue, Jun 23, 2026 at 5:41 PM Fujii Masao <[email protected]> wrote:
> >
> > BTW, isn't the current documentation a bit misleading? It says:
> >
> > This function returns a row of NULL values if the sequence does not exist.
> >
> > But if the specified object does not exist, pg_get_sequence_data()
> > raises an error rather than returning a row of NULL values. On the
> > other hand, it does return a row of NULL values if the specified
> > object exists but is not a sequence. Is my understanding correct? If
> > so, how about something like:
>
> When the provided relation oid doesn't exist, returns NULL:
> postgres=# select pg_get_sequence_data(99999999);
> pg_get_sequence_data
> ----------------------
> (,,)
> (1 row)
Yes. If the specified OID does not exist, the function returns a row of
NULL values. However, if the object is specified by name and it does
not exist, an error is raised because the argument has type regclass.
That's why I find the current wording "This function returns a row of
NULL values if the sequence does not exist." a bit misleading.
How about something like this instead?
This function returns a row of NULL values if the specified relation
OID does not exist, if it is not a sequence, if the current user lacks
<literal>SELECT</literal> privilege on the sequence, if the sequence
is another session's temporary sequence, or if it is an unlogged
sequence on a standby server.
> > The attached patch adds those HINT messages. It also updates the
> > related documentation.
>
> Nice! This looks more explicit and clarifying. The patch LGTM.
Thanks for the review!
I've updated the patch furthermore. Attached.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v2-0001-Add-hints-for-sequence-synchronization-permission.patch (4.4K, ../../CAHGQGwGj0S-eSXfE81qf+xHHy_1LK-XyRr0=DuPC=4PYcB4_aQ@mail.gmail.com/2-v2-0001-Add-hints-for-sequence-synchronization-permission.patch)
download | inline diff:
From f71d95fed5c4c4c1a0492c616c153e1e66555772 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 1 Jul 2026 23:56:10 +0900
Subject: [PATCH v2] Add hints for sequence synchronization permission warnings
Sequence synchronization reports insufficient privileges on publisher
and subscriber sequences, but the warnings do not indicate which role
needs which privilege. This makes common configuration mistakes harder
to diagnose.
Add HINT messages for these warnings. Publisher-side warnings suggest
granting SELECT to the role used for the replication connection.
Subscriber-side warnings suggest granting UPDATE to the subscription
owner when run_as_owner is enabled. Otherwise, the worker runs as the
sequence owner, so no useful GRANT hint can be provided.
---
doc/src/sgml/logical-replication.sgml | 14 ++++++++---
.../replication/logical/sequencesync.c | 23 +++++++++++++++++--
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 5befefd9c5a..d7d75b7c9a5 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2542,9 +2542,10 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
</para>
<para>
- In order to be able to copy the initial table or sequence data, the role
- used for the replication connection must have the <literal>SELECT</literal>
- privilege on a published table or sequence (or be a superuser).
+ In order to be able to copy the initial table data or synchronize
+ sequences, the role used for the replication connection must have the
+ <literal>SELECT</literal> privilege on a published table or sequence (or be
+ a superuser).
</para>
<para>
@@ -2611,6 +2612,13 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
security within the database is of no concern.
</para>
+ <para>
+ When synchronizing sequences with
+ <literal>run_as_owner = true</literal>, the subscription owner similarly
+ needs <literal>UPDATE</literal> privilege on the target sequence and does
+ not need privileges to <literal>SET ROLE</literal> to the sequence owner.
+ </para>
+
<para>
On the publisher, privileges are only checked once at the start of a
replication connection and are not re-checked as each change record is read.
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index f47f962c7db..770fa5de10b 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -201,12 +201,26 @@ report_sequence_errors(List *mismatched_seqs_idx,
if (sub_insuffperm_seqs_idx)
{
get_sequences_string(sub_insuffperm_seqs_idx, &seqstr);
+
+ /*
+ * With run_as_owner enabled, sequence synchronization runs as the
+ * subscription owner, so a missing UPDATE privilege should be granted
+ * to that role. Otherwise, the worker switches to the sequence owner
+ * before checking privileges, so no useful GRANT hint can be
+ * provided.
+ */
ereport(WARNING,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg_plural("insufficient privileges on subscriber sequence (%s)",
"insufficient privileges on subscriber sequences (%s)",
list_length(sub_insuffperm_seqs_idx),
- seqstr.data));
+ seqstr.data),
+ MySubscription->runasowner ?
+ errhint_plural("Grant UPDATE on the sequence to the subscription "
+ "owner on the subscriber.",
+ "Grant UPDATE on the sequences to the subscription "
+ "owner on the subscriber.",
+ list_length(sub_insuffperm_seqs_idx)) : 0);
}
if (pub_insuffperm_seqs_idx)
@@ -217,7 +231,12 @@ report_sequence_errors(List *mismatched_seqs_idx,
errmsg_plural("insufficient privileges on publisher sequence (%s)",
"insufficient privileges on publisher sequences (%s)",
list_length(pub_insuffperm_seqs_idx),
- seqstr.data));
+ seqstr.data),
+ errhint_plural("Grant SELECT on the sequence to the role used for "
+ "the replication connection on the publisher.",
+ "Grant SELECT on the sequences to the role used for "
+ "the replication connection on the publisher.",
+ list_length(pub_insuffperm_seqs_idx)));
}
if (missing_seqs_idx)
--
2.53.0
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-07-02 09:55 Amit Kapila <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Amit Kapila @ 2026-07-02 09:55 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jul 1, 2026 at 9:38 PM Fujii Masao <[email protected]> wrote:
>
> On Wed, Jun 24, 2026 at 2:40 PM Bharath Rupireddy
> <[email protected]> wrote:
> >
> > Hi,
> >
> > On Tue, Jun 23, 2026 at 5:41 PM Fujii Masao <[email protected]> wrote:
> > >
> > > BTW, isn't the current documentation a bit misleading? It says:
> > >
> > > This function returns a row of NULL values if the sequence does not exist.
> > >
> > > But if the specified object does not exist, pg_get_sequence_data()
> > > raises an error rather than returning a row of NULL values. On the
> > > other hand, it does return a row of NULL values if the specified
> > > object exists but is not a sequence. Is my understanding correct? If
> > > so, how about something like:
> >
> > When the provided relation oid doesn't exist, returns NULL:
> > postgres=# select pg_get_sequence_data(99999999);
> > pg_get_sequence_data
> > ----------------------
> > (,,)
> > (1 row)
>
> Yes. If the specified OID does not exist, the function returns a row of
> NULL values. However, if the object is specified by name and it does
> not exist, an error is raised because the argument has type regclass.
>
> That's why I find the current wording "This function returns a row of
> NULL values if the sequence does not exist." a bit misleading.
>
> How about something like this instead?
>
> This function returns a row of NULL values if the specified relation
> OID does not exist, if it is not a sequence, if the current user lacks
> <literal>SELECT</literal> privilege on the sequence, if the sequence
> is another session's temporary sequence, or if it is an unlogged
> sequence on a standby server.
>
Sounds reasonable. But after this we don't need the next para to say:
"It requires <literal>SELECT</literal> privilege on the sequence.".
See attached.
>
> > > The attached patch adds those HINT messages. It also updates the
> > > related documentation.
> >
> > Nice! This looks more explicit and clarifying. The patch LGTM.
>
> Thanks for the review!
>
> I've updated the patch furthermore. Attached.
>
LGTM.
--
With Regards,
Amit Kapila.
Attachments:
[application/octet-stream] update_doc_1.patch (1.4K, ../../CAA4eK1+foYb-vvY2DAq6mg8xn-5q4hY6OiTNPvuHd-ZC5=kwMA@mail.gmail.com/2-update_doc_1.patch)
download | inline diff:
diff --git a/doc/src/sgml/func/func-sequence.sgml b/doc/src/sgml/func/func-sequence.sgml
index de266c36296..9301faa67d2 100644
--- a/doc/src/sgml/func/func-sequence.sgml
+++ b/doc/src/sgml/func/func-sequence.sgml
@@ -163,13 +163,15 @@ SELECT setval('myseq', 42, false); <lineannotation>Next <function>nextval</fu
<structfield>is_called</structfield> indicates whether the sequence has
been used. <structfield>page_lsn</structfield> is the LSN corresponding
to the most recent WAL record that modified this sequence relation.
- This function returns a row of NULL values if the sequence does not
- exist or if the current user lacks privileges on it.
+ This function returns a row of NULL values if the specified relation
+ OID does not exist, if it is not a sequence, if the current user lacks
+ <literal>SELECT</literal> privilege on the sequence, if the sequence
+ is another session's temporary sequence, or if it is an unlogged
+ sequence on a standby server.
</para>
<para>
This function is primarily intended for internal use by pg_dump and by
- logical replication to synchronize sequences. It requires
- <literal>SELECT</literal> privilege on the sequence.
+ logical replication to synchronize sequences.
</para></entry>
</row>
</tbody>
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Fix publisher-side sequence permission reporting
@ 2026-07-08 09:19 Fujii Masao <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Fujii Masao @ 2026-07-08 09:19 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Tristan Partin <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Jul 2, 2026 at 6:55 PM Amit Kapila <[email protected]> wrote:
> > How about something like this instead?
> >
> > This function returns a row of NULL values if the specified relation
> > OID does not exist, if it is not a sequence, if the current user lacks
> > <literal>SELECT</literal> privilege on the sequence, if the sequence
> > is another session's temporary sequence, or if it is an unlogged
> > sequence on a standby server.
> >
>
> Sounds reasonable. But after this we don't need the next para to say:
> "It requires <literal>SELECT</literal> privilege on the sequence.".
> See attached.
Thanks for the patch! I've pushed it.
> > I've updated the patch furthermore. Attached.
> >
>
> LGTM.
Thanks for the review! I've pushed this as well.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2026-07-08 09:19 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-18 15:36 Fix publisher-side sequence permission reporting Fujii Masao <[email protected]>
2026-06-18 23:11 ` Tristan Partin <[email protected]>
2026-06-19 14:15 ` Fujii Masao <[email protected]>
2026-06-19 16:38 ` Tristan Partin <[email protected]>
2026-06-20 09:24 ` Fujii Masao <[email protected]>
2026-06-20 10:20 ` Amit Kapila <[email protected]>
2026-06-20 12:43 ` Fujii Masao <[email protected]>
2026-06-22 04:03 ` Amit Kapila <[email protected]>
2026-06-24 00:41 ` Fujii Masao <[email protected]>
2026-06-24 05:40 ` Bharath Rupireddy <[email protected]>
2026-07-01 16:08 ` Fujii Masao <[email protected]>
2026-07-02 09:55 ` Amit Kapila <[email protected]>
2026-07-08 09:19 ` Fujii Masao <[email protected]>
2026-06-24 07:40 ` Amit Kapila <[email protected]>
2026-06-24 08:50 ` Fujii Masao <[email protected]>
2026-06-24 10:30 ` Fujii Masao <[email protected]>
2026-06-24 10:49 ` vignesh C <[email protected]>
2026-06-26 07:04 ` Fujii Masao <[email protected]>
2026-06-29 16:01 ` vignesh C <[email protected]>
2026-06-29 23:55 ` Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox