On Thu, Feb 26, 2026 at 3:22 PM shveta malik <[email protected]> wrote:
>
> Few more comments on v50-001:
>
> 9)
> + * pub1:
> + * FOR ALL TABLES EXCEPT (part1, part2)
> + * WITH (publish_via_partition_root = true)
>
> This example atop check_publications_except_list() needs to be changed
> as now we can not specify partitions in EXCEPT.
>
Now that we don't support partitions in EXCEPT clauses, won't it be
possible to allow combining two publications with except clauses?
Few other comments:
=================
1.
+/*
+ * Get the list of publication oids associated with a specified relation.
+ *
+ * 'pubids' returns the Oids of the publications the relation is part of.
+ *
+ * 'except_pubids' returns the Oids of publications the relation is excluded
+ * from.
+ *
+ * This function returns true if the relation is part of any publication.
+ */
+bool
+GetRelationPublications(Oid relid, List **pubids, List **except_pubids)
Isn't it better to have two wrapper functions for included and
excluded RelationPublications? The current API signature doesn't look
clean to me as it combines two different things.
2.
The following is from 0001
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -198,7 +198,12 @@ ObjectsInPublicationToOids(List *pubobjspec_list,
ParseState *pstate,
switch (pubobj->pubobjtype)
{
+ case PUBLICATIONOBJ_EXCEPT_TABLE:
+ pubobj->pubtable->except = true;
+ *rels = lappend(*rels, pubobj->pubtable);
+ break;
case PUBLICATIONOBJ_TABLE:
+ pubobj->pubtable->except = false;
*rels = lappend(*rels, pubobj->pubtable);
....
The following is from 0002:
static void
ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
- List **rels, List **schemas)
+ List **rels, List **exceptrels, List **schemas)
{
ListCell *cell;
PublicationObjSpec *pubobj;
@@ -200,7 +200,7 @@ ObjectsInPublicationToOids(List *pubobjspec_list,
ParseState *pstate,
{
case PUBLICATIONOBJ_EXCEPT_TABLE:
pubobj->pubtable->except = true;
- *rels = lappend(*rels, pubobj->pubtable);
+ *exceptrels = lappend(*exceptrels, pubobj->pubtable);
I think it is better to have a separate exceptlist from patch-1 itself
as that keeps code easier to understand for 0001.
3.
+$node_publisher->safe_psql('postgres',
+ "SELECT slot_name FROM pg_replication_slot_advance('test_slot',
pg_current_wal_lsn());"
+);
At above and other places where it uses slot_advance, write a comment
to explain why the same is required.
4.
+ # When the root partitioned table is listed in the EXCEPT clause,
+ # all its partitions are not published.
+ $node_publisher->safe_psql(
Can we change the comment to the following: "If the root partitioned
table is in the EXCEPT clause, all its partitions are excluded from
publication, regardless of the publish_via_partition_root setting."?
--
With Regards,
Amit Kapila.
