Hi Vignesh,
A couple of review comments for v2-0001
======
src/backend/catalog/pg_publication.c
pg_get_publication_tables:
1.
if (pub_elem->alltables)
pub_elem_tables = GetAllPublicationRelations(RELKIND_RELATION,
pub_elem->pubviaroot);
- else
+ else if (!pub_elem->allsequences)
{
List *relids,
*schemarelids;
@@ -1203,8 +1203,13 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
table_infos = lappend(table_infos, table_info);
}
- /* At least one publication is using publish_via_partition_root. */
- if (pub_elem->pubviaroot)
+ /*
+ * At least one publication is using publish_via_partition_root.
+ * Skip sequences only publications, as publish_via_partition_root
+ * is applicable only to table publications.
+ */
+ if (pub_elem->pubviaroot && !PUB_HAS_SEQUENCES_ONLY(pub_elem->allsequences,
+ pub_elem->alltables))
viaroot = true;
Won't it be simpler to check this up-front and then just 'continue'?
Then you wouldn't have to handle "sequence only" for the rest of the
loop logic.
e.g.
pub_elem = ...
/* Skip this publication if no TABLES are published. */
if (PUB_HAS_SEQUENCES_ONLY(pub_elem->allsequences, pub_elem->alltables)
continue;
if (pub_elem->alltables)
...
else
...
======
src/backend/commands/publicationcmds.c
2.
- if (!pubform->puballtables && publish_via_partition_root_given &&
- !publish_via_partition_root)
+ if (!pubform->puballtables && !pubform->puballsequences &&
+ publish_via_partition_root_given && !publish_via_partition_root)
I felt this modified condition ought to be expressed as:
if (!PUB_HAS_SEQUENCES_ONLY(...) && <original condition>
======
Kind Regards,
Peter Smith.
Fujitsu Australia
