Re: Support EXCEPT for TABLES IN SCHEMA publications

On Tue, Jul 28, 2026 at 2:18 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Jul 23, 2026 at 5:26 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> >
> > Attached v23 patch set addressing all of the above comments, as well
> > as the comments in [1] and [2].
> >
>
> A few comments on v23-001:
>
> 1)
> + if (CheckPublicationRelEntry(pubid, root, &root_except) && root_except)
> + ereport(ERROR,
> + errcode(ERRCODE_DUPLICATE_OBJECT),
> + errmsg("cannot add partition \"%s\" to publication \"%s\"",
> + RelationGetQualifiedRelationName(targetrel),
>
> Is the errorcode "ERRCODE_DUPLICATE_OBJECT" intentional? To me it does
> not look duplicate object as ROOT was in EXCEPT while this
> we are trying to add to inclusion list. Other similar places have
> different error, see:
>
> + if (exceptrelid == explicitrelid)
> + ereport(ERROR,
> + errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("table \"%s\" cannot be both published and excluded",
> + RelationGetQualifiedRelationName(pri->relation)));
>
>
> 2)
> + * Also reject any relation in explicitrelids whose partition root is in
> + * except_rels. Otherwise the catalog would be inconsistent (root excluded,
> + * partition explicitly included), and the ancestor-cascade rule would
> + * silently override the include at replication time.
>
> Shall we simply say:
> /*
> * Also reject any relation in explicitrelids whose partition root is in
> * except_rels. Excluding a partition root means that the entire partition
> * tree is excluded, so a partition cannot be explicitly added to the
> * publication if its partition root is excluded.
> */
>
> 3)
> Shall we change this too
> + errdetail("The table is currently named in the EXCEPT clause of the
> publication."),
> similar to:
> + errdetail("Partition root \"%s\" is named in the publication's
> EXCEPT clause for schema \"%s\".",
>
>
> 4)
> +
> + /*
> + * ancestors is NIL for a partition with a pending DETACH
> + * CONCURRENTLY.
> + */
> + if (ancestors != NIL)
> + root = llast_oid(ancestors);
>
> I see non-null ancestors check at some places while other places do it
> directly, see:
> + root = llast_oid(get_partition_ancestors(relid));
>
> is it intentional?
>
> 5)
> + list_free(ancestors);
>
> This is also done at some places, while missed at other places.
>

Nisha, the changes in RelationBuildPublicationDesc() looks quite
tricky now. We are doing 'list_difference_oid' thrice:

--for schema-pub
--for schmea-pubs on complete ancestor tree
--for all-tables pub

Ultimately we are concatenating all individual pubids after
subtracting 'exceptpuboids' from each individual 'puboids' list. The
'exceptpuboids' is computed only once and doesn't change with each
computation. Why don't we uncondiitonally do list_concat_unique_oid
for all pub-ids
and only minus 'exceptpuboids' once at the end? Won't it work? Let me
know if I am missing something.

~~

I think, we can now merge 002 to 001, it will help in combined review
by placing except-validation logic where it should ultimately be.

thanks
Shveta