On Sun, Aug 29, 2021 at 7:22 AM vignesh C <vignes...@gmail.com> wrote: > > On Sat, Aug 28, 2021 at 3:19 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Aug 27, 2021 at 6:09 PM vignesh C <vignes...@gmail.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.