From f10f0ae420ee62400876ab34dca2c09c20dcd030 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 12 Jul 2021 17:01:29 -0400 Subject: Replace RelationOpenSmgr() with RelationGetSmgr(). The idea behind this patch is to design out bugs like the one fixed by commit 9d523119f. Previously, once one did RelationOpenSmgr(rel), it was considered okay to access rel->rd_smgr directly for some not-very-clear interval. But since that pointer will be cleared by relcache flushes, we had bugs arising from overreliance on a previous RelationOpenSmgr call still being effective. Now, very little code except that in rel.h and relcache.c should ever touch the rd_smgr field directly. The normal coding rule is to use RelationGetSmgr(rel) and not expect the result to be valid for longer than one smgr function call. There are a couple of places where using the function every single time seemed like overkill, but they are now annotated with large warning comments. Amul Sul, after an idea of mine. Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com --- src/backend/access/gist/gistbuild.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'src/backend/access/gist/gistbuild.c') diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index f46a42197c..baad28c09f 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -409,8 +409,7 @@ gist_indexsortbuild(GISTBuildState *state) * replaced with the real root page at the end. */ page = palloc0(BLCKSZ); - RelationOpenSmgr(state->indexrel); - smgrextend(state->indexrel->rd_smgr, MAIN_FORKNUM, GIST_ROOT_BLKNO, + smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO, page, true); state->pages_allocated++; state->pages_written++; @@ -450,10 +449,9 @@ gist_indexsortbuild(GISTBuildState *state) gist_indexsortbuild_flush_ready_pages(state); /* Write out the root */ - RelationOpenSmgr(state->indexrel); PageSetLSN(pagestate->page, GistBuildLSN); PageSetChecksumInplace(pagestate->page, GIST_ROOT_BLKNO); - smgrwrite(state->indexrel->rd_smgr, MAIN_FORKNUM, GIST_ROOT_BLKNO, + smgrwrite(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO, pagestate->page, true); if (RelationNeedsWAL(state->indexrel)) log_newpage(&state->indexrel->rd_node, MAIN_FORKNUM, GIST_ROOT_BLKNO, @@ -562,8 +560,6 @@ gist_indexsortbuild_flush_ready_pages(GISTBuildState *state) if (state->ready_num_pages == 0) return; - RelationOpenSmgr(state->indexrel); - for (int i = 0; i < state->ready_num_pages; i++) { Page page = state->ready_pages[i]; @@ -575,7 +571,8 @@ gist_indexsortbuild_flush_ready_pages(GISTBuildState *state) PageSetLSN(page, GistBuildLSN); PageSetChecksumInplace(page, blkno); - smgrextend(state->indexrel->rd_smgr, MAIN_FORKNUM, blkno, page, true); + smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, blkno, page, + true); state->pages_written++; } @@ -860,7 +857,8 @@ gistBuildCallback(Relation index, */ if ((buildstate->buildMode == GIST_BUFFERING_AUTO && buildstate->indtuples % BUFFERING_MODE_SWITCH_CHECK_STEP == 0 && - effective_cache_size < smgrnblocks(index->rd_smgr, MAIN_FORKNUM)) || + effective_cache_size < smgrnblocks(RelationGetSmgr(index), + MAIN_FORKNUM)) || (buildstate->buildMode == GIST_BUFFERING_STATS && buildstate->indtuples >= BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET)) { -- cgit v1.2.1