Re: Support EXCEPT for ALL SEQUENCES publications

[shveta malik]

On Wed, Jul 8, 2026 at 5:38 PM Shlok Kyal <shlok.kyal....@gmail.com> wrote:
>
> I have addressed the comments and attached the updated v20 patch.
>

Thanks Shlok. Please find a few comments:

1)

+ /*
+ * Must be a regular or partitioned table when specified in FOR TABLE or
+ * EXCEPT table list
+ */

EXCEPT table list-->EXCEPT (TABLE ...) clause

+ /* Must be a sequence if specified in EXCEPT sequence list */

EXCEPT sequence list-->EXCEPT (SEQUENCE ...) clause

2)
+
+ /*
+ * TODO: EXCEPT (SEQUENCE ...) is not yet supported with ALTER
+ * PUBLICATION.
+ */
+ Assert(exceptseqs == NIL);

We can remove 'TODO' as it is a conscious decision to not support this
in patch001

3)
Why patch002 needs this change? And is itg onlyh for 002 and not needed by 001?

 get_publication_relations(Oid pubid, PublicationPartOpt pub_partopt,
-   bool except_flag)
+   bool except_flag, char pubrelkind)

- result = GetPubPartitionOptionRelations(result, pub_partopt,
- pubrel->prrelid);
+ {
+ char relkind = get_rel_relkind(pubrel->prrelid);
+
+ if ((pubrelkind == RELKIND_RELATION &&
+ (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)) ||
+ (pubrelkind == RELKIND_SEQUENCE && relkind == RELKIND_SEQUENCE))
+ result = GetPubPartitionOptionRelations(result, pub_partopt,
+ pubrel->prrelid);

4)
It is not clear why 002 has made this change? Why the behaviour is
different for ALL Tables and ALL Seqeunces.


- * This should only be used FOR TABLE publications, the FOR ALL
TABLES/SEQUENCES
- * should use GetAllPublicationRelations().
+ * This is mainly used for FOR TABLE publications and must not be called for
+ * ALL TABLES publications. For ALL SEQUENCES publications, the result is an
+ * empty list.
  */
 List *
 GetIncludedPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt)


5)
AlterPublicationRelations:

Shall we add this Assert in else block as except-sequences must be
NULL if this is case of non all-seq pub.

if (stmt->for_all_tables || stmt->for_all_sequences)
{
....
}
else
{
Assert(!sequences)
oldrelids = GetIncludedPublicationRelations .....
}


thanks
Shveta