On Tue, 4 Feb 2025 at 18:31, vignesh C <vignes...@gmail.com> wrote:
>
> On Thu, 30 Jan 2025 at 17:32, Shlok Kyal <shlok.kyal....@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.
Regards,
Vignesh
