MIME-Version: 1.0
References: <CALDaNm0EA4zuR+PiNmADGORcADQsPSmYKo=tr4hT=pSrLY1B9A@mail.gmail.com>
 <202502050844.4lrgg3aoudat@alvherre.pgsql> <CANhcyEXxjq9U7BXxCba3Njz+eHpNzAsSVY2GzbV=8iy5j=UAUA@mail.gmail.com>
 <CALDaNm2Q1rNN-L8u95pjVRw-aBJkcGJh2bHeaPc2bO=mj_1QMA@mail.gmail.com> <CANhcyEVbFMWnEkShq37SSgW7+X+xOxx9bvc4Jtj6BgAy9jGnqA@mail.gmail.com>
In-Reply-To: <CANhcyEVbFMWnEkShq37SSgW7+X+xOxx9bvc4Jtj6BgAy9jGnqA@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Thu, 13 Feb 2025 20:11:48 +0530
Message-ID: <CALDaNm3bULhtCaaBmdsun0LnHx9Jp0v228kjd=ZujEaaCEOmAA@mail.gmail.com>
Subject: Re: Restrict publishing of partitioned table with a foreign table as partition
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@alvh.no-ip.org>, 
	Sergey Tatarintsev <s.tatarintsev@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="00000000000083a849062e070f9b"
Archived-At: <https://www.postgresql.org/message-id/CALDaNm3bULhtCaaBmdsun0LnHx9Jp0v228kjd%3DZujEaaCEOmAA%40mail.gmail.com>
Precedence: bulk

--00000000000083a849062e070f9b
Content-Type: text/plain; charset="UTF-8"

On Thu, 13 Feb 2025 at 15:50, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
>
> I have fixed the issue. Attached the updated v6 patch.

There is another concurrency issue:
In case of create publication for all tables with
publish_via_partition_root we will call check_foreign_tables:
@@ -876,6 +876,10 @@ CreatePublication(ParseState *pstate,
CreatePublicationStmt *stmt)
        /* Associate objects with the publication. */
        if (stmt->for_all_tables)
        {
+               /* Check if any foreign table is a part of partitioned table */
+               if (publish_via_partition_root)
+                       check_foreign_tables(stmt->pubname);

At the time of check in check_foreign_tables, there are no foreign
tables so this check will be successful:
+check_foreign_tables_in_schema(Oid schemaid, char *pubname)
+{
+       Relation        classRel;
+       ScanKeyData key[2];
+       TableScanDesc scan;
+       HeapTuple       tuple;
+
+       classRel = table_open(RelationRelationId, AccessShareLock);
+
+       ScanKeyInit(&key[0],
+                               Anum_pg_class_relnamespace,
+                               BTEqualStrategyNumber, F_OIDEQ,
+                               schemaid);
+       ScanKeyInit(&key[1],
+                               Anum_pg_class_relkind,
+                               BTEqualStrategyNumber, F_CHAREQ,
+                               CharGetDatum(RELKIND_PARTITIONED_TABLE));
+
+       scan = table_beginscan_catalog(classRel, 2, key);
+       while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)

Now immediately after execution of this, create a foreign table:
postgres=# CREATE FOREIGN TABLE part22 PARTITION OF part2 FOR VALUES
FROM (10) TO (15) SERVER fdw;
CREATE FOREIGN TABLE

And then continue execution of create publication, it will also be successful:
postgres=# create publication pub1 for all tables with (
publish_via_partition_root =true);
CREATE PUBLICATION

One probable way to fix this is to do the search similar to
check_foreign_tables_in_schema where we can skip including schemaid
key for all tables.

How about something like the attached patch.

Regards,
Vignesh

--00000000000083a849062e070f9b
Content-Type: text/x-patch; charset="US-ASCII"; name="Concurrency_all_tables_issue.patch"
Content-Disposition: attachment; 
	filename="Concurrency_all_tables_issue.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m73g7a9b0>
