Hi,
On Wed, Nov11, 2026 at 05:06 PM shveta malik wrote:
On Wed, Nov 11, 2026 at 1:05 PM Zhijie Hou <[email protected]> wrote:
> 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.
I agree with the changes. Few concerns:
Thanks for reviewing.
It would be better to show in the additional steps that
pg_inherits.inhdetachpending is true before we actually try
pg_logical_slot_get_binary_changes.
I checked that no existing tests (either for DETACH CONCURRENTLY or
otherwise) refer to the inhdetachpendin in their test cases, and since
our test basically tests the DETACH behavior as well, I think it's fine
to skip testing this.
Also I feel it would make more sense to create-pub with default
publish_via_partition_root rather than publish_via_partition_root=true
for PG19's testcase.
Changed as suggested.
After some analysis, I think creating slots in src/test/isolation/
wouldn't be stable, since some buildfarm animals may run the main
regression tests under wal_level=minimal. I also didn't find slot
creation in the main isolation tests. So I chose to put the test in
test_decoding/concurrent_ddl_dml.spec, which has precedent for creating
slots with pgoutput under test_decoding tests, so it can be acceptable.
I'm helping to merge the changes into 0001 and preparing patches for all
branches, hoping to close this item soon.
--
Best Regards,
Zhijie Hou
From cf0cae917d2ba78deb69b8b9e868f7191efaab84 Mon Sep 17 00:00:00 2001
From: Mikhail Nikalayeu <[email protected]>
Date: Wed, 11 Nov 2026 19:22:01 +0800
Subject: [PATCH v3] Fix crash decoding a change to a partition pending detach.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY, added by 71f4c8c6f74b,
leaves the partition marked as detaching when its wait is interrupted -- by
a lock timeout, a cancel, a disconnect. Only DETACH PARTITION ... FINALIZE
clears that mark, and the partition cannot be attached back. In that state
pg_class still says relispartition, while get_partition_ancestors() already
reports nothing.
get_rel_sync_entry() was not ready for it: it read relispartition as meaning
that the ancestor list is not empty, and asked for its last element, which is
an assertion failure, and a NULL pointer dereference without assertions.
Decoding any change to such a partition for a FOR ALL TABLES publication gets
there, and as the change is never confirmed, a subscription turns that into a
crash loop.
Fetch the ancestors once, up front, and treat a partition with no ancestors
as a standalone table, which is how it is published once the detach
completes.
The crash became reachable in v14, where 71f4c8c6f74b introduced the state,
and fd366065e06a widened it from a publication with
publish_via_partition_root to any FOR ALL TABLES one.
Reported-by: Mikhail Nikalayeu <[email protected]>
Reported-by: Nisha Moond <[email protected]>
Reported-by: Zsolt Parragi <[email protected]>
Author: Mikhail Nikalayeu <[email protected]>
Author: Zhijie Hou <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Reviewed-by: Nisha Moond <[email protected]>
Reviewed-by: shveta malik <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Zhijie Hou <[email protected]>
Discussion:
https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
Discussion:
https://postgr.es/m/CABdArM64ZSat%3DdB094U8OCBzp1m8_Y85m6T%2BxPEgpLVJb60_rA%40mail.gmail.com
Backpatch-through: 15
---
.../expected/concurrent_ddl_dml.out | 38 +++++++++++++++++++
.../specs/concurrent_ddl_dml.spec | 17 +++++++++
src/backend/catalog/partition.c | 5 +++
src/backend/replication/pgoutput/pgoutput.c | 20 +++++++---
4 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/contrib/test_decoding/expected/concurrent_ddl_dml.out
b/contrib/test_decoding/expected/concurrent_ddl_dml.out
index 3742a2a2474..a09776fea8a 100644
--- a/contrib/test_decoding/expected/concurrent_ddl_dml.out
+++ b/contrib/test_decoding/expected/concurrent_ddl_dml.out
@@ -797,3 +797,41 @@ COMMIT
stop
(1 row)
+
+starting permutation: s1_init_pgoutput s1_begin s1_select_part s2_detach_part
s1_cancel_s2 s1_commit s1_create_pub s1_insert_part s2_get_binary_changes
+step s1_init_pgoutput: SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput');
+?column?
+--------
+init
+(1 row)
+
+step s1_begin: BEGIN;
+step s1_select_part: SELECT * FROM tbl_part;
+val1
+----
+(0 rows)
+
+step s2_detach_part: ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; <waiting ...>
+step s1_cancel_s2: SELECT pg_cancel_backend(pid) FROM pg_stat_activity WHERE
application_name = 'isolation/concurrent_ddl_dml/s2'; <waiting ...>
+step s2_detach_part: <... completed>
+ERROR: canceling statement due to user request
+step s1_cancel_s2: <... completed>
+pg_cancel_backend
+-----------------
+t
+(1 row)
+
+step s1_commit: COMMIT;
+step s1_create_pub: CREATE PUBLICATION pub FOR ALL TABLES WITH
(publish_via_partition_root = true);
+step s1_insert_part: INSERT INTO tbl_part_p1 VALUES (1);
+step s2_get_binary_changes: SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub');
+?column?
+--------
+t
+(1 row)
+
+?column?
+--------
+stop
+(1 row)
+
diff --git a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
index d16515e6f48..77ec8c133a5 100644
--- a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
+++ b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
@@ -2,12 +2,17 @@ setup
{
DROP TABLE IF EXISTS tbl1;
DROP TABLE IF EXISTS tbl2;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
CREATE TABLE tbl1(val1 integer, val2 integer);
CREATE TABLE tbl2(val1 integer, val2 integer);
+ CREATE TABLE tbl_part(val1 integer) PARTITION BY LIST (val1);
+ CREATE TABLE tbl_part_p1 PARTITION OF tbl_part FOR VALUES IN (1);
}
teardown
{
+ DROP PUBLICATION IF EXISTS pub;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
DROP TABLE tbl1;
DROP TABLE tbl2;
SELECT 'stop' FROM pg_drop_replication_slot('isolation_slot');
@@ -17,10 +22,15 @@ session "s1"
setup { SET synchronous_commit=on; }
step "s1_init" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'test_decoding'); }
+step "s1_init_pgoutput" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput'); }
step "s1_begin" { BEGIN; }
step "s1_insert_tbl1" { INSERT INTO tbl1 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2" { INSERT INTO tbl2 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2_3col" { INSERT INTO tbl2 (val1, val2, val3) VALUES (1, 1,
1); }
+step "s1_select_part" { SELECT * FROM tbl_part; }
+step "s1_cancel_s2" { SELECT pg_cancel_backend(pid) FROM pg_stat_activity
WHERE application_name = 'isolation/concurrent_ddl_dml/s2'; }
+step "s1_create_pub" { CREATE PUBLICATION pub FOR ALL TABLES WITH
(publish_via_partition_root = true); }
+step "s1_insert_part" { INSERT INTO tbl_part_p1 VALUES (1); }
step "s1_commit" { COMMIT; }
session "s2"
@@ -43,8 +53,10 @@ step "s2_alter_tbl2_drop_3rd_col" { ALTER TABLE tbl2 DROP
COLUMN val3; }
step "s2_alter_tbl2_3rd_char" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE
character varying; }
step "s2_alter_tbl2_3rd_text" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE text; }
step "s2_alter_tbl2_3rd_int" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE int
USING val3::integer; }
+step "s2_detach_part" { ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; }
step "s2_get_changes" { SELECT data FROM
pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0',
'skip-empty-xacts', '1'); }
+step "s2_get_binary_changes" { SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub'); }
@@ -86,3 +98,8 @@ permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin"
"s1_insert_tbl1" "s2_a
permutation "s1_init" "s2_alter_tbl2_add_text" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_3rd_char" "s1_insert_tbl2_3col" "s1_commit"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl2" "s2_get_changes"
permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl1" "s1_commit" "s2_get_changes"
+
+# Verify that pgoutput can decode changes to a partition whose concurrent
+# detach was canceled, when the relation is still marked as a partition in
+# pg_class but already reports no ancestors.
+permutation "s1_init_pgoutput" "s1_begin" "s1_select_part" "s2_detach_part"
"s1_cancel_s2"("s2_detach_part") "s1_commit" "s1_create_pub" "s1_insert_part"
"s2_get_binary_changes"
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index f8780ce57da..86d964fc233 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -125,6 +125,11 @@ get_partition_parent_worker(Relation inhRel, Oid relid,
bool *detach_pending)
*
* Returns a list of ancestors of the given relation.
*
+ * A partition pending concurrent detach is treated as having no parent. The
+ * list can therefore be empty even if the relation is still marked as a
+ * partition in pg_class. If an ancestor is pending detach, the list ends at
+ * that ancestor.
+ *
* Note: Because this function assumes that the relation whose OID is passed
* as an argument and each ancestor will have precisely one parent, it should
* only be called when it is known that the relation is a partition.
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index ad13f58e649..565775431d8 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2049,9 +2049,19 @@ 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;
+
+ /*
+ * A partition whose concurrent detach has been committed but
not
+ * finalized reports no ancestors, even though relispartition
is still
+ * set. Determine publication membership and identity as for a
+ * standalone table: changes are no longer published via its
former
+ * ancestors, but can still be published via its own
publications.
+ */
+ if (get_rel_relispartition(relid))
+ ancestors = get_partition_ancestors(relid);
/* Reload publications if needed before use. */
if (!publications_valid)
@@ -2157,10 +2167,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
if (pub->alltables)
{
publish = true;
- if (pub->pubviaroot && am_partition)
+ if (pub->pubviaroot && ancestors)
{
- List *ancestors =
get_partition_ancestors(relid);
-
pub_relid = llast_oid(ancestors);
ancestor_level = list_length(ancestors);
}
@@ -2176,11 +2184,10 @@ 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;
- List *ancestors =
get_partition_ancestors(relid);
ancestor =
GetTopMostAncestorInPublication(pub->oid,
ancestors,
@@ -2275,6 +2282,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
list_free(pubids);
list_free(schemaPubids);
list_free(rel_publications);
+ list_free(ancestors);
entry->replicate_valid = true;
}
--
2.54.0 (Apple Git-157)
From e4b367d6c8c476b20dc54599d5fd1beab5707e02 Mon Sep 17 00:00:00 2001
From: Mikhail Nikalayeu <[email protected]>
Date: Wed, 11 Nov 2026 19:21:58 +0800
Subject: [PATCH v3] Fix crash decoding a change to a partition pending detach.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY, added by 71f4c8c6f74b,
leaves the partition marked as detaching when its wait is interrupted -- by
a lock timeout, a cancel, a disconnect. Only DETACH PARTITION ... FINALIZE
clears that mark, and the partition cannot be attached back. In that state
pg_class still says relispartition, while get_partition_ancestors() already
reports nothing.
get_rel_sync_entry() was not ready for it: it read relispartition as meaning
that the ancestor list is not empty, and asked for its last element, which is
an assertion failure, and a NULL pointer dereference without assertions.
Decoding any change to such a partition for a FOR ALL TABLES publication gets
there, and as the change is never confirmed, a subscription turns that into a
crash loop.
Fetch the ancestors once, up front, and treat a partition with no ancestors
as a standalone table, which is how it is published once the detach
completes.
The crash became reachable in v14, where 71f4c8c6f74b introduced the state,
and fd366065e06a widened it from a publication with
publish_via_partition_root to any FOR ALL TABLES one.
Reported-by: Mikhail Nikalayeu <[email protected]>
Reported-by: Nisha Moond <[email protected]>
Reported-by: Zsolt Parragi <[email protected]>
Author: Mikhail Nikalayeu <[email protected]>
Author: Zhijie Hou <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Reviewed-by: Nisha Moond <[email protected]>
Reviewed-by: shveta malik <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Zhijie Hou <[email protected]>
Discussion:
https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
Discussion:
https://postgr.es/m/CABdArM64ZSat%3DdB094U8OCBzp1m8_Y85m6T%2BxPEgpLVJb60_rA%40mail.gmail.com
Backpatch-through: 15
---
.../expected/concurrent_ddl_dml.out | 38 +++++++++++++++++++
.../specs/concurrent_ddl_dml.spec | 17 +++++++++
src/backend/catalog/partition.c | 5 +++
src/backend/replication/pgoutput/pgoutput.c | 20 +++++++---
4 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/contrib/test_decoding/expected/concurrent_ddl_dml.out
b/contrib/test_decoding/expected/concurrent_ddl_dml.out
index 3742a2a2474..a09776fea8a 100644
--- a/contrib/test_decoding/expected/concurrent_ddl_dml.out
+++ b/contrib/test_decoding/expected/concurrent_ddl_dml.out
@@ -797,3 +797,41 @@ COMMIT
stop
(1 row)
+
+starting permutation: s1_init_pgoutput s1_begin s1_select_part s2_detach_part
s1_cancel_s2 s1_commit s1_create_pub s1_insert_part s2_get_binary_changes
+step s1_init_pgoutput: SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput');
+?column?
+--------
+init
+(1 row)
+
+step s1_begin: BEGIN;
+step s1_select_part: SELECT * FROM tbl_part;
+val1
+----
+(0 rows)
+
+step s2_detach_part: ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; <waiting ...>
+step s1_cancel_s2: SELECT pg_cancel_backend(pid) FROM pg_stat_activity WHERE
application_name = 'isolation/concurrent_ddl_dml/s2'; <waiting ...>
+step s2_detach_part: <... completed>
+ERROR: canceling statement due to user request
+step s1_cancel_s2: <... completed>
+pg_cancel_backend
+-----------------
+t
+(1 row)
+
+step s1_commit: COMMIT;
+step s1_create_pub: CREATE PUBLICATION pub FOR ALL TABLES WITH
(publish_via_partition_root = true);
+step s1_insert_part: INSERT INTO tbl_part_p1 VALUES (1);
+step s2_get_binary_changes: SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub');
+?column?
+--------
+t
+(1 row)
+
+?column?
+--------
+stop
+(1 row)
+
diff --git a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
index d16515e6f48..77ec8c133a5 100644
--- a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
+++ b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
@@ -2,12 +2,17 @@ setup
{
DROP TABLE IF EXISTS tbl1;
DROP TABLE IF EXISTS tbl2;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
CREATE TABLE tbl1(val1 integer, val2 integer);
CREATE TABLE tbl2(val1 integer, val2 integer);
+ CREATE TABLE tbl_part(val1 integer) PARTITION BY LIST (val1);
+ CREATE TABLE tbl_part_p1 PARTITION OF tbl_part FOR VALUES IN (1);
}
teardown
{
+ DROP PUBLICATION IF EXISTS pub;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
DROP TABLE tbl1;
DROP TABLE tbl2;
SELECT 'stop' FROM pg_drop_replication_slot('isolation_slot');
@@ -17,10 +22,15 @@ session "s1"
setup { SET synchronous_commit=on; }
step "s1_init" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'test_decoding'); }
+step "s1_init_pgoutput" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput'); }
step "s1_begin" { BEGIN; }
step "s1_insert_tbl1" { INSERT INTO tbl1 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2" { INSERT INTO tbl2 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2_3col" { INSERT INTO tbl2 (val1, val2, val3) VALUES (1, 1,
1); }
+step "s1_select_part" { SELECT * FROM tbl_part; }
+step "s1_cancel_s2" { SELECT pg_cancel_backend(pid) FROM pg_stat_activity
WHERE application_name = 'isolation/concurrent_ddl_dml/s2'; }
+step "s1_create_pub" { CREATE PUBLICATION pub FOR ALL TABLES WITH
(publish_via_partition_root = true); }
+step "s1_insert_part" { INSERT INTO tbl_part_p1 VALUES (1); }
step "s1_commit" { COMMIT; }
session "s2"
@@ -43,8 +53,10 @@ step "s2_alter_tbl2_drop_3rd_col" { ALTER TABLE tbl2 DROP
COLUMN val3; }
step "s2_alter_tbl2_3rd_char" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE
character varying; }
step "s2_alter_tbl2_3rd_text" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE text; }
step "s2_alter_tbl2_3rd_int" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE int
USING val3::integer; }
+step "s2_detach_part" { ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; }
step "s2_get_changes" { SELECT data FROM
pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0',
'skip-empty-xacts', '1'); }
+step "s2_get_binary_changes" { SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub'); }
@@ -86,3 +98,8 @@ permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin"
"s1_insert_tbl1" "s2_a
permutation "s1_init" "s2_alter_tbl2_add_text" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_3rd_char" "s1_insert_tbl2_3col" "s1_commit"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl2" "s2_get_changes"
permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl1" "s1_commit" "s2_get_changes"
+
+# Verify that pgoutput can decode changes to a partition whose concurrent
+# detach was canceled, when the relation is still marked as a partition in
+# pg_class but already reports no ancestors.
+permutation "s1_init_pgoutput" "s1_begin" "s1_select_part" "s2_detach_part"
"s1_cancel_s2"("s2_detach_part") "s1_commit" "s1_create_pub" "s1_insert_part"
"s2_get_binary_changes"
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 93d72157a46..c8e116bd1fb 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -126,6 +126,11 @@ get_partition_parent_worker(Relation inhRel, Oid relid,
bool *detach_pending)
* The first element is the immediate parent and the last one is the topmost
* parent in the partition hierarchy.
*
+ * A partition pending concurrent detach is treated as having no parent. The
+ * list can therefore be empty even if the relation is still marked as a
+ * partition in pg_class. If an ancestor is pending detach, the list ends at
+ * that ancestor.
+ *
* Note: Because this function assumes that the relation whose OID is passed
* as an argument and each ancestor will have precisely one parent, it should
* only be called when it is known that the relation is a partition.
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index 7eed467edd5..e9781da17b1 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2100,9 +2100,19 @@ 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;
+
+ /*
+ * A partition whose concurrent detach has been committed but
not
+ * finalized reports no ancestors, even though relispartition
is still
+ * set. Determine publication membership and identity as for a
+ * standalone table: changes are no longer published via its
former
+ * ancestors, but can still be published via its own
publications.
+ */
+ if (get_rel_relispartition(relid))
+ ancestors = get_partition_ancestors(relid);
/* Reload publications if needed before use. */
if (!publications_valid)
@@ -2207,10 +2217,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
if (pub->alltables)
{
publish = true;
- if (pub->pubviaroot && am_partition)
+ if (pub->pubviaroot && ancestors)
{
- List *ancestors =
get_partition_ancestors(relid);
-
pub_relid = llast_oid(ancestors);
ancestor_level = list_length(ancestors);
}
@@ -2226,11 +2234,10 @@ 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;
- List *ancestors =
get_partition_ancestors(relid);
ancestor =
GetTopMostAncestorInPublication(pub->oid,
ancestors,
@@ -2328,6 +2335,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
list_free(pubids);
list_free(schemaPubids);
list_free(rel_publications);
+ list_free(ancestors);
entry->replicate_valid = true;
}
--
2.54.0 (Apple Git-157)
From 55b99bc32f569530d79f5f5912df348170c7ffa0 Mon Sep 17 00:00:00 2001
From: Mikhail Nikalayeu <[email protected]>
Date: Wed, 11 Nov 2026 21:28:52 +0800
Subject: [PATCH v3] Fix crash decoding a change to a partition pending detach.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY, added by 71f4c8c6f74b,
leaves the partition marked as detaching when its wait is interrupted -- by
a lock timeout, a cancel, a disconnect. Only DETACH PARTITION ... FINALIZE
clears that mark, and the partition cannot be attached back. In that state
pg_class still says relispartition, while get_partition_ancestors() already
reports nothing.
get_rel_sync_entry() was not ready for it: it read relispartition as meaning
that the ancestor list is not empty, and asked for its last element, which is
an assertion failure, and a NULL pointer dereference without assertions.
Decoding any change to such a partition for a FOR ALL TABLES publication gets
there, and as the change is never confirmed, a subscription turns that into a
crash loop.
Fetch the ancestors once, up front, and treat a partition with no ancestors
as a standalone table, which is how it is published once the detach
completes.
The crash became reachable in v14, where 71f4c8c6f74b introduced the state,
and fd366065e06a widened it from a publication with
publish_via_partition_root to any FOR ALL TABLES one.
Reported-by: Mikhail Nikalayeu <[email protected]>
Reported-by: Nisha Moond <[email protected]>
Reported-by: Zsolt Parragi <[email protected]>
Author: Mikhail Nikalayeu <[email protected]>
Author: Zhijie Hou <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Reviewed-by: Nisha Moond <[email protected]>
Reviewed-by: shveta malik <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Zhijie Hou <[email protected]>
Discussion:
https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
Discussion:
https://postgr.es/m/CABdArM64ZSat%3DdB094U8OCBzp1m8_Y85m6T%2BxPEgpLVJb60_rA%40mail.gmail.com
Backpatch-through: 15
---
.../expected/concurrent_ddl_dml.out | 38 +++++++++++++++++++
.../specs/concurrent_ddl_dml.spec | 17 +++++++++
src/backend/catalog/partition.c | 5 +++
src/backend/replication/pgoutput/pgoutput.c | 19 +++++++---
4 files changed, 74 insertions(+), 5 deletions(-)
diff --git a/contrib/test_decoding/expected/concurrent_ddl_dml.out
b/contrib/test_decoding/expected/concurrent_ddl_dml.out
index 3742a2a2474..4dddf045071 100644
--- a/contrib/test_decoding/expected/concurrent_ddl_dml.out
+++ b/contrib/test_decoding/expected/concurrent_ddl_dml.out
@@ -797,3 +797,41 @@ COMMIT
stop
(1 row)
+
+starting permutation: s1_init_pgoutput s1_begin s1_select_part s2_detach_part
s1_cancel_s2 s1_commit s1_create_pub s1_insert_part s2_get_binary_changes
+step s1_init_pgoutput: SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput');
+?column?
+--------
+init
+(1 row)
+
+step s1_begin: BEGIN;
+step s1_select_part: SELECT * FROM tbl_part;
+val1
+----
+(0 rows)
+
+step s2_detach_part: ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; <waiting ...>
+step s1_cancel_s2: SELECT pg_cancel_backend(pid) FROM pg_stat_activity WHERE
application_name = 'isolation/concurrent_ddl_dml/s2'; <waiting ...>
+step s2_detach_part: <... completed>
+ERROR: canceling statement due to user request
+step s1_cancel_s2: <... completed>
+pg_cancel_backend
+-----------------
+t
+(1 row)
+
+step s1_commit: COMMIT;
+step s1_create_pub: CREATE PUBLICATION pub FOR ALL TABLES;
+step s1_insert_part: INSERT INTO tbl_part_p1 VALUES (1);
+step s2_get_binary_changes: SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub');
+?column?
+--------
+t
+(1 row)
+
+?column?
+--------
+stop
+(1 row)
+
diff --git a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
index d16515e6f48..e33a614c2e0 100644
--- a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
+++ b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
@@ -2,12 +2,17 @@ setup
{
DROP TABLE IF EXISTS tbl1;
DROP TABLE IF EXISTS tbl2;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
CREATE TABLE tbl1(val1 integer, val2 integer);
CREATE TABLE tbl2(val1 integer, val2 integer);
+ CREATE TABLE tbl_part(val1 integer) PARTITION BY LIST (val1);
+ CREATE TABLE tbl_part_p1 PARTITION OF tbl_part FOR VALUES IN (1);
}
teardown
{
+ DROP PUBLICATION IF EXISTS pub;
+ DROP TABLE IF EXISTS tbl_part, tbl_part_p1;
DROP TABLE tbl1;
DROP TABLE tbl2;
SELECT 'stop' FROM pg_drop_replication_slot('isolation_slot');
@@ -17,10 +22,15 @@ session "s1"
setup { SET synchronous_commit=on; }
step "s1_init" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'test_decoding'); }
+step "s1_init_pgoutput" { SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'pgoutput'); }
step "s1_begin" { BEGIN; }
step "s1_insert_tbl1" { INSERT INTO tbl1 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2" { INSERT INTO tbl2 (val1, val2) VALUES (1, 1); }
step "s1_insert_tbl2_3col" { INSERT INTO tbl2 (val1, val2, val3) VALUES (1, 1,
1); }
+step "s1_select_part" { SELECT * FROM tbl_part; }
+step "s1_cancel_s2" { SELECT pg_cancel_backend(pid) FROM pg_stat_activity
WHERE application_name = 'isolation/concurrent_ddl_dml/s2'; }
+step "s1_create_pub" { CREATE PUBLICATION pub FOR ALL TABLES; }
+step "s1_insert_part" { INSERT INTO tbl_part_p1 VALUES (1); }
step "s1_commit" { COMMIT; }
session "s2"
@@ -43,8 +53,10 @@ step "s2_alter_tbl2_drop_3rd_col" { ALTER TABLE tbl2 DROP
COLUMN val3; }
step "s2_alter_tbl2_3rd_char" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE
character varying; }
step "s2_alter_tbl2_3rd_text" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE text; }
step "s2_alter_tbl2_3rd_int" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE int
USING val3::integer; }
+step "s2_detach_part" { ALTER TABLE tbl_part DETACH PARTITION tbl_part_p1
CONCURRENTLY; }
step "s2_get_changes" { SELECT data FROM
pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0',
'skip-empty-xacts', '1'); }
+step "s2_get_binary_changes" { SELECT count(*) > 0 FROM
pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'pub'); }
@@ -86,3 +98,8 @@ permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin"
"s1_insert_tbl1" "s2_a
permutation "s1_init" "s2_alter_tbl2_add_text" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_3rd_char" "s1_insert_tbl2_3col" "s1_commit"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl2" "s2_get_changes"
permutation "s1_init" "s2_alter_tbl2_add_char" "s1_begin" "s1_insert_tbl1"
"s2_alter_tbl2_drop_3rd_col" "s1_insert_tbl1" "s1_commit" "s2_get_changes"
+
+# Verify that pgoutput can decode changes to a partition whose concurrent
+# detach was canceled, when the relation is still marked as a partition in
+# pg_class but already reports no ancestors.
+permutation "s1_init_pgoutput" "s1_begin" "s1_select_part" "s2_detach_part"
"s1_cancel_s2"("s2_detach_part") "s1_commit" "s1_create_pub" "s1_insert_part"
"s2_get_binary_changes"
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 28f3cade6ff..5efa7d7fa84 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -126,6 +126,11 @@ get_partition_parent_worker(Relation inhRel, Oid relid,
bool *detach_pending)
* The first element is the immediate parent and the last one is the topmost
* parent in the partition hierarchy.
*
+ * A partition pending concurrent detach is treated as having no parent. The
+ * list can therefore be empty even if the relation is still marked as a
+ * partition in pg_class. If an ancestor is pending detach, the list ends at
+ * that ancestor.
+ *
* Note: Because this function assumes that the relation whose OID is passed
* as an argument and each ancestor will have precisely one parent, it should
* only be called when it is known that the relation is a partition.
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index c8caa163c15..0af841c636c 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2103,9 +2103,19 @@ 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;
+
+ /*
+ * A partition whose concurrent detach has been committed but
not
+ * finalized reports no ancestors, even though relispartition
is still
+ * set. Determine publication membership and identity as for a
+ * standalone table: changes are no longer published via its
former
+ * ancestors, but can still be published via its own
publications.
+ */
+ if (get_rel_relispartition(relid))
+ ancestors = get_partition_ancestors(relid);
/* Reload publications if needed before use. */
if (!publications_valid)
@@ -2211,9 +2221,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
{
List *exceptpubids = NIL;
- if (am_partition)
+ if (ancestors)
{
- List *ancestors =
get_partition_ancestors(relid);
Oid
last_ancestor_relid = llast_oid(ancestors);
/*
@@ -2262,11 +2271,10 @@ 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;
- List *ancestors =
get_partition_ancestors(relid);
ancestor =
GetTopMostAncestorInPublication(pub->oid,
ancestors,
@@ -2364,6 +2372,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
list_free(pubids);
list_free(schemaPubids);
list_free(rel_publications);
+ list_free(ancestors);
entry->replicate_valid = true;
}
--
2.54.0 (Apple Git-157)
