On Sun, Mar 1, 2026 at 8:41 AM vignesh C <[email protected]> wrote:
>
>
> The attached v53 version patch has the changes for the same.
>

Thanks. Please find a few comments on 001:

1)
pg_get_publication_sequences():
- sequences = GetAllPublicationRelations(RELKIND_SEQUENCE, false);
+ sequences = GetAllPublicationRelations(publication, RELKIND_SEQUENCE);

This could be problematic. Earlier we were passing 'false' always for
sequences, now we are letting it extracted from publication which can
be true as well and thus we may hit Assert inside
GetAllPublicationRelations() :

Assert(!(relkind == RELKIND_SEQUENCE && pubviaroot));

As an example, this will assert:
create sequence seq1;
create publication pub1 for all sequences with
(publish_via_partition_root=true);
select * from pg_publication_sequences ;

2)
get_rel_sync_entry():

+ if (pub->pubviaroot)
+ {
+ pub_relid = llast_oid(ancestors);
+ ancestor_level = list_length(ancestors);
+ }
+ else
+ {
+ /*
+ * When publishing partitions individually, we must
+ * still check whether the top-most ancestor is
+ * excluded. If the root is excluded, none of its
+ * partitions should be published.
+ */
+ ancestorexceptpubids = GetRelationExcludedPublications(llast_oid(ancestors));
+ }
+
+ exceptpubids = GetRelationExcludedPublications(pub_relid);

Bot pub->pubviaroot=On/OFF will ultimately end up doing these 2 steps:

---First get llast_oid(ancestors)
-- Then execute GetRelationExcludedPublications().  (outside if-block
but inside else-block)

Why do we need an if-else block at all, as both are doing the same steps?

And why do we need 2 variables except pubids and ancestorexceptpubids?


3)
+bool
+publication_has_any_except_table(Oid pubid)

I do not see its usage anywhere now. Shall we remove it?

4)
+static List *
+GetRelationPublications(Oid relid, bool except_flag)

Since it is static function, we shall rename it to
get_relation_publications() similar to get_publication_relations()

5)
+/* Gets list of publication oids for a relation */
+List *
+GetRelationIncludedPublications(Oid relid)

+/* Gets list of publication oids which has relation in EXCEPT clause */
+List *
+GetRelationExcludedPublications(Oid relid)

We can make comment format multi-line similar to other functions in
this file. See GetAllTablesPublications, GetSchemaPublications etc.

~~
Reviewing further.

thanks
Shveta


Reply via email to