On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote <amitlangot...@gmail.com> wrote: > > > > +logicalrep_partmap_invalidate > > > > I wonder why not call this logicalrep_partmap_update() to go with > > logicalrep_relmap_update()? It seems confusing to have > > logicalrep_partmap_invalidate() right next to > > logicalrep_partmap_invalidate_cb(). > > > > I am thinking about why we need to update the relmap in this new > function logicalrep_partmap_invalidate()? I think it may be better to > do it in logicalrep_partition_open() when actually required, > otherwise, we end up doing a lot of work that may not be of use unless > the corresponding partition is accessed. Also, it seems awkward to me > that we do the same thing in this new function > logicalrep_partmap_invalidate() and then also in > logicalrep_partition_open() under different conditions.
Both logicalrep_rel_open() and logicalrel_partition_open() only ever touch the local Relation, never the LogicalRepRelation. Updating the latter is the responsibility of logicalrep_relmap_update(), which is there to support handling of the RELATION message by apply_handle_relation(). Given that we make a separate copy of the parent's LogicalRepRelMapEntry for each partition to put into the corresponding LogicalRepPartMapEntry, those copies must be updated as well when a RELATION message targeting the parent's entry arrives. So it seems fine that the patch is making it the logicalrep_relmap_update()'s responsibility to update the partition copies using the new logicalrep_partition_invalidate/update() subroutine. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
