On Wed, Jan 13, 2021 at 3:16 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Jan 13, 2021 at 2:36 PM japin <japi...@hotmail.com> wrote: > > > In summary, I feel we need to fix the publisher sending the inserts > > > even though the table is dropped from the publication, that is the > > > patch patch proposed by japin. This solves the bug reported in this > > > thread. > > > > > > And also, it's good to have the LogicalRepRelMap invalidation fix as a > > > 0002 patch in invalidate_syncing_table_states, subscription_change_cb > > > and logicalrep_rel_open as proposed by me. > > > > > > Thoughts? > > > > > > > I think invalidate the LogicalRepRelMap is necessary. If the table isn't in > > subscription, can we remove the LogicalRepRelMapEntry from LogicalRepRelMap? > > IIUC, it's not possible to know the relid of the alter > publication...dropped table in the invalidation callbacks > invalidate_syncing_table_states or subscription_change_cb, so it's > better to set state of all the entries to SUBREL_STATE_UNKNOWN, so > that, in the next logicalrep_rel_open call the function > GetSubscriptionRelState will be called and the pg_subscription_rel > will be read freshly. > > This is inline with the reasoning specified at [1]. > > [1] - > https://www.postgresql.org/message-id/CAFiTN-udwuc_h0TaFrSEKb-Yo1vVvkjz9TDRw7VE3P-KiPSGbQ%40mail.gmail.com
Attaching following two patches: 0001 - Makes publisher to not publish the changes for the alter publication ... dropped tables. Original patch is by japin, I added comments, changed the function name and adjusted the code a bit. 0002 - Invalidates the relation map cache in subscriber syscache invalidation callbacks. Currently, I'm setting entry->state to SUBREL_STATE_UNKNOWN in the new invalidation function that's introduced logicalrep_relmap_invalidate, so that in the next call to logicalrep_rel_open the entry's state will be read from the system catalogues using GetSubscriptionRelState. If this is sound's bit strange, I can add a boolean invalid to LogicalRepRelMapEntry structure and set that here and in logicalrep_rel_open, I can have something like: if (entry->state != SUBREL_STATE_READY || entry->invalid) entry->state = GetSubscriptionRelState(MySubscription->oid, entry->localreloid, &entry->statelsn); if (entry->invalid) entry->invalid = false; make check and make check-world passes on both patches. If okay with the fixes, I will try to add a test case and also a cf entry so that these patches get a chance to be tested on all the platforms. Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
v1-0001-Fix-ALTER-PUBLICATION.DROP-TABLE-behaviour.patch
Description: Binary data
v1-0002-Invalidate-relation-map-cache-in-subscriber-sysca.patch
Description: Binary data