From cdbf4dcd758013d44e35b51254a6bc532f404be2 Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Tue, 20 Oct 2026 12:39:56 +0800 Subject: [PATCH v2] 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 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 | 51 +++++++++++++++++++ 2 files changed, 82 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..153bc765584 100644 --- a/src/test/subscription/t/036_sequences.pl +++ b/src/test/subscription/t/036_sequences.pl @@ -382,4 +382,55 @@ $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; +)); + +# Wait until the apply worker has started under the new subscription owner. +$node_subscriber->poll_query_until('postgres', + "SELECT count(1) = 1 FROM pg_stat_subscription s JOIN pg_stat_activity a USING (pid) WHERE s.subname = 'regress_seq_sub' AND s.worker_type = 'apply' AND a.usename = 'regress_seq_sub_owner'" +) or die "Timed out while waiting for apply worker to start as new owner"; + +$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.50.1 (Apple Git-155)