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



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