On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul <sula...@gmail.com> wrote in > > On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier <mich...@paquier.xyz> > > wrote: > > > > > > On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote: > > > > We forgot this patch earlier in the commitfest. Do people think we > > > > should still get it in on this cycle? I'm +1 on that, since it's a > > > > safety feature poised to prevent more bugs than it's likely to > > > > introduce. > > > > > > No objections from here to do that now even after feature freeze. I > > > also wonder, while looking at that, why you don't just remove the last > > > call within src/backend/catalog/heap.c. This way, nobody is tempted > > > to use RelationOpenSmgr() anymore, and it could just be removed from > > > rel.h. > > > > Agree, did the same in the attached version, thanks. > > + 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. > > P.S. commitfest entry https://commitfest.postgresql.org/33/3084/ > > Isn't this a kind of open item? > Sorry, I didn't get you. Do I need to move this to some other bucket? Regards, Amul
