Re: Support EXCEPT for TABLES IN SCHEMA publications

On Thu, Jul 9, 2026 at 2:53 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> >
> > 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.
>

Each TABLES IN SCHEMA ... EXCEPT (...) entry stores its own nested
list of EXCEPT tables. Flatten means collecting the PublicationTable
from each nested list into a single top-level except_pubtables list,
giving it the same structure as a FOR TABLE list that can be passed
directly to OpenTableList().

I've reworded the comment. We can also remove it if it seems unnecessary 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.

Yes, the logic was introduced in patch 0001 itself. I just highlighted
it for the case you mentioned, where the error is deferred instead of
being raised at parse time.

> 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.
>

My thought was the same: to keep the logic in one place for now. Let's
see if others have any opinions on it as well.

> ~~
>
> 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?
>

Yes, that works for me as we have FOR TABLES example to follow. I've
made the changes in v20.

--
Thanks,
Nisha