X-Attachment-Id: f_m73g7a9b0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2NhdGFsb2cvcGdfcHVibGljYXRpb24uYyBiL3NyYy9i
YWNrZW5kL2NhdGFsb2cvcGdfcHVibGljYXRpb24uYwppbmRleCA1MjBmYTJmMzgyLi4zMjE0MTA0
ZGVjIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9jYXRhbG9nL3BnX3B1YmxpY2F0aW9uLmMKKysr
IGIvc3JjL2JhY2tlbmQvY2F0YWxvZy9wZ19wdWJsaWNhdGlvbi5jCkBAIC0xMzg4LDIxICsxMzg4
LDI0IEBAIGNoZWNrX2ZvcmVpZ25fdGFibGVzX2luX3NjaGVtYShPaWQgc2NoZW1haWQsIGNoYXIg
KnB1Ym5hbWUpCiB7CiAJUmVsYXRpb24JY2xhc3NSZWw7CiAJU2NhbktleURhdGEga2V5WzJdOwor
CWludCAJCWtleWNvdW50ID0gMDsKIAlUYWJsZVNjYW5EZXNjIHNjYW47CiAJSGVhcFR1cGxlCXR1
cGxlOwogCiAJY2xhc3NSZWwgPSB0YWJsZV9vcGVuKFJlbGF0aW9uUmVsYXRpb25JZCwgQWNjZXNz
U2hhcmVMb2NrKTsKIAotCVNjYW5LZXlJbml0KCZrZXlbMF0sCi0JCQkJQW51bV9wZ19jbGFzc19y
ZWxuYW1lc3BhY2UsCi0JCQkJQlRFcXVhbFN0cmF0ZWd5TnVtYmVyLCBGX09JREVRLAotCQkJCXNj
aGVtYWlkKTsKLQlTY2FuS2V5SW5pdCgma2V5WzFdLAorCVNjYW5LZXlJbml0KCZrZXlba2V5Y291
bnQrK10sCiAJCQkJQW51bV9wZ19jbGFzc19yZWxraW5kLAogCQkJCUJURXF1YWxTdHJhdGVneU51
bWJlciwgRl9DSEFSRVEsCiAJCQkJQ2hhckdldERhdHVtKFJFTEtJTkRfUEFSVElUSU9ORURfVEFC
TEUpKTsKLQotCXNjYW4gPSB0YWJsZV9iZWdpbnNjYW5fY2F0YWxvZyhjbGFzc1JlbCwgMiwga2V5
KTsKKwkKKwlpZiAoT2lkSXNWYWxpZChzY2hlbWFpZCkpCisJCVNjYW5LZXlJbml0KCZrZXlba2V5
Y291bnQrK10sCisJCQkJCQlBbnVtX3BnX2NsYXNzX3JlbG5hbWVzcGFjZSwKKwkJCQkJCUJURXF1
YWxTdHJhdGVneU51bWJlciwgRl9PSURFUSwKKwkJCQkJCXNjaGVtYWlkKTsKKworCXNjYW4gPSB0
YWJsZV9iZWdpbnNjYW5fY2F0YWxvZyhjbGFzc1JlbCwga2V5Y291bnQsIGtleSk7CiAJd2hpbGUg
KCh0dXBsZSA9IGhlYXBfZ2V0bmV4dChzY2FuLCBGb3J3YXJkU2NhbkRpcmVjdGlvbikpICE9IE5V
TEwpCiAJewogCQlGb3JtX3BnX2NsYXNzIHJlbEZvcm0gPSAoRm9ybV9wZ19jbGFzcykgR0VUU1RS
VUNUKHR1cGxlKTsKQEAgLTE0MzYsNDYgKzE0MzksMyBAQCBjaGVja19mb3JlaWduX3RhYmxlc19p
bl9zY2hlbWEoT2lkIHNjaGVtYWlkLCBjaGFyICpwdWJuYW1lKQogCXRhYmxlX2VuZHNjYW4oc2Nh
bik7CiAJdGFibGVfY2xvc2UoY2xhc3NSZWwsIEFjY2Vzc1NoYXJlTG9jayk7CiB9Ci0KLS8qIENo
ZWNrIGlmIGFueSBmb3JlaWduIHRhYmxlIGlzIGEgcGFydGl0aW9uIHRhYmxlICovCi12b2lkCi1j
aGVja19mb3JlaWduX3RhYmxlcyhjaGFyICpwdWJuYW1lKQotewotCVJlbGF0aW9uCWNsYXNzUmVs
OwotCVNjYW5LZXlEYXRhIGtleVsxXTsKLQlUYWJsZVNjYW5EZXNjIHNjYW47Ci0JSGVhcFR1cGxl
CXR1cGxlOwotCi0JY2xhc3NSZWwgPSB0YWJsZV9vcGVuKFJlbGF0aW9uUmVsYXRpb25JZCwgQWNj
ZXNzU2hhcmVMb2NrKTsKLQotCVNjYW5LZXlJbml0KCZrZXlbMF0sCi0JCQkJQW51bV9wZ19jbGFz
c19yZWxraW5kLAotCQkJCUJURXF1YWxTdHJhdGVneU51bWJlciwgRl9DSEFSRVEsCi0JCQkJQ2hh
ckdldERhdHVtKFJFTEtJTkRfRk9SRUlHTl9UQUJMRSkpOwotCi0Jc2NhbiA9IHRhYmxlX2JlZ2lu
c2Nhbl9jYXRhbG9nKGNsYXNzUmVsLCAxLCBrZXkpOwotCXdoaWxlICgodHVwbGUgPSBoZWFwX2dl
dG5leHQoc2NhbiwgRm9yd2FyZFNjYW5EaXJlY3Rpb24pKSAhPSBOVUxMKQotCXsKLQkJRm9ybV9w
Z19jbGFzcyByZWxGb3JtID0gKEZvcm1fcGdfY2xhc3MpIEdFVFNUUlVDVCh0dXBsZSk7Ci0KLQkJ
aWYgKHJlbEZvcm0tPnJlbGlzcGFydGl0aW9uKQotCQl7Ci0JCQlPaWQJCQlwYXJlbnRfb2lkOwot
CQkJY2hhcgkgICAqcGFyZW50X25hbWU7Ci0JCQlMaXN0CSAgICphbmNlc3RvcnMgPSBnZXRfcGFy
dGl0aW9uX2FuY2VzdG9ycyhyZWxGb3JtLT5vaWQpOwotCi0JCQlwYXJlbnRfb2lkID0gbGxhc3Rf
b2lkKGFuY2VzdG9ycyk7Ci0JCQlwYXJlbnRfbmFtZSA9IGdldF9yZWxfbmFtZShwYXJlbnRfb2lk
KTsKLQotCQkJZXJlcG9ydChFUlJPUiwKLQkJCQkJKGVycmNvZGUoRVJSQ09ERV9JTlZBTElEX1BB
UkFNRVRFUl9WQUxVRSksCi0JCQkJCSBlcnJtc2coImNhbm5vdCBzZXQgcGFyYW1ldGVyIFwiJXNc
IiB0byB0cnVlIGZvciBwdWJsaWNhdGlvbiBcIiVzXCIiLAotCQkJCQkJCSJwdWJsaXNoX3ZpYV9w
YXJ0aXRpb25fcm9vdCIsIHB1Ym5hbWUpLAotCQkJCQkgZXJyZGV0YWlsKCJwYXJ0aXRpb24gdGFi
bGUgXCIlc1wiIGluIHB1YmxpY2F0aW9uIGNvbnRhaW5zIGEgZm9yZWlnbiBwYXJ0aXRpb24iLAot
CQkJCQkJCSAgIHBhcmVudF9uYW1lKSkpOwotCQl9Ci0JfQotCi0JdGFibGVfZW5kc2NhbihzY2Fu
KTsKLQl0YWJsZV9jbG9zZShjbGFzc1JlbCwgQWNjZXNzU2hhcmVMb2NrKTsKLX0KZGlmZiAtLWdp
dCBhL3NyYy9iYWNrZW5kL2NvbW1hbmRzL3B1YmxpY2F0aW9uY21kcy5jIGIvc3JjL2JhY2tlbmQv
Y29tbWFuZHMvcHVibGljYXRpb25jbWRzLmMKaW5kZXggZTcxZDU0MDhjNy4uYjhkYTRlZDEzMCAx
MDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvY29tbWFuZHMvcHVibGljYXRpb25jbWRzLmMKKysrIGIv
c3JjL2JhY2tlbmQvY29tbWFuZHMvcHVibGljYXRpb25jbWRzLmMKQEAgLTg3OCw3ICs4NzgsNyBA
QCBDcmVhdGVQdWJsaWNhdGlvbihQYXJzZVN0YXRlICpwc3RhdGUsIENyZWF0ZVB1YmxpY2F0aW9u
U3RtdCAqc3RtdCkKIAl7CiAJCS8qIENoZWNrIGlmIGFueSBmb3JlaWduIHRhYmxlIGlzIGEgcGFy
dCBvZiBwYXJ0aXRpb25lZCB0YWJsZSAqLwogCQlpZiAocHVibGlzaF92aWFfcGFydGl0aW9uX3Jv
b3QpCi0JCQljaGVja19mb3JlaWduX3RhYmxlcyhzdG10LT5wdWJuYW1lKTsKKwkJCWNoZWNrX2Zv
cmVpZ25fdGFibGVzX2luX3NjaGVtYShJbnZhbGlkT2lkLCBzdG10LT5wdWJuYW1lKTsKIAogCQkv
KiBJbnZhbGlkYXRlIHJlbGNhY2hlIHNvIHRoYXQgcHVibGljYXRpb24gaW5mbyBpcyByZWJ1aWx0
LiAqLwogCQlDYWNoZUludmFsaWRhdGVSZWxjYWNoZUFsbCgpOwpAQCAtMTA1Nyw3ICsxMDU3LDcg
QEAgQWx0ZXJQdWJsaWNhdGlvbk9wdGlvbnMoUGFyc2VTdGF0ZSAqcHN0YXRlLCBBbHRlclB1Ymxp
Y2F0aW9uU3RtdCAqc3RtdCwKIAkJY2hhcgkgICAqcHVibmFtZSA9IHN0bXQtPnB1Ym5hbWU7CiAK
IAkJaWYgKHB1YmZvcm0tPnB1YmFsbHRhYmxlcykKLQkJCWNoZWNrX2ZvcmVpZ25fdGFibGVzKHB1
Ym5hbWUpOworCQkJY2hlY2tfZm9yZWlnbl90YWJsZXNfaW5fc2NoZW1hKEludmFsaWRPaWQsIHB1
Ym5hbWUpOwogCiAJCXNjaGVtYW9pZHMgPSBHZXRQdWJsaWNhdGlvblNjaGVtYXMocHViZm9ybS0+
b2lkKTsKIApkaWZmIC0tZ2l0IGEvc3JjL2luY2x1ZGUvY2F0YWxvZy9wZ19wdWJsaWNhdGlvbi5o
IGIvc3JjL2luY2x1ZGUvY2F0YWxvZy9wZ19wdWJsaWNhdGlvbi5oCmluZGV4IGIxYmI4NjRkMmYu
LmZjZDQyODQ2NWUgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL2NhdGFsb2cvcGdfcHVibGljYXRp
b24uaAorKysgYi9zcmMvaW5jbHVkZS9jYXRhbG9nL3BnX3B1YmxpY2F0aW9uLmgKQEAgLTE5Niw2
ICsxOTYsNCBAQCBleHRlcm4gYm9vbCBjaGVja19wYXJ0cmVsX2hhc19mb3JlaWduX3RhYmxlKEZv
cm1fcGdfY2xhc3MgcmVsZm9ybSk7CiAKIGV4dGVybiB2b2lkIGNoZWNrX2ZvcmVpZ25fdGFibGVz
X2luX3NjaGVtYShPaWQgc2NoZW1haWQsIGNoYXIgKnB1Ym5hbWUpOwogCi1leHRlcm4gdm9pZCBj
aGVja19mb3JlaWduX3RhYmxlcyhjaGFyICpwdWJuYW1lKTsKLQogI2VuZGlmCQkJCQkJCS8qIFBH
X1BVQkxJQ0FUSU9OX0ggKi8K
--00000000000083a849062e070f9b--