On Thu, Feb 19, 2026 at 10:13 AM Shlok Kyal <[email protected]> wrote:
>
> Thanks for reviewing the patch. I have addressed the remaining
> comments in the v46 patch..
>
I am looking at the latest version some questions/comments
1.
GetRelationPublications()
{
..
for (int i = 0; i < pubrellist->n_members; i++)
{
+ if (pubrel->prexcept)
+ {
+ if (except_pubids)
+ *except_pubids = lappend_oid(*except_pubids, pubid);
+ }
+ else
+ {
+ if (pubids)
+ *pubids = lappend_oid(*pubids, pubid);
+ found = true;
+ }
}
Can we simplify this multi-level check to something simple by using
some temporary variable?
Suggestions:
for (int i = 0; i < pubrellist->n_members; i++)
{
List **target_list = pubrel->prexcept ? except_pubids : pubids;
if (target_list)
*target_list = lappend_oid(*target_list, pubid);
if (!pubrel->prexcept)
found = true;
}
2.
-extern List *GetPublicationRelations(Oid pubid, PublicationPartOpt
pub_partopt);
+extern List *GetIncludedRelationsByPub(Oid pubid,
+ PublicationPartOpt pub_partopt);
I don't really like renaming this function, IMHO the
GetPublicationRelations() means all the published relation, we don't
need to be explicit about *Included* relation. Then I see we have
another function to fetch the excluded relation i.e.
GetExcludedTablesByPub() so maybe this makes sense considering we have
a requirement for fetching included as well as excluded.
3. Question, what would be the behavior of
pg_get_publication_tables(), if we have created PUBLICATION FOR ALL
TABLES with EXCEPT, I assume it will give all table even in the EXCEPT
list as we have another function to get effective tables i.e.
pg_get_publication_effective_tables(), so my question is are we
planning to add an additional field in pg_get_publication_tables() to
explicitly mention that the table is EXCLUDED?
--
Regards,
Dilip Kumar
Google
