From 45f1a69367eb9429284ed39440c3c4a768f6ea59 Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Tue, 20 Oct 2026 12:39:56 +0800 Subject: [PATCH v3] Add error context for sequence synchronization. When an unhandled ERROR occurs while synchronizing a sequence (for example, when SwitchToUntrustedUser() fails because the subscription owner cannot SET ROLE to the sequence owner), the error message does not identify which sequence or subscription triggered the failure. Add an error context callback around per-sequence validation and synchronization in copy_sequences() so that any error reports the schema-qualified sequence name and subscription name in CONTEXT. Reported-by: Zsolt Parragi Author: Zhijie Hou Reviewed-by: Ajin Cherian Reviewed-by: Hayato Kuroda (Fujitsu) Discussion: https://postgr.es/m/CAN4CZFMGiyksxVD1M7cc7_5E+evfi51xmDxw4uqdmvoeH53Ugw@mail.gmail.com Backpatch-through: 19 --- .../replication/logical/sequencesync.c | 32 ++++++++++++- src/test/subscription/t/036_sequences.pl | 46 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index 8d6b0cfef19..e56c9dde782 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -163,6 +163,22 @@ get_sequences_string(List *seqindexes, StringInfo buf) } } +/* + * Error callback to give more context info about the sequence being + * synchronized. + */ +static void +sequence_sync_error_callback(void *arg) +{ + LogicalRepSequenceInfo *seqinfo = *(LogicalRepSequenceInfo **) arg; + + if (seqinfo == NULL) + return; + + errcontext("synchronizing sequence \"%s.%s\" for subscription \"%s\"", + seqinfo->nspname, seqinfo->seqname, MySubscription->name); +} + /* * report_sequence_errors * @@ -621,9 +637,10 @@ copy_sequences(WalReceiverConn *conn) while (tuplestore_gettupleslot(res->tuplestore, true, false, slot)) { CopySeqResult sync_status; - LogicalRepSequenceInfo *seqinfo; + LogicalRepSequenceInfo *seqinfo = NULL; Relation sequence_rel = NULL; int seqidx; + ErrorContextCallback errcallback; CHECK_FOR_INTERRUPTS(); @@ -633,12 +650,25 @@ copy_sequences(WalReceiverConn *conn) ProcessConfigFile(PGC_SIGHUP); } + /* + * Set up callback to identify the sequence in error messages. + * seqinfo will be filled once get_and_validate_seq_info() + * resolves the sequence index. + */ + errcallback.callback = sequence_sync_error_callback; + errcallback.arg = &seqinfo; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + sync_status = get_and_validate_seq_info(slot, &sequence_rel, &seqinfo, &seqidx); if (sync_status == COPYSEQ_SUCCESS) sync_status = copy_sequence(seqinfo, sequence_rel->rd_rel->relowner); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + switch (sync_status) { case COPYSEQ_SUCCESS: diff --git a/src/test/subscription/t/036_sequences.pl b/src/test/subscription/t/036_sequences.pl index dd6fa515df3..aebb59f99a3 100644 --- a/src/test/subscription/t/036_sequences.pl +++ b/src/test/subscription/t/036_sequences.pl @@ -382,4 +382,50 @@ $node_subscriber->wait_for_log( qr/WARNING: ( [A-Z0-9]+:)? missing sequence on publisher \("public.regress_s2"\)/, $log_offset); +########## +# Ensure that an error raised while synchronizing a sequence includes the +# sequence name in the error context. +########## + +# Remove the dropped sequence (regress_s2) from the subscription, then disable +# the subscription and wait for its workers to stop before changing its +# connection and owner. +$node_subscriber->safe_psql( + 'postgres', qq( + ALTER SUBSCRIPTION regress_seq_sub REFRESH PUBLICATION WITH (copy_data = false); + ALTER SUBSCRIPTION regress_seq_sub DISABLE; +)); +$node_subscriber->poll_query_until('postgres', + "SELECT count(1) = 0 FROM pg_stat_subscription WHERE subname = 'regress_seq_sub' AND pid IS NOT NULL" +) or die "Timed out while waiting for subscription workers to stop"; + +# Change the subscription owner to regress_seq_sub_owner, and make regress_s4 +# owned by a separate role (regress_seq_other) that regress_seq_sub_owner +# cannot SET ROLE to. With run_as_owner = false (the default), synchronizing +# regress_s4 will fail when switching to its owner. +$node_subscriber->safe_psql( + 'postgres', qq( + ALTER SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr'; + ALTER SUBSCRIPTION regress_seq_sub SET (password_required = false); + CREATE ROLE regress_seq_sub_owner LOGIN; + CREATE ROLE regress_seq_other; + GRANT CREATE ON DATABASE postgres TO regress_seq_sub_owner; + ALTER SEQUENCE regress_s1 OWNER TO regress_seq_sub_owner; + ALTER SEQUENCE regress_s3 OWNER TO regress_seq_sub_owner; + ALTER SEQUENCE "regress'quote" OWNER TO regress_seq_sub_owner; + ALTER SEQUENCE regress_s4 OWNER TO regress_seq_other; + ALTER SUBSCRIPTION regress_seq_sub OWNER TO regress_seq_sub_owner; + ALTER SUBSCRIPTION regress_seq_sub ENABLE; +)); + +$log_offset = -s $node_subscriber->logfile; + +$node_subscriber->safe_psql('postgres', + "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES"); + +# Verify that the SET ROLE error reports the failing sequence in CONTEXT. +$node_subscriber->wait_for_log( + qr/ERROR: ( [A-Z0-9]+:)? role "regress_seq_sub_owner" cannot SET ROLE to "regress_seq_other"\n.*CONTEXT: synchronizing sequence "public\.regress_s4" for subscription "regress_seq_sub"/, + $log_offset); + done_testing(); -- 2.34.1