On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 19 Apr 2021 16:27:25 +0530, Amul Sul <sulamul@gmail.com> wrote in
> > On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote:
> > > + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BLOOM_METAPAGE_BLKNO,
> > > (char *) metapage, true);
> > > - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
> > > + log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
> > >
> > > At the log_newpage, index is guaranteed to have rd_smgr. So I prefer
> > > to leave the line alone.. I don't mind other sccessive calls if any
> > > since what I don't like is the notation there.
> > >
> >
> > Perhaps, isn't that bad. It is good to follow the practice of using
> > RelationGetSmgr() for rd_smgr access, IMHO.
>
> I don't mind RelationGetSmgr(index)->smgr_rnode alone or
> &variable->member alone and there's not the previous call to
> RelationGetSmgr just above. How about using a temporary variable?
>
> SMgrRelation srel = RelationGetSmgr(index);
> smgrwrite(srel, ...);
> log_newpage(srel->..);
>
Understood. Used a temporary variable for the place where
RelationGetSmgr() calls are placed too close or in a loop.
Please have a look at the attached version, thanks for the review.
Regards,
Amul
