On Thu, 13 Feb 2025 at 11:25, vignesh C <vignes...@gmail.com> wrote: > > On Tue, 11 Feb 2025 at 16:55, Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > I have handled the above cases and added tests for the same. > > There is a concurrency issue with the patch: > +check_partrel_has_foreign_table(Form_pg_class relform) > +{ > + bool has_foreign_tbl = false; > + > + if (relform->relkind == RELKIND_PARTITIONED_TABLE) > + { > + List *relids = NIL; > + > + relids = find_all_inheritors(relform->oid, NoLock, NULL); > + > + foreach_oid(relid, relids) > + { > + Relation rel = table_open(relid, > AccessShareLock); > + > + if (RelationGetForm(rel)->relkind == > RELKIND_FOREIGN_TABLE) > + has_foreign_tbl = true; > + > + table_close(rel, AccessShareLock); > + > + if (has_foreign_tbl) > + break; > + } > + } > + > + return has_foreign_tbl; > +} > > In an ideal scenario, the creation of a foreign table should fail if > there is an associated publication, as demonstrated below: > CREATE TABLE t(id int) PARTITION BY RANGE(id); > CREATE TABLE part1 PARTITION OF t FOR VALUES FROM (0) TO (5); > CREATE TABLE part2 PARTITION OF t FOR VALUES FROM (5) TO (15) > PARTITION BY RANGE(id); > CREATE PUBLICATION pub1 FOR TABLE t with (publish_via_partition_root = true); > > postgres=# CREATE FOREIGN TABLE part22 PARTITION OF part2 FOR VALUES > FROM (10) TO (15) SERVER fdw; > ERROR: cannot create table foreign partition "part22" > DETAIL: partition table "part2" is published with option > publish_via_partition_root > > Consider a scenario where the publication is being created and after > the check_partrel_has_foreign_table execution is done, concurrently > creation of foreign table is executed, then the creation will be > successful. > postgres=# CREATE FOREIGN TABLE part22 PARTITION OF part2 FOR VALUES > FROM (10) TO (15) SERVER fdw; > CREATE FOREIGN TABLE > > I felt the problem here is that you have released the lock: > + if (RelationGetForm(rel)->relkind == > RELKIND_FOREIGN_TABLE) > + has_foreign_tbl = true; > + > + table_close(rel, AccessShareLock); > > We should retain the lock to fix this issue: > + if (RelationGetForm(rel)->relkind == > RELKIND_FOREIGN_TABLE) > + has_foreign_tbl = true; > + > + table_close(rel, NoLock); >
Hi Vignesh, I have fixed the issue. Attached the updated v6 patch. Thanks and Regards, Shlok Kyal
v6-0001-Restrict-publishing-of-partitioned-table-with-for.patch
Description: Binary data