Re: Added schema level support for publication.

On Sun, Aug 29, 2021 at 7:22 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Sat, Aug 28, 2021 at 3:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, Aug 27, 2021 at 6:09 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> >
> > What do you mean by update should fail? I think all the relations in
> > RelationSyncCache via rel_sync_cache_publication_cb because you have
> > updated pg_publication_schema and that should register syscache
> > invalidation.
>
> By update should fail, I meant the updates without setting replica
> identity before creating the decoding context. The scenario is like
> below (all in the same session, the subscription is not created):
> create schema sch1;
> create table sch1.t1(c1 int);
> insert into sch1.t1 values(10);
> # Before updating we will check CheckCmdReplicaIdentity, as there are
> no publications on this table rd_pubactions will be set accordingly in
> relcache entry.
> update sch1.t1 set c1 = 11;
> # Now we will create the publication after rd_pubactions has been set
> in the cache. Now when we create this publication we should invalidate
> the relations present in the schema, this is required so that when the
> next update happens, we should check the publication actions again in
> CheckCmdReplicaIdentity and fail the update which does not set
> replica identity after the publication is created.
> create publication pub1 for all tables in schema sch1;
> # After the above publication is created the relations present in this
> schema will be invalidated. Now we will check the publication actions
> again in CheckCmdReplicaIdentity and the update will fail.
> update sch1.t1 set c1 = 11;
> ERROR: cannot update table "t1" because it does not have a replica
> identity and publishes updates
> HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.
>

Okay, I got it but let's add few comments in the code related to it.
Also, I noticed that the code in InvalidatePublicationRels() already
exists in AlterPublicationOptions(). You can try to refactor the
existing code as a separate initial patch.

BTW, I noticed that "for all tables", we don't register invalidations
in the above scenario, and then later that causes conflict on the
subscriber. I think that is a bug in the current code and we can deal
with that separately.

--
With Regards,
Amit Kapila.