Thread: Re: Restrict publishing of partitioned table with a foreign table as partition
On Thu, 30 Jan 2025 at 17:32, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote: > > @@ -1428,6 +1427,12 @@ check_foreign_tables_in_schema(Oid schemaid) > errdetail("foreign table \"%s\" is a partition of > partitioned table \"%s\"", > get_rel_name(foreign_tbl_relid), parent_name))); > } > + else if (relForm->relkind == RELKIND_FOREIGN_TABLE) > + ereport(ERROR, > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("cannot add relation \"%s\" to publication", > + get_rel_name(relForm->oid)), > + errdetail_relkind_not_supported(RELKIND_FOREIGN_TABLE))); > } > > We should only throw error when foreign table is part of a partition > table in case of 'FOR TABLES IN SCHEMA' . We should not throw an error > otherwise because in case of 'FOR TABLES IN SCHEMA' foreign tables are > not published by default. > > I have added the changes in v3-0001. In case of all tables publication you have retrieved all the foreign tables and then checked if any of the foreign tables is a partition of a partitioned table. In case of all tables in schema publication you have retrieved all the partitioned tables and then check if it includes foreign tables. I felt you can keep the all tables in schema publication also similar to all tables publication(as generally the number of foreign tables will be lesser than the tables) i.e. retrieve all the foreign tables and then check if any of the foreign tables is a partition of a partitioned table. Regards, Vignesh
Re: Restrict publishing of partitioned table with a foreign table as partition
From
Shlok Kyal
Date:
On Mon, 10 Feb 2025 at 16:11, vignesh C <vignesh21@gmail.com> wrote: > > On Tue, 4 Feb 2025 at 18:31, vignesh C <vignesh21@gmail.com> wrote: > > > > On Thu, 30 Jan 2025 at 17:32, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote: > > > > > > @@ -1428,6 +1427,12 @@ check_foreign_tables_in_schema(Oid schemaid) > > > errdetail("foreign table \"%s\" is a partition of > > > partitioned table \"%s\"", > > > get_rel_name(foreign_tbl_relid), parent_name))); > > > } > > > + else if (relForm->relkind == RELKIND_FOREIGN_TABLE) > > > + ereport(ERROR, > > > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > > > + errmsg("cannot add relation \"%s\" to publication", > > > + get_rel_name(relForm->oid)), > > > + errdetail_relkind_not_supported(RELKIND_FOREIGN_TABLE))); > > > } > > > > > > We should only throw error when foreign table is part of a partition > > > table in case of 'FOR TABLES IN SCHEMA' . We should not throw an error > > > otherwise because in case of 'FOR TABLES IN SCHEMA' foreign tables are > > > not published by default. > > > > > > I have added the changes in v3-0001. > > > > In case of all tables publication you have retrieved all the foreign > > tables and then checked if any of the foreign tables is a partition of > > a partitioned table. In case of all tables in schema publication you > > have retrieved all the partitioned tables and then check if it > > includes foreign tables. I felt you can keep the all tables in schema > > publication also similar to all tables publication(as generally the > > number of foreign tables will be lesser than the tables) i.e. retrieve > > all the foreign tables and then check if any of the foreign tables is > > a partition of a partitioned table. > > I believe you chose this approach because partitioned tables can have > their partitions as foreign tables in a different schema. As a result, > the current approach works well for the 'TABLES IN SCHEMA' case. It > would be helpful to include a brief comment explaining this reasoning > where you're handling tables in the schema publication. > Correct, I used this approach because partitioned tables can have their foreign partitions in a different schema. I have added a comment for the same in the v5 patch [1]. [1]: https://www.postgresql.org/message-id/CANhcyEXxjq9U7BXxCba3Njz%2BeHpNzAsSVY2GzbV%3D8iy5j%3DUAUA%40mail.gmail.com Thanks and Regards, Shlok Kyal