On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Jun 10, 2022 at 2:26 PM Amit Langote <amitlangote09@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