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
