RE: Added schema level support for publication.

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Smith <smithpb2250(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: RE: Added schema level support for publication.
Date: 2021-10-13 02:48:37
Message-ID: OS0PR01MB5716F1E1A6AA82F66F26295094B79@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tuesday, October 12, 2021 9:15 PM vignesh C <vignesh21(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-10-13 02:49:28 Re: Feature Request: Allow additional special characters at the beginning of the name.
Previous Message Bossart, Nathan 2021-10-13 02:20:29 Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable