| From: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, "ashutosh(dot)bapat(dot)oss(at)gmail(dot)com" <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Crashes on a partition whose concurrent detach never finished |
| Date: | 2026-09-22 11:01:10 |
| Message-ID: | CABdArM7Bc7SZPSHk5uT-yA6=DYVFW-Os4noB6CANH6GQw9rvew@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Sep 22, 2026 at 2:31 AM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> Hello!
>
> An automated claude review found a residual crash in pgoutput after
> the committed fix, which repeatedly crashes the walsender following an
> interrupted ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY.
>
> 0001 contains a test case and a simple fix.
>
Thanks for looking into this.
The issue that the 0001 patch you shared is trying to fix is the same
one reported upthread [1] in the v1-0001 patch.
The fix is still under discussion and is not a PG19 regression. We
need a detailed analysis of the expected behavior for detached-pending
partitions requiring relcache invalidation and its side-effects,
please see discussions [2],[3].
> And after further looking, I realized that there's one more leftover
> issue, which doesn't cause a crash but failures in subscribers:
> pg_get_publication_tables only relied on relispartition instead of
> following the same approach as the previous fix commit and 0001. 0002
> modifies it to match the other locations.
Good catch!
Here is my analysis of the 0002 fix:
It is a pre-existing bug, not an EXCEPT TABLE feature regression.
0002 fixes one thing: a detach-pending partition is skipped under
publish_via_partition_root = true, with or without an EXCEPT clause.
EXCEPT just makes it easy to reproduce; I think the core problem is
how the publishable decision is taken when pubviaroot = true.
Two parts:
1) GetAllPublicationRelations(): pre-existing, reachable since v14
Example: table t_part has t_part_p1, t_part_p2; t_part_p2 is detach-pending:
CREATE PUBLICATION pub_root FOR ALL TABLES WITH
(publish_via_partition_root = true);
CREATE PUBLICATION pub_leaf FOR ALL TABLES;
-- before (head):
postgres=# SELECT pubname, tablename FROM pg_publication_tables
WHERE tablename LIKE 't_part%';
pubname | tablename
----------+-----------
pub_root | t_part
pub_leaf | t_part_p1
pub_leaf | t_part_p2
-- after (0002 fix):
postgres=# SELECT pubname, tablename FROM pg_publication_tables
WHERE tablename LIKE 't_part%';
pubname | tablename
----------+-----------
pub_root | t_part_p2
pub_root | t_part
pub_leaf | t_part_p2
pub_leaf | t_part_p1
-- pub_root does not list t_part_p2, although the decoding side
publishes it under its own name as we treat a detach-pending partition
as a standalone table everywhere else.
2) is_table_publishable_in_publication(): new function in v19, added by -
```
commit fd7a25af11e2cad4f48ffc4e50f18644e657ed53
Author: Masahiko Sawada <msawada(at)postgresql(dot)org>
Date: Thu Apr 2 11:34:50 2026 -0700
Add target_relid parameter to pg_get_publication_tables().
```
2a) for ALL TABLES, is the detach-pending partition publishable?
-- before (head)
postgres=# SELECT * FROM pg_get_publication_tables(ARRAY['pub_root'],
't_part_p2'::regclass::oid);
pubid | relid | attrs | qual
-------+-------+-------+------
(0 rows)
postgres=# SELECT * FROM pg_get_publication_tables(ARRAY['pub_leaf'],
't_part_p2'::regclass::oid);
pubid | relid | attrs | qual
-------+-------+-------+------
16479 | 16475 | 1 |
-- after (0002 fix)
postgres=# SELECT * FROM pg_get_publication_tables(ARRAY['pub_root'],
't_part_p2'::regclass::oid);
pubid | relid | attrs | qual
-------+-------+-------+------
24581 | 16392 | 1 |
postgres=# SELECT * FROM pg_get_publication_tables(ARRAY['pub_leaf'],
't_part_p2'::regclass::oid);
pubid | relid | attrs | qual
-------+-------+-------+------
24582 | 16392 | 1 |
-- pub_root reports the detach-pending partition as not publishable
(0 rows), while pub_leaf reports it.
2b) For Non-ALL-TABLES publications, the behavior was already correct;
the change below is a no-op:
- if (relispartition &&
+ if (ancestors &&
OidIsValid(GetTopMostAncestorInPublication(pub->oid,
ancestors, NULL)))
return !pub->pubviaroot;
For a detach-pending partition ancestors is empty, so
GetTopMostAncestorInPublication() already returned InvalidOid and the
branch was already not taken. But we can consider the change for
clarity/cost improvement.
~~~
[1] https://www.postgresql.org/message-id/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAJpy0uA6womAP7fkmT%3DsSfKG_0CfKUSb7HhP_J9mw071MXc39w%40mail.gmail.com
[3] https://www.postgresql.org/message-id/CABdArM4fJ8JAWyP0MAF%3D8n66M6%3DCouUmtSGwqizn2pD%3DgfPEZw%40mail.gmail.com
Thanks,
Nisha
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-22 11:23:54 | Re: Add a permission check to pg_stat_get_backend_subxact() |
| Previous Message | Michael Paquier | 2026-09-22 10:49:03 | Re: Internal error codes triggered by regression tests and user queries, take 2 |