RE: why can't a table be part of the same publication as its schema

On Monday, September 12, 2022 10:14 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> On Sat, 10 Sept 2022 at 07:32, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, Sep 9, 2022 at 8:48 PM Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > >
> > > On Fri, Sep 9, 2022 at 10:29 AM houzj(dot)fnst(at)fujitsu(dot)com
> > > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > > IIRC, the feature currently works almost the same as you
> > > > described. It doesn't create entry for tables that are published
> > > > via its schema level, it only record the published schema and check which
> tables are part of it.
> > >
> > > Oh, well if that's the case, that is great news.
> > >
> >
> > Yes, the feature works as you and Hou-San have mentioned.
> >
> > > But then I don't
> > > understand Amit's comment from before:
> > >
> > > > Yes, because otherwise, there was confusion while dropping the
> > > > objects from publication. Consider in the above case, if we would
> > > > have allowed it and then the user performs ALTER PUBLICATION p1
> > > > DROP ALL TABLES IN SCHEMA s1, then (a) shall we remove both schema
> > > > s1 and a table that is separately added (s1.t1) from that schema,
> > > > or (b) just remove schema s1?
> > >
> > > I believe that (b) is the correct behavior, so I assumed that this
> > > issue must be some difficulty in implementing it, like a funny
> > > catalog representation.
> > >
> >
> > No, it was because of syntax. IIRC, during development, Greg Nancarrow
> > raised a point [1] that a user can expect the individually added
> > tables for a schema which is also part of the publication to also get
> > dropped when she specifies DROP ALL TABLES IN SCHEMA. IIRC,
> > originally, the patch had a behavior (b) but then changed due to
> > discussion around this point. But now that it seems you and others
> > don't feel that was right, we can change back to (b) as I think that
> > shouldn't be difficult to achieve.
>
> I have made the changes to allow creation of publication with a schema and
> table of the same schema. The attached patch has the changes for the same.
> I'm planning to review and test the patch further.

Thanks for the patch. While reviewing it, I found that the column list behavior
might need to be changed or confirmed after allowing the above case.

After applying the patch, we support adding a table with column list along with
the table's schema[1], and it will directly apply the column list in the
logical replication after applying the patch.

[1]--
CREATE PUBLICATION pub FOR TABLE public.test(a), FOR ALL TABLES IN SCHEMA public;
-----

If from the point of view of consistency, for column list, we could report an
ERROR because we currently don't allow using different column lists for a
table. Maybe an ERROR like:

"ERROR: cannot use column for table x when the table's schema is also in the publication"

But if we want to report an ERROR for column list in above case. We might need
to restrict the ALTER TABLE SET SCHEMA as well because user could move a table
which is published with column list to a schema that is also published in the
publication, so we might need to add some similar check(which is removed in
Vignesh's patch) to tablecmd.c to disallow this case.

Another option could be just ingore the column list if table's schema is also
part of publication. But it seems slightly inconsistent with the rule that we
disallow using different column list for a table.

Best regards,
Hou zj