Re: Added schema level support for publication.

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: vignesh C <vignesh21@gmail.com>, Greg Nancarrow <gregn4422@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Smith <smithpb2250@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Ajin Cherian <itsajin@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-09-29T09:13:56Z
Lists: pgsql-hackers
On Wed, Sep 29, 2021 at 11:59 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Wed, Sep 29, 2021 12:34 PM Amit Kapila <amit.kapila16@gmail.com>
> > On Wed, Sep 29, 2021 at 9:07 AM houzj.fnst@fujitsu.com
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Tues, Sep 28, 2021 10:46 PM vignesh C <vignesh21@gmail.com> wrote:
> > > > Attached v34 patch has the changes for the same.
> > >
> > > 3)
> > > +       /*
> > > +        * Check if setting the relation to a different schema will result in the
> > > +        * publication having schema and same schema's table in the
> > publication.
> > > +        */
> > > +       if (stmt->objectType == OBJECT_TABLE)
> > > +       {
> > > +               ListCell   *lc;
> > > +               List       *schemaPubids =
> > GetSchemaPublications(nspOid);
> > > +               foreach(lc, schemaPubids)
> > > +               {
> > > +                       Oid             pubid = lfirst_oid(lc);
> > > +                       if (list_member_oid(GetPublicationRelations(pubid,
> > PUBLICATION_PART_ALL),
> > > +                                                               relid))
> > > +                               ereport(ERROR,
> > >
> > > How about we check this case like the following ?
> > >
> > > List       *schemaPubids = GetSchemaPublications(nspOid);
> > > List       *relPubids = GetRelationPublications(RelationGetRelid(rel));
> > > if (list_intersection(schemaPubids, relPubids))
> > >         ereport(ERROR, ...
> > >
> >
> > Won't this will allow changing one of the partitions for which only partitioned
> > table is part of the target schema?
>
> I think it still disallow changing partition's schema to the published one.
> I tested with the following SQLs.
> -----
> create schema sch1;
> create schema sch2;
> create schema sch3;
>
> create table sch1.tbl1 (a int) partition by range ( a );
> create table sch2.tbl1_part1 partition of sch1.tbl1 for values from (1) to (101);
> create table sch3.tbl1_part2 partition of sch1.tbl1 for values from (101) to (200);
> create publication pub for ALL TABLES IN schema sch1, TABLE sch2.tbl1_part1;
> alter table sch2.tbl1_part1 set schema sch1;
> ---* It will report an error here *
> -----
>

Use all steps before "create publication" and then try below. These
will give an error with the patch proposed but if I change it to what
you are proposing then it won't give an error.
create publication pub for ALL TABLES IN schema sch2, Table sch1.tbl1;
alter table sch3.tbl1_part2 set schema sch2;

But now again thinking about it, I am not sure if we really want to
give error in this case. What do you think? Also, if we use
list_intersection trick, then how will we tell the publication due to
which this problem has occurred, or do you think we should leave that
as an exercise for the user?

-- 
With Regards,
Amit Kapila.



Commits

  1. Include schema/table publications even with exclude options in dump.

  2. Rename some enums to use TABLE instead of REL.

  3. Add tap tests for the schema publications.

  4. Allow publishing the tables of schema.

  5. In pg_dump, use simplehash.h to look up dumpable objects by OID.