On Mon, Apr 19, 2021 at 04:27:25PM +0530, Amul Sul wrote: > 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?
It's not a new feature, and shouldn't wait for July's CF since it's targetting v14. The original crash was fixed by Tom by commit 9d523119f. So it's not exactly an "open item" for v14, but there's probably no better place for it, so you could add it if you think it's at risk of being forgotten (again). https://wiki.postgresql.org/wiki/PostgreSQL_14_Open_Items -- Justin