From 24a2b541bd450a2db9d8f3ddefc92ace58ef9a42 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 17 Jul 2026 00:50:54 +0900 Subject: [PATCH] Handle concurrent sequence drops during synchronization Commit d4a657b0a4d added a call to has_sequence_privilege() while fetching sequence information from the publisher, so that publisher-side permission failures could be distinguished from missing sequences. It also assumed that has_sequence_privilege() could never return NULL, and asserted accordingly. However, that assumption was incorrect. If a sequence is dropped after the synchronization worker collects its metadata but while fetching the sequence information, has_sequence_privilege() can return NULL. This can trigger the assertion failure. This was also reported in a buildfarm failure on member culicidae. Fix this by treating a NULL result from has_sequence_privilege() as indicating that the sequence was dropped concurrently, and report it as a missing sequence instead of asserting that the result is never NULL. Reported-by: Noah Misch Author: Vignesh C Reviewed-by: Hayato Kuroda Reviewed-by: Zsolt Parragi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com Discussion: https://postgr.es/m/CALDaNm2fHGLeiQKj0r6OG7N9QeayxSmpLrWYJRyt4dL_m3VRWw@mail.gmail.com Backpatch-through: 19 --- .../replication/logical/sequencesync.c | 15 ++++- src/test/subscription/t/036_sequences.pl | 56 ++++++++++++++++++- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index 770fa5de10b..63ad46d7fd7 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -289,14 +289,23 @@ get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel, *seqinfo = seqinfo_local = (LogicalRepSequenceInfo *) list_nth(seqinfos, *seqidx); + /* + * has_sequence_privilege() itself returns NULL, rather than false, when + * the sequence has been dropped concurrently after it was identified in + * the catalog snapshot (see has_sequence_privilege_id()). Treat that as a + * missing sequence on the publisher. + */ + datum = slot_getattr(slot, ++col, &isnull); + if (isnull) + return COPYSEQ_SKIPPED; + + remote_has_select_priv = DatumGetBool(datum); + /* * 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 remote_has_select_priv ? COPYSEQ_SKIPPED : diff --git a/src/test/subscription/t/036_sequences.pl b/src/test/subscription/t/036_sequences.pl index 8b02b24a7e9..b3b3b20f82b 100644 --- a/src/test/subscription/t/036_sequences.pl +++ b/src/test/subscription/t/036_sequences.pl @@ -188,6 +188,60 @@ is($result, '1|f', 'REFRESH PUBLICATION will not sync newly published sequence with copy_data as false' ); +########## +# A sequence dropped concurrently on the publisher, while the sequencesync +# worker's batch query is executing, must be treated the same as any other +# concurrently-dropped sequence (reported as "missing sequence on publisher"). +########## + +my $log_offset = -s $node_subscriber->logfile; + +# Block the sequencesync worker's batch query on the publisher: an +# uncommitted DROP SEQUENCE holds AccessExclusiveLock, on which the +# pg_get_sequence_data() call in the batch query will wait. +my $pub_session = $node_publisher->background_psql('postgres'); +$pub_session->query_safe( + qq( + BEGIN; + DROP SEQUENCE regress_s3; +)); + +$node_subscriber->safe_psql('postgres', + "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES"); + +# Wait until the worker's batch query is blocked on the still uncommitted +# DROP. +$node_publisher->poll_query_until( + 'postgres', qq( + SELECT EXISTS ( + SELECT 1 FROM pg_locks + WHERE relation = 'regress_s3'::regclass + AND mode = 'AccessShareLock' + AND NOT granted); +)) or die "timed out waiting for sequencesync worker to block on publisher"; + +# Commit the DROP while the batch query is blocked inside it, so the query +# resumes against a sequence that no longer exists. +# After pg_get_sequence_data() is unblocked, the batch query evaluates +# has_sequence_privilege(c.oid, 'SELECT') in the target list. Since the DROP +# has been committed by then, has_sequence_privilege() observes the missing +# sequence and returns NULL. +$pub_session->query_safe("COMMIT"); +$pub_session->quit; + +$node_subscriber->wait_for_log( + qr/WARNING: ( [A-Z0-9]+:)? missing sequence on publisher \("public.regress_s3"\)/, + $log_offset); + +$node_publisher->safe_psql( + 'postgres', qq( + CREATE SEQUENCE regress_s3; +)); + +# Wait for the recreated sequence to be synced. +$node_subscriber->poll_query_until('postgres', $synced_query) + or die "Timed out while waiting for subscriber to synchronize data"; + ########## # ALTER SUBSCRIPTION ... REFRESH PUBLICATION should report an error when: # a) sequence definitions differ between the publisher and subscriber, or @@ -206,7 +260,7 @@ $node_subscriber->safe_psql( CREATE SEQUENCE regress_s4 START 10 INCREMENT 2; )); -my $log_offset = -s $node_subscriber->logfile; +$log_offset = -s $node_subscriber->logfile; # Do ALTER SUBSCRIPTION ... REFRESH PUBLICATION $node_subscriber->safe_psql('postgres', -- 2.47.3