| From: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Date: | 2026-07-16 05:44:01 |
| Message-ID: | CABdArM64ZSat=dB094U8OCBzp1m8_Y85m6T+xPEgpLVJb60_rA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jul 16, 2026 at 10:15 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Wed, Jul 15, 2026 at 10:16 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 13, 2026 at 3:31 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Jul 10, 2026 at 5:07 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> > > >
> > > >
> > > > Yes, that works for me as we have FOR TABLES example to follow. I've
> > > > made the changes in v20.
> > > >
> > >
> > > Thanks! I had a quick look, few initial comments:
> > >
> > >
> > > 1)
> > > GetTopMostAncestorInPublication():
> > >
> > > + if (ancestors == NIL)
> > > + return InvalidOid;
> > >
> > > Should this be Assert or can there be a case where ancestors list can
> > > come as NIL?
> > >
> >
> > I found one case where ancestors can be NIL.
> > Test case:
> > CREATE TABLE t1 (a int) PARTITION BY RANGE (a);
> > CREATE TABLE t1_part PARTITION OF t1 FOR VALUES FROM (1) TO (100);
> > CREATE PUBLICATION mypub FOR TABLE t1;
> >
> > -- Get the partition into the pending-detach state:
> > -- Session A:
> > BEGIN;
> > SELECT * FROM t1;
> > -- leave open
> >
> > -- Session B (blocks, waiting on Session A's lock on t1):
> > ALTER TABLE t1 DETACH PARTITION t1_part CONCURRENTLY;
> >
> > -- Session C, while B is blocked:
> > SELECT * FROM pg_get_publication_tables('{mypub}'::text[],
> > 't1_part'::regclass);
> >
> > In this scenario, Session C reaches GetTopMostAncestorInPublication()
> > with ancestors == NIL. So I think we should keep return InvalidOid;
> > rather than replacing it with an Assert.
> >
> > While investigating this race, I also hit assertions at places that
> > call "llast_oid(ancestors)" without checking for NIL, for example in
> > check_publication_add_relation() and RelationBuildPublicationDesc(). I
> > fixed those by guarding with if (ancestors != NIL).
>
> Okay, I will review.
>
> > But due to this same race, other existing callers of llast_oid() on
> > HEAD that assume ancestors is never NIL can also hit the following
> > assertion:
> > "TRAP: failed Assert("list != NIL"), File:
> > "../../../../src/include/nodes/pg_list.h" ..."
> >
> > One such case is pgoutput.c:get_rel_sync_entry(). To reproduce on HEAD:
> > -- create a slot
> > SELECT pg_create_logical_replication_slot('myslot', 'pgoutput');
> >
> > -- get the partition into the pending-detach state (using same A & B
> > sessions), and in session C run -
> > INSERT INTO t1_part VALUES (5);
> > SELECT * FROM pg_logical_slot_get_binary_changes('myslot', NULL, NULL,
> > 'proto_version', '1', 'publication_names', 'mypub');
> >
> > I think this should be handled separately after analyzing whether
> > there's a better fix than simply adding if (ancestors != NIL) guards
> > at each call site.
> > Note: Other callers of llast_oid(ancestors) may need similar review.
> >
> > Thoughts?
> >
>
> Nisha, I am not able to reproduce this on HEAD. Can you please verify
> if you missed providing any steps? This I what I have tried:
>
> Session1:
> postgres=# SELECT pg_create_logical_replication_slot('myslot', 'pgoutput');
> pg_create_logical_replication_slot
> ------------------------------------
> (myslot,0/0178F5B8)
> (1 row)
>
> postgres=# CREATE TABLE t1 (a int) PARTITION BY RANGE (a);
> CREATE TABLE t1_part PARTITION OF t1 FOR VALUES FROM (1) TO (100);
> CREATE PUBLICATION mypub FOR TABLE t1;
> CREATE TABLE
> CREATE TABLE
> CREATE PUBLICATION
>
> Session2:
> postgres=# BEGIN;
> SELECT * FROM t1;
> BEGIN
> a
> ---
> (0 rows)
>
> Session3:
> postgres=# ALTER TABLE t1 DETACH PARTITION t1_part CONCURRENTLY;
> <hanging>
>
> Session4:
> postgres=# INSERT INTO t1_part VALUES (5);
> SELECT * FROM pg_logical_slot_get_binary_changes('myslot', NULL, NULL,
> 'proto_version', '1', 'publication_names', 'mypub');
> INSERT 0 1
> lsn | xid | data
> -----+-----+------
> (0 rows)
>
A zero-row result indicates the INSERT wasn't decoded, so seems
get_rel_sync_entry() wasn't reached. Sorry for the ambiguous test
steps -- the slot creation order matters.
Please follow these steps to reproduce the issue on HEAD:
Session1:
CREATE TABLE t1 (a int) PARTITION BY RANGE (a);
CREATE TABLE t1_part PARTITION OF t1 FOR VALUES FROM (1) TO (100);
CREATE PUBLICATION mypub FOR ALL TABLES;
SELECT pg_create_logical_replication_slot('myslot', 'pgoutput');
Session 2:
BEGIN;
SELECT * FROM t1;
-- leave open
Session 3 (hangs):
ALTER TABLE t1 DETACH PARTITION t1_part CONCURRENTLY;
Session 4, while 3 is blocked:
INSERT INTO t1_part VALUES (5);
SELECT * FROM pg_logical_slot_get_binary_changes('myslot', NULL,
NULL, 'proto_version', '1', 'publication_names', 'mypub');
--
Thanks,
Nisha
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-07-16 05:49:24 | Re: walsummarizer can get stuck when switching timelines |
| Previous Message | Srinath Reddy Sadipiralla | 2026-07-16 05:06:49 | Re: Fix "unexpected logical decoding status change" error; from concurrent logical decoding activation |