| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Nisha Moond <nisha(dot)moond412(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, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Date: | 2026-07-09 09:23:34 |
| Message-ID: | CAJpy0uDhBEc2Qkq_-v0ae==kB35BXYymB=OzdY-H71W6NY6ChQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Jul 8, 2026 at 8:50 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Tue, Jul 7, 2026 at 8:40 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Tue, Jul 7, 2026 at 4:31 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > Hi Nisha.
> > >
> > > Bug: The current patch is allowing conflicting exclusion lists...
> > >
> > > ======
> > >
> > > Background:
> > >
> > > It's always been possible for CREATE PUBLICATION to specify the same
> > > schema or same table multiple times.
> > >
> > > test_pub=# CREATE SCHEMA MYSCHEMA;
> > > CREATE SCHEMA
> > > test_pub=# CREATE TABLE T1(A INT);
> > > CREATE TABLE
> > > test_pub=# CREATE TABLE MYSCHEMA.T2(A INT);
> > > CREATE TABLE
> > > test_pub=# CREATE TABLE MYSCHEMA.T3(A INT);
> > > CREATE TABLE
> > >
> > > // same table multiple times is OK
> > > test_pub=# CREATE PUBLICATION PUB3 FOR TABLE T1,T1,T1,T1;
> > > CREATE PUBLICATION
> > >
> > > // same schema multiple times is OK
> > > test_pub=# CREATE PUBLICATION PUB1 FOR TABLES IN SCHEMA MYSCHEMA,
> > > MYSCHEMA, MYSCHEMA;
> > > CREATE PUBLICATION
> > >
> > > ~~~
> > >
> > > But, this behaviour is only allowed if there are no specification
> > > conflicts. e.g. You can't have multiple tables with column lists:
> > >
> > > test_pub=# CREATE PUBLICATION PUB4 FOR TABLE T1,T1(A),T1,T1;
> > > ERROR: conflicting or redundant column lists for table "t1"
> > >
> > > ======
> > >
> > > Bug:
> > >
> > > IMO similar restrictions and a similar error message needs to happen
> > > for clashing schema exclusions. Specifying the same schema multiple
> > > times is fine, but it must be disallowed if there are conflicting
> > > exclusion lists.
> > >
> >
> > +1. Good Catch.
> >
>
> Thanks for the bug report. I've fixed it and attached the fix as a
> separate top-up patch, v19-0002.
>
> Until v18, schema qualification checks for EXCEPT tables were split
> between two places:
>
> 1) TABLES IN SCHEMA <schema-name> (EXCEPT TABLE <table-name>);
> -- the schema qualification check was performed in gram.y, since all
> required information was available during parsing.
>
> 2) TABLES IN SCHEMA CURRENT_SCHEMA (EXCEPT TABLE <table-name>);
> -- CURRENT_SCHEMA is resolved only at execution time, so the schema
> qualification and eligibility checks were performed there.
>
> The reported bug also affects cases such as:
> CREATE PUBLICATION PUB2 FOR TABLES IN SCHEMA MYSCHEMA EXCEPT (TABLE
> T2), CURRENT_SCHEMA EXCEPT (TABLE T3);
> -- where CURRENT_SCHEMA = myschema. Correct handling of this case
> requires the checks to be performed after schema resolution, i.e., at
> execution time. To reduce duplicate code, I moved the schema
> qualification and eligibility checks for both cases to execution time
> and removed the corresponding processing from gram.y.
>
> Since this changes a fair amount of code, I've kept it as a separate
> patch (v19-0002) to make review easier. Depending on the feedback, we
> can optimize the implementation or move parts of it back into gram.y
> before merging it into patch 0001.
> ~~~
>
Nisha,
- * Also flattens except_tables from TABLES IN SCHEMA nodes into the list so
- * that ObjectsInPublicationToOids() sees them as top-level
EXCEPT_TABLE entries.
+ * The except_tables attached to TABLES IN SCHEMA nodes are left in place;
+ * ObjectsInPublicationToOids() qualifies names, validates schema membership,
+ * and flattens them once the schema OID is known.
Can you please explain what does 'flatten' mean here.
The flatten logic here is added by your patch001 only, it is not an
existing logic. To me (in my initial round of review), it seems okay
if we moved it to ObjectsInPublicationToOids(). The only thing is that
the one error (table in EXCEPT clause does not belong to schema) which
is possible to get at the parsing stage will now be emitted during
pub-creation stage. But since it will keep the entire logic of
except-list verification at one place and considering that
creat/alter-pub is not so frequent operation, I am fine with it. But
lets see if others have any comment here.
~~
I had a quick look at the changes:
+ * Include the ONLY-ness of this entry in its signature. EXCEPT
+ * (TABLE parent) excludes parent and its inheritance children,
+ * while EXCEPT (TABLE ONLY parent) excludes only parent itself.
I think this will unconditionally apply to paritioned tables as well
but for paritioned table, EXCEPT doc says that ONLY and * has no
effect, but here it shows the effect:
Thsi gives error:
postgres=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public EXCEPT
(TABLE ONLY tab_root), TABLES IN SCHEMA public EXCEPT (TABLE
tab_root);
ERROR: 42P17: schema "public" specified with conflicting EXCEPT lists
LOCATION: ProcessSchemaExceptTables, publicationcmds.c:299
While this does not:
postgres=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public EXCEPT
(TABLE tab_root), TABLES IN SCHEMA public EXCEPT (TABLE tab_root);
CREATE PUBLICATION
But I also checked column-list errors and found that even if columns
are exact sames, we get error:
postgres=# CREATE PUBLICATION PUB4 FOR TABLE T1(i), T1(i);
ERROR: 42710: conflicting or redundant column lists for table "t1"
LOCATION: OpenTableList, publicationcmds.c:2312
While this works:
postgres=# CREATE PUBLICATION PUB4 FOR TABLE T1, T1;
CREATE PUBLICATION
This makes me think that even introducing the smartness of identifying
ONLY in case of inherited tables and distinguishing it from
parititoned tables is not required. We can just change your error
message to 'conflicting or redundant EXCEPT lists for schmea "s1"' and
throw error in all cases where schmea with except list repeats without
actually comparing them. Does this sound reasonable?
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ashutosh Bapat | 2026-07-09 09:40:48 | Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() |
| Previous Message | Dilip Kumar | 2026-07-09 09:15:09 | Re: Proposal: Conflict log history table for Logical Replication |