Hi,
On Sun, Sep27, 2026 at 10:26 PM Mihail Nikalayeu wrote:
Attached is v2 of the remaining fixes, rebased on master and
registered in the commitfest as [3] for tracking purposes.
I tried to combine everything into the series. It is mostly
AI-generated, but I still think structuring everything is helpful
anyway.
The rule throughout is that a partition pending detach is published as
a standalone table, as it will be after FINALIZE; publishing it via
the former root would cause the divergence Nisha described in [1].
0001 (pgoutput crash): The test comment Shveta asked about was
misleading; it's fixed. pgindent-clean applied.
Since we are considering whether to push 0001 first, I reviewed it and
have few suggestions:
1. We can remove the am_partition variable completely and simply check
"ancestors" in the subsequent code, to be consistent with
RelationBuildPublicationDesc.
2. The test can be simplified to cover only the decoding crash (since
the subscriber side is not touched by this patch), which saves buildfarm
test time. In addition, we can move the test into the existing isolation
test to speed it up further.
I have attached a top-up patch for 0001 with these changes. Feel free to
merge it if it looks good.
--
Best Regards,
Zhijie Hou
From 21de09d8b6e5814555738932fe6f3e615ce3eea7 Mon Sep 17 00:00:00 2001
From: Zhijie Hou <[email protected]>
Date: Wed, 11 Nov 2026 15:30:40 +0800
Subject: [PATCH] Top-up for v2-0001: Simplify am_partition check and test in
isolation.
In get_rel_sync_entry(), remove the boolean am_partition variable and
check the ancestors list directly, matching RelationBuildPublicationDesc()
in relcache.c.
Also, replace the new subscription TAP test in 100_bugs.pl with a
decoding step in the existing detach-partition-concurrently-3 isolation
permutation. Since this patch only touches pgoutput decoding on the
publisher side, testing via pg_logical_slot_get_binary_changes() in the
existing isolation permutation exercises the exact code path without the
overhead of setting up a subscriber and restarting nodes.
Author: Zhijie Hou <[email protected]>
Discussion:
https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
---
src/backend/replication/pgoutput/pgoutput.c | 10 +-
.../detach-partition-concurrently-3.out | 18 ++-
.../detach-partition-concurrently-3.spec | 17 ++-
src/test/subscription/t/100_bugs.pl | 110 ------------------
4 files changed, 31 insertions(+), 124 deletions(-)
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index e74a9f156e8..b5fb5ead85a 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2103,7 +2103,6 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
ListCell *lc;
Oid publish_as_relid = relid;
int publish_ancestor_level = 0;
- bool am_partition = get_rel_relispartition(relid);
char relkind = get_rel_relkind(relid);
List *rel_publications = NIL;
List *ancestors = NIL;
@@ -2115,11 +2114,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
* standalone table: changes are no longer published via its
former
* ancestors, but can still be published via its own
publications.
*/
- if (am_partition)
- {
+ if (get_rel_relispartition(relid))
ancestors = get_partition_ancestors(relid);
- am_partition = (ancestors != NIL);
- }
/* Reload publications if needed before use. */
if (!publications_valid)
@@ -2225,7 +2221,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
{
List *exceptpubids = NIL;
- if (am_partition)
+ if (ancestors)
{
Oid
last_ancestor_relid = llast_oid(ancestors);
@@ -2275,7 +2271,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
* published via this publication, which will
be used as the
* relation via which to publish the
partition's changes.
*/
- if (am_partition)
+ if (ancestors)
{
Oid ancestor;
int level;
diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out
b/src/test/isolation/expected/detach-partition-concurrently-3.out
index c1705d80aa1..75ff86b1701 100644
--- a/src/test/isolation/expected/detach-partition-concurrently-3.out
+++ b/src/test/isolation/expected/detach-partition-concurrently-3.out
@@ -106,7 +106,7 @@ t
step s1c: COMMIT;
step s1insertpart: INSERT INTO d3_listp1 VALUES (1);
-starting permutation: s2snitch s1b s1s s2detach s1cancel s1c s1updpart
s1delpart s1exceptpart
+starting permutation: s2snitch s1b s1s s2detach s1cancel s1c s1updpart
s1delpart s1exceptpart s1initslot s1insertpart s1decodepart
step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid();
step s1b: BEGIN;
step s1s: SELECT * FROM d3_listp;
@@ -130,6 +130,22 @@ step s1updpart: UPDATE d3_listp1 SET a = 1;
step s1delpart: DELETE FROM d3_listp1;
step s1exceptpart: CREATE PUBLICATION pub_d3 FOR ALL TABLES EXCEPT (TABLE
d3_listp1);
ERROR: cannot specify relation "public.d3_listp1" in the publication EXCEPT
clause
+step s1initslot: SELECT 'init' FROM
pg_create_logical_replication_slot('d3_slot', 'pgoutput', true);
+ CREATE PUBLICATION pub_d3 FOR ALL
TABLES WITH (publish_via_partition_root = true);
+?column?
+--------
+init
+(1 row)
+
+step s1insertpart: INSERT INTO d3_listp1 VALUES (1);
+step s1decodepart: SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('d3_slot', NULL, NULL,
+ 'proto_version', '1',
'publication_names', 'pub_d3');
+ DROP PUBLICATION pub_d3;
+?column?
+--------
+t
+(1 row)
+
starting permutation: s2snitch s1b s1s s2detach2 s1cancel s1c s1brr s1insert
s1s s1insert s1c
step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid();
diff --git a/src/test/isolation/specs/detach-partition-concurrently-3.spec
b/src/test/isolation/specs/detach-partition-concurrently-3.spec
index 6d359756e31..1e9cd66d52b 100644
--- a/src/test/isolation/specs/detach-partition-concurrently-3.spec
+++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec
@@ -34,6 +34,11 @@ step s1insertpart { INSERT INTO d3_listp1 VALUES (1); }
step s1updpart { UPDATE d3_listp1 SET a = 1; }
step s1delpart { DELETE FROM d3_listp1; }
step s1exceptpart { CREATE PUBLICATION pub_d3 FOR ALL TABLES EXCEPT
(TABLE d3_listp1); }
+step s1initslot { SELECT 'init' FROM
pg_create_logical_replication_slot('d3_slot', 'pgoutput', true);
+ CREATE PUBLICATION pub_d3 FOR ALL
TABLES WITH (publish_via_partition_root = true); }
+step s1decodepart { SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('d3_slot', NULL, NULL,
+ 'proto_version', '1',
'publication_names', 'pub_d3');
+ DROP PUBLICATION pub_d3; }
step s1drop { DROP TABLE d3_listp; }
step s1droppart { DROP TABLE d3_listp1; }
step s1trunc { TRUNCATE TABLE d3_listp; }
@@ -58,12 +63,12 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach)
s1insert s1c
permutation s2snitch s1brr s1s s2detach s1cancel(s2detach) s1insert s1c s1spart
permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1insertpart
-# Verify that UPDATE, DELETE, and publication exclusion addition
-# using EXCEPT clause correctly handle a concurrently detached partition.
-# A partition whose concurrent detach has been committed but not finalized
-# reports no ancestors, even though relispartition is still set, which
-# would otherwise trigger a topmost ancestor lookup crash.
-permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1updpart
s1delpart s1exceptpart
+# Verify that UPDATE, DELETE, publication exclusion addition using EXCEPT
+# clause, and pgoutput decoding correctly handle a concurrently detached
+# partition. A partition whose concurrent detach has been committed but not
+# finalized reports no ancestors, even though relispartition is still set,
+# which would otherwise trigger a topmost ancestor lookup crash.
+permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1updpart
s1delpart s1exceptpart s1initslot s1insertpart s1decodepart
# Test partition descriptor caching
permutation s2snitch s1b s1s s2detach2 s1cancel(s2detach2) s1c s1brr s1insert
s1s s1insert s1c
diff --git a/src/test/subscription/t/100_bugs.pl
b/src/test/subscription/t/100_bugs.pl
index 6a21b188b4b..a39d6b0d100 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -79,116 +79,6 @@ $node_publisher->stop('fast');
$node_subscriber->stop('fast');
-# Replicating changes to a partition whose concurrent detach never finished.
-
-# Between the two transactions of ALTER TABLE ... DETACH PARTITION ...
-# CONCURRENTLY, pg_class still says relispartition while
-# get_partition_ancestors() already reports nothing, and get_rel_sync_entry()
-# crashed on that; for a subscription that is a crash loop. The state outlives
-# the command: only DETACH PARTITION ... FINALIZE clears it.
-
-$node_publisher->append_conf('postgresql.conf',
- 'max_prepared_transactions = 1');
-$node_publisher->rotate_logfile();
-$node_publisher->start();
-$node_subscriber->rotate_logfile();
-$node_subscriber->start();
-
-my $detach_ddl = q[
- CREATE TABLE parted (a int, b int) PARTITION BY LIST (a);
- CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1);
- CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2);
-];
-$node_publisher->safe_psql('postgres', $detach_ddl);
-$node_subscriber->safe_psql('postgres', $detach_ddl);
-
-$node_publisher->safe_psql(
- 'postgres', q[
- CREATE PUBLICATION pub_detach FOR ALL TABLES
- WITH (publish_via_partition_root = true);
-]);
-$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION sub_detach CONNECTION '$publisher_connstr'
PUBLICATION pub_detach"
-);
-$node_subscriber->wait_for_subscription_sync($node_publisher, 'sub_detach');
-
-$node_publisher->safe_psql('postgres',
- 'INSERT INTO parted VALUES (1, 1), (2, 1)');
-$node_publisher->wait_for_catchup('sub_detach');
-
-is( $node_subscriber->safe_psql(
- 'postgres', 'SELECT * FROM parted ORDER BY a, b'),
- "1|1\n2|1",
- 'both partitions replicate while attached');
-
-# Leave part1 pending detach: a prepared transaction holds the snapshot the
-# detach waits for, so the wait ends in the lock timeout.
-$node_publisher->safe_psql('postgres',
- q[BEGIN; SELECT count(*) FROM parted; PREPARE TRANSACTION 'holder';]);
-{
- local $ENV{PGOPTIONS} = '-c lock_timeout=1s';
- $node_publisher->psql(
- 'postgres',
- 'ALTER TABLE parted DETACH PARTITION part1 CONCURRENTLY',
- on_error_stop => 0);
-}
-$node_publisher->safe_psql('postgres', q[ROLLBACK PREPARED 'holder']);
-
-is( $node_publisher->safe_psql(
- 'postgres', q[
- SELECT inhdetachpending FROM pg_inherits
- WHERE inhrelid = 'part1'::regclass]),
- 't',
- 'the partition is marked as detaching');
-
-# Changes nothing; drops the walsender's cached mapping.
-$node_publisher->safe_psql('postgres',
- 'ALTER PUBLICATION pub_detach SET (publish_via_partition_root = true)');
-
-# Use another slot to check which changes pgoutput sends, independently of
-# whether the subscriber applies them.
-$node_publisher->safe_psql('postgres',
- q[SELECT pg_create_logical_replication_slot('detach_slot',
'pgoutput')]);
-
-# This is what crashed. The part1 change is sent using part1's identity, but
-# the subscriber skips it because only parted is registered in
-# pg_subscription_rel. The following change shows decoding got past it.
-$node_publisher->safe_psql(
- 'postgres', q[
- INSERT INTO part1 VALUES (1, 2);
- INSERT INTO parted VALUES (2, 2);
-]);
-$node_publisher->wait_for_catchup('sub_detach');
-
-is( $node_subscriber->safe_psql(
- 'postgres', 'SELECT * FROM parted ORDER BY a, b'),
- "1|1\n2|1\n2|2",
- 'replication got past the partition pending detach');
-
-# In protocol version 1, an Insert message starts with 'I' followed by the
-# relation OID. Verify that pgoutput sends the change using part1's identity.
-is( $node_publisher->safe_psql(
- 'postgres', q[
- SELECT count(*)
- FROM pg_logical_slot_peek_binary_changes('detach_slot', NULL,
NULL,
- 'proto_version', '1', 'publication_names', 'pub_detach')
- WHERE get_byte(data, 0) = ascii('I')
- AND substring(data FROM 2 FOR 4) =
int4send('part1'::regclass::oid::int)]),
- '1',
- 'the change is sent as the partition pending detach');
-$node_publisher->safe_psql('postgres',
- q[SELECT pg_drop_replication_slot('detach_slot')]);
-
-# Drop replication state and the tables, as the tests below re-use the nodes.
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub_detach");
-$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_detach");
-$node_publisher->safe_psql('postgres', "DROP TABLE parted, part1");
-$node_subscriber->safe_psql('postgres', "DROP TABLE parted");
-
-$node_publisher->stop('fast');
-$node_subscriber->stop('fast');
-
-
# Handling of temporary and unlogged tables with FOR ALL TABLES publications
# If a FOR ALL TABLES publication exists, temporary and unlogged
--
2.54.0 (Apple Git-157)
