On Tuesday, October 12, 2021 9:15 PM vignesh C <vignesh21@gmail.com>
> Attached v40 patch has the fix for the above comments.
Thanks for the update, I have some minor issues about partition related behavior.
1)
Tang tested and discussed this issue with me.
The testcase is:
We publish a schema and there is a partition in the published schema. If
publish_via_partition_root is on and the partition's parent table is not in the
published schema, neither the change on the partition nor the parent table will
not be published.
But if we publish by FOR TABLE partition and set publish_via_partition_root to
on, the change on the partition will be published. So, I think it'd be better to
publish the change on partition for FOR ALL TABLES IN SCHEMA case if its parent table
is not published in the same publication.
It seems we should pass publication oid to the GetSchemaPublicationRelations()
and add some check like the following:
if (is_publishable_class(relid, relForm) &&
!(relForm->relispartition && pub_partopt == PUBLICATION_PART_ROOT))
result = lappend_oid(result, relid);
+ if (relForm->relispartition && pub_partopt == PUBLICATION_PART_ROOT)
+ {
+ bool skip = false;
+ List *ancestors = get_partition_ancestors(relid);
+ List *schemas = GetPublicationSchemas(pubid);
+ ListCell *lc;
+ foreach(lc, ancestors)
+ {
+ if (list_member_oid(schemas, get_rel_namespace(lfirst_oid(lc))))
+ {
+ skip = true;
+ break;
+ }
+ }
+ if (!skip)
+ result = lappend_oid(result, relid);
+ }
2)
+ /*
+ * It is quite possible that some of the partitions are in a different
+ * schema than the parent table, so we need to get such partitions
+ * separately.
+ */
+ scan = table_beginscan_catalog(classRel, keycount, key);
+ while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+ {
+ Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
+
+ if (is_publishable_class(relForm->oid, relForm))
+ {
+ List *partitionrels = NIL;
+
+ partitionrels = GetPubPartitionOptionRelations(partitionrels,
+ pub_partopt,
+ relForm->oid);
I think a partitioned table could also be a partition which should not be
appended to the list. I think we should also filter these cases here by same
check in 1).
Thoughts ?
Best regards,
Hou zj
