Re: Support EXCEPT for TABLES IN SCHEMA publications

On Fri, May 15, 2026 at 12:49 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Nisha.
>
> Some review comments for patch v5-0001.

Thanks Peter for the review.

>
> ~~~
>
> GetIncludedPublicationRelations:
>
> 6.
> - * This should only be used FOR ALL TABLES publications.
> + * This is used for FOR ALL TABLES and FOR TABLES IN SCHEMA publications,
> + * both of which support EXCEPT TABLE.
>
> So, because it might come from 2 places, shouldn't the assert in this
> function be modified?
>
I think you meant GetExcludedPublicationTables.
Patch-0003 updates the assert in this fuction for schema publications.

> Also, since the assert was not yet modified, how does this even pass
> the tests if 'alltables' was false?
>

As you pointed out in the next (7th) comment also, the first two
patches are not calling GetExcludedPublicationTables(), but are using
get_publication_relations() directly. Hence, the tests are passing
even without the assert modification. But patch-0003 does call it, so
the assert was updated there.

> ~~~
>
> GetAllSchemaPublicationRelations:
>
> 7.
> + /* get the list of tables excluded via EXCEPT TABLE for this publication */
> + if (pubschemalist != NIL)
> + exceptlist = get_publication_relations(pubid, pub_partopt, true);
> +
>
> Should this be calling 'GetExcludedPublicationTables' instead of
> directly calling get_publication_relations?
>

Agreed, I will make the change. With that, the above mentioned assert
will also need to be updated in patch-1 itself.
~~~~

I agree with the rest of the comments and will update and upload the
patches soon.

--
Thanks,
Nisha