From 33b5e9e51efb9b561246e479bb68658551734dd0 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 5 Sep 2026 13:50:08 +0200 Subject: [PATCH v2 3/4] Include partitions pending detach in publication table lists For FOR ALL TABLES publications with publish_via_partition_root enabled, pgoutput publishes a partition with no active ancestors separately, but the publication table lookup still excludes it based on relispartition. As a result, REFRESH PUBLICATION cannot register the table before FINALIZE, and the subscriber silently skips its changes until then. The same holds when the EXCEPT clause names the former root: it no longer covers the partition. Use the active ancestor list in both the full and targeted publication table lookups. Cover regular and partitioned tables pending detach, including publication membership with and without an EXCEPT clause, decoded changes, the initial sync of a new subscription, and applying new changes after refresh with copy_data disabled. The full lookup, GetAllPublicationRelations(), has been affected since v14. The targeted one, is_table_publishable_in_publication(), was only added in v19 by fd7a25af11e2, so older branches need just the first half. Author: Zsolt Parragi Reported-by: shveta malik Reviewed-by: shveta malik Reviewed-by: Amit Kapila Reviewed-by: Nisha Moond Reviewed-by: Mikhail Nikalayeu Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com Backpatch-through: 14 --- src/backend/catalog/pg_publication.c | 40 +++- src/test/subscription/t/100_bugs.pl | 261 ++++++++++++++++++++------- 2 files changed, 225 insertions(+), 76 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f11ea8839ca..d9eda70ecf3 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1077,6 +1077,25 @@ GetAllTablesPublications(void) return result; } +/* + * Returns true if the given partition still has ancestors. + * + * A partition whose concurrent detach has been committed but not finalized + * has relispartition set, but get_partition_ancestors() reports nothing for + * it. Publications treat such a partition as a standalone table, as after + * the detach is finalized, so relispartition alone must not be used to + * decide whether a relation is published via its root. + */ +static bool +partition_has_ancestors(Oid relid) +{ + List *ancestors = get_partition_ancestors(relid); + bool result = (ancestors != NIL); + + list_free(ancestors); + return result; +} + /* * Gets list of all relations published by FOR ALL TABLES/SEQUENCES * publication. @@ -1122,7 +1141,8 @@ GetAllPublicationRelations(Oid pubid, char relkind, bool pubviaroot) Oid relid = relForm->oid; if (is_publishable_class(relid, relForm) && - !(relForm->relispartition && pubviaroot) && + !(pubviaroot && relForm->relispartition && + partition_has_ancestors(relid)) && !list_member_oid(exceptlist, relid)) result = lappend_oid(result, relid); } @@ -1144,7 +1164,8 @@ GetAllPublicationRelations(Oid pubid, char relkind, bool pubviaroot) Oid relid = relForm->oid; if (is_publishable_class(relid, relForm) && - !relForm->relispartition && + !(relForm->relispartition && + partition_has_ancestors(relid)) && !list_member_oid(exceptlist, relid)) result = lappend_oid(result, relid); } @@ -1370,7 +1391,6 @@ GetPublicationByName(const char *pubname, bool missing_ok) static bool is_table_publishable_in_publication(Oid relid, Publication *pub) { - bool relispartition; List *ancestors = NIL; /* @@ -1380,9 +1400,13 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) if (!pub->pubviaroot && get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE) return false; - relispartition = get_rel_relispartition(relid); - - if (relispartition) + /* + * A partition whose concurrent detach has been committed but not + * finalized reports no ancestors, even though relispartition is still + * set. Treat such a partition as a standalone table, as after the detach + * is finalized. + */ + if (get_rel_relispartition(relid)) ancestors = get_partition_ancestors(relid); if (pub->alltables) @@ -1391,7 +1415,7 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) * ALL TABLES with pubviaroot includes only regular tables or top-most * partitioned tables -- never child partitions. */ - if (pub->pubviaroot && relispartition) + if (pub->pubviaroot && ancestors) return false; /* @@ -1428,7 +1452,7 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) * If it's false, the partition is covered by its ancestor's presence in * the publication, it should be included (return true). */ - if (relispartition && + if (ancestors && OidIsValid(GetTopMostAncestorInPublication(pub->oid, ancestors, NULL))) return !pub->pubviaroot; diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index 7f125e49fc1..67cdb312f9d 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -94,96 +94,221 @@ $node_publisher->start(); $node_subscriber->rotate_logfile(); $node_subscriber->start(); -my $detach_ddl = q[ +for my $partition_type ('regular', 'partitioned') +{ + note "pending detach of a $partition_type partition"; + my $partition_by = + $partition_type eq 'partitioned' ? 'PARTITION BY LIST (b)' : ''; + my $detach_ddl = qq[ CREATE TABLE parted (a int, b int) PARTITION BY LIST (a); - CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1); + CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1) $partition_by; CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2); ]; -$node_publisher->safe_psql('postgres', $detach_ddl); -$node_subscriber->safe_psql('postgres', $detach_ddl); + $detach_ddl .= q[ + CREATE TABLE part1_leaf PARTITION OF part1 DEFAULT; +] if $partition_type eq 'partitioned'; + $node_publisher->safe_psql('postgres', $detach_ddl); + $node_subscriber->safe_psql('postgres', $detach_ddl); -$node_publisher->safe_psql( - 'postgres', q[ + $node_publisher->safe_psql( + 'postgres', q[ CREATE PUBLICATION pub_detach FOR ALL TABLES WITH (publish_via_partition_root = true); + CREATE PUBLICATION pub_detach_root FOR TABLE parted + WITH (publish_via_partition_root = true); + CREATE PUBLICATION pub_detach_except FOR ALL TABLES EXCEPT (TABLE parted) + 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_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', q[ + SELECT srrelid::regclass FROM pg_subscription_rel + WHERE srsubid = (SELECT oid FROM pg_subscription + WHERE subname = 'sub_detach')]), + 'parted', + 'only the root is registered in the subscription while attached'); -is( $node_subscriber->safe_psql( - 'postgres', 'SELECT * FROM parted ORDER BY a, b'), - "1|1\n2|1", - 'both partitions replicate while attached'); + $node_publisher->safe_psql('postgres', + 'INSERT INTO parted VALUES (1, 1), (2, 1)'); + $node_publisher->wait_for_catchup('sub_detach'); -# 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_subscriber->safe_psql( + 'postgres', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n2|1", + 'both partitions replicate while attached'); -is( $node_publisher->safe_psql( - 'postgres', q[ + # 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'); + '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)'); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass::text + FROM pg_get_publication_tables('pub_detach') ORDER BY 1]), + "part1\nparted", + 'ALL TABLES includes the partition pending detach as a separate table' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass + FROM pg_get_publication_tables(ARRAY['pub_detach'], 'part1'::regclass)]), + 'part1', + 'targeted publication lookup includes the partition pending detach'); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT count(*) + FROM pg_get_publication_tables(ARRAY['pub_detach_root'], 'part1'::regclass)] + ), + '0', + 'publishing only the former root does not include the partition pending detach' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass::text + FROM pg_get_publication_tables('pub_detach_except') ORDER BY 1]), + 'part1', + 'excluding the former root does not exclude the partition pending detach' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass + FROM pg_get_publication_tables(ARRAY['pub_detach_except'], 'part1'::regclass)] + ), + 'part1', + 'targeted lookup does not exclude the partition pending detach either' + ); -# 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')]); + # Changes nothing; drops the walsender's cached mapping. + $node_publisher->safe_psql('postgres', + 'ALTER PUBLICATION pub_detach SET (publish_via_partition_root = true)' + ); -# 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[ + # 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'); + $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'); + 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')]); + my %sent_as = ( + pub_detach => "part1\nparted", + pub_detach_root => 'parted', + pub_detach_except => 'part1'); + for my $publication (sort keys %sent_as) + { + # In protocol version 1, an Insert message starts with 'I' followed by + # the relation OID. List the relations the two inserts are sent as: + # ALL TABLES sends the part1 change as part1, whereas publishing only + # the former root does not send it at all, and neither sends it as + # parted. Excluding the former root leaves only the part1 change. + is( $node_publisher->safe_psql( + 'postgres', qq[ + SELECT (('x' || encode(substring(data FROM 2 FOR 4), 'hex'))::bit(32)::int)::oid::regclass::text AS relname + FROM pg_logical_slot_peek_binary_changes('detach_slot', NULL, NULL, + 'proto_version', '1', 'publication_names', '$publication') + WHERE get_byte(data, 0) = ascii('I') + ORDER BY relname]), + $sent_as{$publication}, + "relations the changes are sent as by $publication"); + } + $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"); + # Refresh must discover part1 even before FINALIZE. Do not copy existing + # rows, some of which were already replicated via parted before the detach. + $node_subscriber->safe_psql('postgres', + 'ALTER SUBSCRIPTION sub_detach REFRESH PUBLICATION WITH (copy_data = false)' + ); + is( $node_subscriber->safe_psql( + 'postgres', q[ + SELECT srrelid::regclass::text FROM pg_subscription_rel + WHERE srsubid = (SELECT oid FROM pg_subscription + WHERE subname = 'sub_detach') ORDER BY 1]), + "part1\nparted", + 'refresh registers the partition pending detach separately'); + $node_publisher->safe_psql('postgres', + 'INSERT INTO part1 VALUES (1, 3); INSERT INTO parted VALUES (2, 3)'); + $node_publisher->wait_for_catchup('sub_detach'); + is( $node_subscriber->safe_psql( + 'postgres', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n1|3\n2|1\n2|2\n2|3", + 'new changes to the partition pending detach are applied after refresh' + ); + + # The initial sync of a new subscription also sees part1 as a table of its + # own. Subscribe from another database, so that copying part1's rows does + # not clash with what sub_detach has already applied. + $node_subscriber->safe_psql('postgres', 'CREATE DATABASE detach_except'); + $node_subscriber->safe_psql('detach_except', $detach_ddl); + $node_subscriber->safe_psql('detach_except', + "CREATE SUBSCRIPTION sub_detach_except CONNECTION '$publisher_connstr' PUBLICATION pub_detach_except" + ); + $node_subscriber->wait_for_subscription_sync($node_publisher, + 'sub_detach_except', 'detach_except'); + is( $node_subscriber->safe_psql( + 'detach_except', + 'SELECT srrelid::regclass FROM pg_subscription_rel'), + 'part1', + 'a subscription excluding the former root registers the partition pending detach' + ); + is( $node_subscriber->safe_psql( + 'detach_except', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n1|2\n1|3", + 'initial sync copies the partition pending detach'); + $node_publisher->safe_psql('postgres', + 'INSERT INTO part1 VALUES (1, 4); INSERT INTO parted VALUES (2, 4)'); + $node_publisher->wait_for_catchup('sub_detach_except'); + is( $node_subscriber->safe_psql( + 'detach_except', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n1|2\n1|3\n1|4", + 'changes to the partition pending detach are applied, those via the excluded former root are not' + ); + $node_subscriber->safe_psql('detach_except', + 'DROP SUBSCRIPTION sub_detach_except'); + $node_subscriber->safe_psql('postgres', 'DROP DATABASE detach_except'); + + # 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, pub_detach_root, pub_detach_except"); + $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'); -- 2.43.0