RE: Added schema level support for publication.
Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: vignesh C <vignesh21@gmail.com>
Cc: "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Greg Nancarrow <gregn4422@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter
Eisentraut <peter.eisentraut@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Smith <smithpb2250@gmail.com>, Ajin Cherian <itsajin@gmail.com>, Rahila
Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2021-10-13T02:48:37Z
Lists: pgsql-hackers
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
Commits
-
Include schema/table publications even with exclude options in dump.
- 4aa6fa3cd0a2 18.0 landed
-
Rename some enums to use TABLE instead of REL.
- b3812d0b9bcf 15.0 landed
-
Add tap tests for the schema publications.
- 6b0f6f79eef2 15.0 landed
-
Allow publishing the tables of schema.
- 5a2832465fd8 15.0 landed
-
In pg_dump, use simplehash.h to look up dumpable objects by OID.
- 92316a4582a5 15.0 cited