On Tue, Jul 21, 2026 at 11:12 AM shveta malik <[email protected]> wrote:
>
>
> A few more comments:
>
> 8)
> CreatePublication:
> + /*
> + * Collect explicit table OIDs now, before we close the relation
> + * list, so that except-table validation below can check for
> + * contradictions without relying on a catalog scan that might not
> + * yet see the just-inserted rows.
> + */
> + if (except_pubtables != NIL)
> + {
> + foreach_ptr(PublicationRelInfo, pri, rels)
> + explicitrelids = lappend_oid(explicitrelids,
> + RelationGetRelid(pri->relation));
> + }
> +
>   PublicationAddTables(puboid, rels, true, NULL);
>
> Shall we make the explicitrelids collection after
> 'PublicationAddTables', as PublicationAddTables may error out too
> while doing 'check_publication_add_relation'.
>

make sense. Updated.

> 9)
> In the check above, IIUC, except_pubtables can be there even for ALL
> TABLES, but we want this handling only for schema-except list, so
> shall we have the check as:
>
> if (schemaidlist != NIL && except_pubtables != NIL)
>

Here FOR ALL TABLES EXCEPT is already handled in the for_all_tables
branch, so it can't reach this code.

The only possible cases here are FOR TABLE and FOR TABLES IN SCHEMA.
Since EXCEPT is not supported with plain FOR TABLE, except_pubtables
!= NIL already implies the schema case.

That said, should we add an assertion here, e.g.:

if (except_pubtables != NIL)
{
    /* EXCEPT is only accepted with TABLES IN SCHEMA */
    Assert(schemaidlist != NIL);
...

Or please correct me if I've misunderstood something.

--
Thanks,
Nisha


Reply via email to