Replace RelationOpenSmgr() with RelationGetSmgr().
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 12 Jul 2021 21:01:29 +0000 (17:01 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 12 Jul 2021 21:01:36 +0000 (17:01 -0400)
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

21 files changed:
contrib/amcheck/verify_nbtree.c
contrib/bloom/blinsert.c
contrib/pg_prewarm/autoprewarm.c
contrib/pg_prewarm/pg_prewarm.c
contrib/pg_visibility/pg_visibility.c
src/backend/access/gist/gistbuild.c
src/backend/access/hash/hashpage.c
src/backend/access/heap/heapam_handler.c
src/backend/access/heap/rewriteheap.c
src/backend/access/heap/visibilitymap.c
src/backend/access/nbtree/nbtree.c
src/backend/access/nbtree/nbtsort.c
src/backend/access/spgist/spginsert.c
src/backend/access/table/tableam.c
src/backend/catalog/heap.c
src/backend/catalog/index.c
src/backend/catalog/storage.c
src/backend/commands/tablecmds.c
src/backend/storage/buffer/bufmgr.c
src/backend/storage/freespace/freespace.c
src/include/utils/rel.h

index fdfc320e84f9b4075c03058e5fd40048fb03026f..d19f73127cd460e6ab0095db9dabd60c7078d301 100644 (file)
@@ -301,8 +301,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
                bool            heapkeyspace,
                                        allequalimage;
 
-               RelationOpenSmgr(indrel);
-               if (!smgrexists(indrel->rd_smgr, MAIN_FORKNUM))
+               if (!smgrexists(RelationGetSmgr(indrel), MAIN_FORKNUM))
                        ereport(ERROR,
                                        (errcode(ERRCODE_INDEX_CORRUPTED),
                                         errmsg("index \"%s\" lacks a main relation fork",
index c34a640d1c44230ed94467b27c4cf6adc4a053f5..23661d1105ce5f3aeca1f841fa072cf45e84b8b9 100644 (file)
@@ -177,9 +177,9 @@ blbuildempty(Relation index)
         * this even when wal_level=minimal.
         */
        PageSetChecksumInplace(metapage, BLOOM_METAPAGE_BLKNO);
-       smgrwrite(index->rd_smgr, INIT_FORKNUM, BLOOM_METAPAGE_BLKNO,
+       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,
                                BLOOM_METAPAGE_BLKNO, metapage, true);
 
        /*
@@ -187,7 +187,7 @@ blbuildempty(Relation index)
         * write did not go through shared_buffers and therefore a concurrent
         * checkpoint may have moved the redo pointer past our xlog record.
         */
-       smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
+       smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM);
 }
 
 /*
index b3f73ea92d6519b43dd481817360fa7b900aa99c..0289ea657cb64dfc3ab74f5e2f43370a00257172 100644 (file)
@@ -514,15 +514,13 @@ autoprewarm_database_main(Datum main_arg)
                        old_blk->filenode != blk->filenode ||
                        old_blk->forknum != blk->forknum)
                {
-                       RelationOpenSmgr(rel);
-
                        /*
                         * smgrexists is not safe for illegal forknum, hence check whether
                         * the passed forknum is valid before using it in smgrexists.
                         */
                        if (blk->forknum > InvalidForkNumber &&
                                blk->forknum <= MAX_FORKNUM &&
-                               smgrexists(rel->rd_smgr, blk->forknum))
+                               smgrexists(RelationGetSmgr(rel), blk->forknum))
                                nblocks = RelationGetNumberOfBlocksInFork(rel, blk->forknum);
                        else
                                nblocks = 0;
index 48d0132a0d0e14bf2ad598af51dcd53249a4a52e..00438239749287f601e40f53cb80d020ae4badcf 100644 (file)
@@ -109,8 +109,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
                aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind), get_rel_name(relOid));
 
        /* Check that the fork exists. */
-       RelationOpenSmgr(rel);
-       if (!smgrexists(rel->rd_smgr, forkNumber))
+       if (!smgrexists(RelationGetSmgr(rel), forkNumber))
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                 errmsg("fork \"%s\" does not exist for this relation",
@@ -178,7 +177,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
                for (block = first_block; block <= last_block; ++block)
                {
                        CHECK_FOR_INTERRUPTS();
-                       smgrread(rel->rd_smgr, forkNumber, block, blockbuffer.data);
+                       smgrread(RelationGetSmgr(rel), forkNumber, block, blockbuffer.data);
                        ++blocks_done;
                }
        }
index c6d983183dbc1b91ffcac2144766ed0a219ade9e..b5362edcee5ba0f445fd7c0e30177c0265726982 100644 (file)
@@ -391,14 +391,14 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
        /* Only some relkinds have a visibility map */
        check_relation_relkind(rel);
 
-       RelationOpenSmgr(rel);
-       rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber;
+       /* Forcibly reset cached file size */
+       RelationGetSmgr(rel)->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber;
 
        block = visibilitymap_prepare_truncate(rel, 0);
        if (BlockNumberIsValid(block))
        {
                fork = VISIBILITYMAP_FORKNUM;
-               smgrtruncate(rel->rd_smgr, &fork, 1, &block);
+               smgrtruncate(RelationGetSmgr(rel), &fork, 1, &block);
        }
 
        if (RelationNeedsWAL(rel))
index f46a42197c9791dde81f9dfbb596b12efe5d3edf..baad28c09fa466db21b923009168c26447c77e20 100644 (file)
@@ -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))
        {
index c5c2382b36affab79ca100473d570d262944bd6d..b73002535669df6fd8c0f0424c2c5d92ed4d77a5 100644 (file)
@@ -1024,9 +1024,9 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
                                        zerobuf.data,
                                        true);
 
-       RelationOpenSmgr(rel);
        PageSetChecksumInplace(page, lastblock);
-       smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf.data, false);
+       smgrextend(RelationGetSmgr(rel), MAIN_FORKNUM, lastblock, zerobuf.data,
+                          false);
 
        return true;
 }
index 1b8b640012a660e1f04b17395fe52cbeb0aee250..beb8f20708802c232f36b7858e929d3f9bc1b247 100644 (file)
@@ -625,7 +625,6 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
        SMgrRelation dstrel;
 
        dstrel = smgropen(*newrnode, rel->rd_backend);
-       RelationOpenSmgr(rel);
 
        /*
         * Since we copy the file directly without looking at the shared buffers,
@@ -645,14 +644,14 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
        RelationCreateStorage(*newrnode, rel->rd_rel->relpersistence);
 
        /* copy main fork */
-       RelationCopyStorage(rel->rd_smgr, dstrel, MAIN_FORKNUM,
+       RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM,
                                                rel->rd_rel->relpersistence);
 
        /* copy those extra forks that exist */
        for (ForkNumber forkNum = MAIN_FORKNUM + 1;
                 forkNum <= MAX_FORKNUM; forkNum++)
        {
-               if (smgrexists(rel->rd_smgr, forkNum))
+               if (smgrexists(RelationGetSmgr(rel), forkNum))
                {
                        smgrcreate(dstrel, forkNum, false);
 
@@ -664,7 +663,7 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
                                (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
                                 forkNum == INIT_FORKNUM))
                                log_smgrcreate(newrnode, forkNum);
-                       RelationCopyStorage(rel->rd_smgr, dstrel, forkNum,
+                       RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum,
                                                                rel->rd_rel->relpersistence);
                }
        }
index 1aff62cd423a14314e8c3a6343f6c28c88953368..986a776bbd541facf471464541fc2c353100a0bf 100644 (file)
@@ -326,9 +326,8 @@ end_heap_rewrite(RewriteState state)
 
                PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
 
-               RelationOpenSmgr(state->rs_new_rel);
-               smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno,
-                                  (char *) state->rs_buffer, true);
+               smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM,
+                                  state->rs_blockno, (char *) state->rs_buffer, true);
        }
 
        /*
@@ -339,11 +338,7 @@ end_heap_rewrite(RewriteState state)
         * wrote before the checkpoint.
         */
        if (RelationNeedsWAL(state->rs_new_rel))
-       {
-               /* for an empty table, this could be first smgr access */
-               RelationOpenSmgr(state->rs_new_rel);
-               smgrimmedsync(state->rs_new_rel->rd_smgr, MAIN_FORKNUM);
-       }
+               smgrimmedsync(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM);
 
        logical_end_heap_rewrite(state);
 
@@ -695,11 +690,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
                         * need for smgr to schedule an fsync for this write; we'll do it
                         * ourselves in end_heap_rewrite.
                         */
-                       RelationOpenSmgr(state->rs_new_rel);
-
                        PageSetChecksumInplace(page, state->rs_blockno);
 
-                       smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM,
+                       smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM,
                                           state->rs_blockno, (char *) page, true);
 
                        state->rs_blockno++;
index e198df65d82769fa9c34ab2c08f071434c532294..4720b35ee5cdc50845adbcc3560c41cae0bb413f 100644 (file)
@@ -455,13 +455,11 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks)
        elog(DEBUG1, "vm_truncate %s %d", RelationGetRelationName(rel), nheapblocks);
 #endif
 
-       RelationOpenSmgr(rel);
-
        /*
         * If no visibility map has been created yet for this relation, there's
         * nothing to truncate.
         */
-       if (!smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM))
+       if (!smgrexists(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM))
                return InvalidBlockNumber;
 
        /*
@@ -528,7 +526,7 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks)
        else
                newnblocks = truncBlock;
 
-       if (smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM) <= newnblocks)
+       if (smgrnblocks(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM) <= newnblocks)
        {
                /* nothing to do, the file was already smaller than requested size */
                return InvalidBlockNumber;
@@ -547,30 +545,29 @@ static Buffer
 vm_readbuf(Relation rel, BlockNumber blkno, bool extend)
 {
        Buffer          buf;
+       SMgrRelation reln;
 
        /*
-        * We might not have opened the relation at the smgr level yet, or we
-        * might have been forced to close it by a sinval message.  The code below
-        * won't necessarily notice relation extension immediately when extend =
-        * false, so we rely on sinval messages to ensure that our ideas about the
-        * size of the map aren't too far out of date.
+        * Caution: re-using this smgr pointer could fail if the relcache entry
+        * gets closed.  It's safe as long as we only do smgr-level operations
+        * between here and the last use of the pointer.
         */
-       RelationOpenSmgr(rel);
+       reln = RelationGetSmgr(rel);
 
        /*
         * If we haven't cached the size of the visibility map fork yet, check it
         * first.
         */
-       if (rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber)
+       if (reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber)
        {
-               if (smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM))
-                       smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM);
+               if (smgrexists(reln, VISIBILITYMAP_FORKNUM))
+                       smgrnblocks(reln, VISIBILITYMAP_FORKNUM);
                else
-                       rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = 0;
+                       reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = 0;
        }
 
        /* Handle requests beyond EOF */
-       if (blkno >= rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM])
+       if (blkno >= reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM])
        {
                if (extend)
                        vm_extend(rel, blkno + 1);
@@ -618,6 +615,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
 {
        BlockNumber vm_nblocks_now;
        PGAlignedBlock pg;
+       SMgrRelation reln;
 
        PageInit((Page) pg.data, BLCKSZ, 0);
 
@@ -633,29 +631,32 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
         */
        LockRelationForExtension(rel, ExclusiveLock);
 
-       /* Might have to re-open if a cache flush happened */
-       RelationOpenSmgr(rel);
+       /*
+        * Caution: re-using this smgr pointer could fail if the relcache entry
+        * gets closed.  It's safe as long as we only do smgr-level operations
+        * between here and the last use of the pointer.
+        */
+       reln = RelationGetSmgr(rel);
 
        /*
         * Create the file first if it doesn't exist.  If smgr_vm_nblocks is
         * positive then it must exist, no need for an smgrexists call.
         */
-       if ((rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == 0 ||
-                rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) &&
-               !smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM))
-               smgrcreate(rel->rd_smgr, VISIBILITYMAP_FORKNUM, false);
+       if ((reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == 0 ||
+                reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) &&
+               !smgrexists(reln, VISIBILITYMAP_FORKNUM))
+               smgrcreate(reln, VISIBILITYMAP_FORKNUM, false);
 
        /* Invalidate cache so that smgrnblocks() asks the kernel. */
-       rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber;
-       vm_nblocks_now = smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM);
+       reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber;
+       vm_nblocks_now = smgrnblocks(reln, VISIBILITYMAP_FORKNUM);
 
        /* Now extend the file */
        while (vm_nblocks_now < vm_nblocks)
        {
                PageSetChecksumInplace((Page) pg.data, vm_nblocks_now);
 
-               smgrextend(rel->rd_smgr, VISIBILITYMAP_FORKNUM, vm_nblocks_now,
-                                  pg.data, false);
+               smgrextend(reln, VISIBILITYMAP_FORKNUM, vm_nblocks_now, pg.data, false);
                vm_nblocks_now++;
        }
 
@@ -666,7 +667,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
         * to keep checking for creation or extension of the file, which happens
         * infrequently.
         */
-       CacheInvalidateSmgr(rel->rd_smgr->smgr_rnode);
+       CacheInvalidateSmgr(reln->smgr_rnode);
 
        UnlockRelationForExtension(rel, ExclusiveLock);
 }
index 1360ab80c1e80d8c16d7a16f93d5c1e48166deb6..be23395dfbb2e586b344a7c249305b387985ed03 100644 (file)
@@ -163,9 +163,9 @@ btbuildempty(Relation index)
         * this even when wal_level=minimal.
         */
        PageSetChecksumInplace(metapage, BTREE_METAPAGE);
-       smgrwrite(index->rd_smgr, INIT_FORKNUM, BTREE_METAPAGE,
+       smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BTREE_METAPAGE,
                          (char *) metapage, true);
-       log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
+       log_newpage(&RelationGetSmgr(index)->smgr_rnode.node, INIT_FORKNUM,
                                BTREE_METAPAGE, metapage, true);
 
        /*
@@ -173,7 +173,7 @@ btbuildempty(Relation index)
         * write did not go through shared_buffers and therefore a concurrent
         * checkpoint may have moved the redo pointer past our xlog record.
         */
-       smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
+       smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM);
 }
 
 /*
index 5fa6ea8ad9a097a53be8d8c8ae8962b5f7b53a69..54c8eb1289d3412f042f366727f2099df3923e31 100644 (file)
@@ -637,9 +637,6 @@ _bt_blnewpage(uint32 level)
 static void
 _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 {
-       /* Ensure rd_smgr is open (could have been closed by relcache flush!) */
-       RelationOpenSmgr(wstate->index);
-
        /* XLOG stuff */
        if (wstate->btws_use_wal)
        {
@@ -659,7 +656,7 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
                if (!wstate->btws_zeropage)
                        wstate->btws_zeropage = (Page) palloc0(BLCKSZ);
                /* don't set checksum for all-zero page */
-               smgrextend(wstate->index->rd_smgr, MAIN_FORKNUM,
+               smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM,
                                   wstate->btws_pages_written++,
                                   (char *) wstate->btws_zeropage,
                                   true);
@@ -674,14 +671,14 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
        if (blkno == wstate->btws_pages_written)
        {
                /* extending the file... */
-               smgrextend(wstate->index->rd_smgr, MAIN_FORKNUM, blkno,
+               smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno,
                                   (char *) page, true);
                wstate->btws_pages_written++;
        }
        else
        {
                /* overwriting a block we zero-filled before */
-               smgrwrite(wstate->index->rd_smgr, MAIN_FORKNUM, blkno,
+               smgrwrite(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno,
                                  (char *) page, true);
        }
 
@@ -1428,10 +1425,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
         * still not be on disk when the crash occurs.
         */
        if (wstate->btws_use_wal)
-       {
-               RelationOpenSmgr(wstate->index);
-               smgrimmedsync(wstate->index->rd_smgr, MAIN_FORKNUM);
-       }
+               smgrimmedsync(RelationGetSmgr(wstate->index), MAIN_FORKNUM);
 }
 
 /*
index 1af0af7da21f3491ce5a9884a378b13504ce9299..cc4394b1c8d628c99eb4d3f68e2e31ba591870d1 100644 (file)
@@ -169,27 +169,27 @@ spgbuildempty(Relation index)
         * replayed.
         */
        PageSetChecksumInplace(page, SPGIST_METAPAGE_BLKNO);
-       smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO,
+       smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_METAPAGE_BLKNO,
                          (char *) page, true);
-       log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
+       log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
                                SPGIST_METAPAGE_BLKNO, page, true);
 
        /* Likewise for the root page. */
        SpGistInitPage(page, SPGIST_LEAF);
 
        PageSetChecksumInplace(page, SPGIST_ROOT_BLKNO);
-       smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_ROOT_BLKNO,
+       smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_ROOT_BLKNO,
                          (char *) page, true);
-       log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
+       log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
                                SPGIST_ROOT_BLKNO, page, true);
 
        /* Likewise for the null-tuples root page. */
        SpGistInitPage(page, SPGIST_LEAF | SPGIST_NULLS);
 
        PageSetChecksumInplace(page, SPGIST_NULL_BLKNO);
-       smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_NULL_BLKNO,
+       smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_NULL_BLKNO,
                          (char *) page, true);
-       log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
+       log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
                                SPGIST_NULL_BLKNO, page, true);
 
        /*
@@ -197,7 +197,7 @@ spgbuildempty(Relation index)
         * writes did not go through shared buffers and therefore a concurrent
         * checkpoint may have moved the redo pointer past our xlog record.
         */
-       smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
+       smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM);
 }
 
 /*
index 5ea5bdd810433a93805b064832ba5155f1a2b49b..66f0f84386c43478cedd6a64780a4a8572d67ea3 100644 (file)
@@ -629,17 +629,14 @@ table_block_relation_size(Relation rel, ForkNumber forkNumber)
 {
        uint64          nblocks = 0;
 
-       /* Open it at the smgr level if not already done */
-       RelationOpenSmgr(rel);
-
        /* InvalidForkNumber indicates returning the size for all forks */
        if (forkNumber == InvalidForkNumber)
        {
                for (int i = 0; i < MAX_FORKNUM; i++)
-                       nblocks += smgrnblocks(rel->rd_smgr, i);
+                       nblocks += smgrnblocks(RelationGetSmgr(rel), i);
        }
        else
-               nblocks = smgrnblocks(rel->rd_smgr, forkNumber);
+               nblocks = smgrnblocks(RelationGetSmgr(rel), forkNumber);
 
        return nblocks * BLCKSZ;
 }
index 09370a8a5a0a495ebf228248028bc5fbe6d50b0b..83746d3fd91a4662e063aedc462c78eafeb66344 100644 (file)
@@ -415,8 +415,6 @@ heap_create(const char *relname,
         */
        if (create_storage)
        {
-               RelationOpenSmgr(rel);
-
                switch (rel->rd_rel->relkind)
                {
                        case RELKIND_VIEW:
index 50b7a16bce94aaf962a8f435006fd5379b51cebd..26bfa74ce757181349e550ae6725aea26f4bb68f 100644 (file)
@@ -2982,10 +2982,9 @@ index_build(Relation heapRelation,
         * relfilenode won't change, and nothing needs to be done here.
         */
        if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
-               !smgrexists(indexRelation->rd_smgr, INIT_FORKNUM))
+               !smgrexists(RelationGetSmgr(indexRelation), INIT_FORKNUM))
        {
-               RelationOpenSmgr(indexRelation);
-               smgrcreate(indexRelation->rd_smgr, INIT_FORKNUM, false);
+               smgrcreate(RelationGetSmgr(indexRelation), INIT_FORKNUM, false);
                indexRelation->rd_indam->ambuildempty(indexRelation);
        }
 
index cba7a9ada07e1cbbe18459175780c354371b6c39..c5ad28d71febdb6db375ac235f6256d42d5d48ab 100644 (file)
@@ -282,16 +282,16 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
        ForkNumber      forks[MAX_FORKNUM];
        BlockNumber blocks[MAX_FORKNUM];
        int                     nforks = 0;
-
-       /* Open it at the smgr level if not already done */
-       RelationOpenSmgr(rel);
+       SMgrRelation reln;
 
        /*
-        * Make sure smgr_targblock etc aren't pointing somewhere past new end
+        * Make sure smgr_targblock etc aren't pointing somewhere past new end.
+        * (Note: don't rely on this reln pointer below this loop.)
         */
-       rel->rd_smgr->smgr_targblock = InvalidBlockNumber;
+       reln = RelationGetSmgr(rel);
+       reln->smgr_targblock = InvalidBlockNumber;
        for (int i = 0; i <= MAX_FORKNUM; ++i)
-               rel->rd_smgr->smgr_cached_nblocks[i] = InvalidBlockNumber;
+               reln->smgr_cached_nblocks[i] = InvalidBlockNumber;
 
        /* Prepare for truncation of MAIN fork of the relation */
        forks[nforks] = MAIN_FORKNUM;
@@ -299,7 +299,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
        nforks++;
 
        /* Prepare for truncation of the FSM if it exists */
-       fsm = smgrexists(rel->rd_smgr, FSM_FORKNUM);
+       fsm = smgrexists(RelationGetSmgr(rel), FSM_FORKNUM);
        if (fsm)
        {
                blocks[nforks] = FreeSpaceMapPrepareTruncateRel(rel, nblocks);
@@ -312,7 +312,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
        }
 
        /* Prepare for truncation of the visibility map too if it exists */
-       vm = smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM);
+       vm = smgrexists(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM);
        if (vm)
        {
                blocks[nforks] = visibilitymap_prepare_truncate(rel, nblocks);
@@ -364,7 +364,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
        }
 
        /* Do the real work to truncate relation forks */
-       smgrtruncate(rel->rd_smgr, forks, nforks, blocks);
+       smgrtruncate(RelationGetSmgr(rel), forks, nforks, blocks);
 
        /*
         * Update upper-level FSM pages to account for the truncation. This is
@@ -389,9 +389,9 @@ RelationPreTruncate(Relation rel)
 
        if (!pendingSyncHash)
                return;
-       RelationOpenSmgr(rel);
 
-       pending = hash_search(pendingSyncHash, &(rel->rd_smgr->smgr_rnode.node),
+       pending = hash_search(pendingSyncHash,
+                                                 &(RelationGetSmgr(rel)->smgr_rnode.node),
                                                  HASH_FIND, NULL);
        if (pending)
                pending->is_truncated = true;
@@ -403,6 +403,12 @@ RelationPreTruncate(Relation rel)
  * Note that this requires that there is no dirty data in shared buffers. If
  * it's possible that there are, callers need to flush those using
  * e.g. FlushRelationBuffers(rel).
+ *
+ * Also note that this is frequently called via locutions such as
+ *             RelationCopyStorage(RelationGetSmgr(rel), ...);
+ * That's safe only because we perform only smgr and WAL operations here.
+ * If we invoked anything else, a relcache flush could cause our SMgrRelation
+ * argument to become a dangling pointer.
  */
 void
 RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
@@ -445,13 +451,23 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
 
                if (!PageIsVerifiedExtended(page, blkno,
                                                                        PIV_LOG_WARNING | PIV_REPORT_STAT))
+               {
+                       /*
+                        * For paranoia's sake, capture the file path before invoking the
+                        * ereport machinery.  This guards against the possibility of a
+                        * relcache flush caused by, e.g., an errcontext callback.
+                        * (errcontext callbacks shouldn't be risking any such thing, but
+                        * people have been known to forget that rule.)
+                        */
+                       char       *relpath = relpathbackend(src->smgr_rnode.node,
+                                                                                                src->smgr_rnode.backend,
+                                                                                                forkNum);
+
                        ereport(ERROR,
                                        (errcode(ERRCODE_DATA_CORRUPTED),
                                         errmsg("invalid page in block %u of relation %s",
-                                                       blkno,
-                                                       relpathbackend(src->smgr_rnode.node,
-                                                                                  src->smgr_rnode.backend,
-                                                                                  forkNum))));
+                                                       blkno, relpath)));
+               }
 
                /*
                 * WAL-log the copied page. Unfortunately we don't know what kind of a
index 03dfd2e7fa667e221f2ed4c264af90cb3d49f231..dcf14b9dc0e549be13a116a368723033b9dbe662 100644 (file)
@@ -14151,7 +14151,6 @@ index_copy_data(Relation rel, RelFileNode newrnode)
        SMgrRelation dstrel;
 
        dstrel = smgropen(newrnode, rel->rd_backend);
-       RelationOpenSmgr(rel);
 
        /*
         * Since we copy the file directly without looking at the shared buffers,
@@ -14171,14 +14170,14 @@ index_copy_data(Relation rel, RelFileNode newrnode)
        RelationCreateStorage(newrnode, rel->rd_rel->relpersistence);
 
        /* copy main fork */
-       RelationCopyStorage(rel->rd_smgr, dstrel, MAIN_FORKNUM,
+       RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM,
                                                rel->rd_rel->relpersistence);
 
        /* copy those extra forks that exist */
        for (ForkNumber forkNum = MAIN_FORKNUM + 1;
                 forkNum <= MAX_FORKNUM; forkNum++)
        {
-               if (smgrexists(rel->rd_smgr, forkNum))
+               if (smgrexists(RelationGetSmgr(rel), forkNum))
                {
                        smgrcreate(dstrel, forkNum, false);
 
@@ -14190,7 +14189,7 @@ index_copy_data(Relation rel, RelFileNode newrnode)
                                (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
                                 forkNum == INIT_FORKNUM))
                                log_smgrcreate(&newrnode, forkNum);
-                       RelationCopyStorage(rel->rd_smgr, dstrel, forkNum,
+                       RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum,
                                                                rel->rd_rel->relpersistence);
                }
        }
index 4b296a22c45a4ed974378748424b46a6e9fe6013..86ef607ff3811b73880e4461ad71149bebc5cf32 100644 (file)
@@ -589,9 +589,6 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)
        Assert(RelationIsValid(reln));
        Assert(BlockNumberIsValid(blockNum));
 
-       /* Open it at the smgr level if not already done */
-       RelationOpenSmgr(reln);
-
        if (RelationUsesLocalBuffers(reln))
        {
                /* see comments in ReadBufferExtended */
@@ -601,12 +598,12 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)
                                         errmsg("cannot access temporary tables of other sessions")));
 
                /* pass it off to localbuf.c */
-               return PrefetchLocalBuffer(reln->rd_smgr, forkNum, blockNum);
+               return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum);
        }
        else
        {
                /* pass it to the shared buffer version */
-               return PrefetchSharedBuffer(reln->rd_smgr, forkNum, blockNum);
+               return PrefetchSharedBuffer(RelationGetSmgr(reln), forkNum, blockNum);
        }
 }
 
@@ -747,9 +744,6 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum,
        bool            hit;
        Buffer          buf;
 
-       /* Open it at the smgr level if not already done */
-       RelationOpenSmgr(reln);
-
        /*
         * Reject attempts to read non-local temporary relations; we would be
         * likely to get wrong data since we have no visibility into the owning
@@ -765,7 +759,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum,
         * miss.
         */
        pgstat_count_buffer_read(reln);
-       buf = ReadBuffer_common(reln->rd_smgr, reln->rd_rel->relpersistence,
+       buf = ReadBuffer_common(RelationGetSmgr(reln), reln->rd_rel->relpersistence,
                                                        forkNum, blockNum, mode, strategy, &hit);
        if (hit)
                pgstat_count_buffer_hit(reln);
@@ -2949,10 +2943,7 @@ RelationGetNumberOfBlocksInFork(Relation relation, ForkNumber forkNum)
                case RELKIND_SEQUENCE:
                case RELKIND_INDEX:
                case RELKIND_PARTITIONED_INDEX:
-                       /* Open it at the smgr level if not already done */
-                       RelationOpenSmgr(relation);
-
-                       return smgrnblocks(relation->rd_smgr, forkNum);
+                       return smgrnblocks(RelationGetSmgr(relation), forkNum);
 
                case RELKIND_RELATION:
                case RELKIND_TOASTVALUE:
@@ -3527,9 +3518,6 @@ FlushRelationBuffers(Relation rel)
        int                     i;
        BufferDesc *bufHdr;
 
-       /* Open rel at the smgr level if not already done */
-       RelationOpenSmgr(rel);
-
        if (RelationUsesLocalBuffers(rel))
        {
                for (i = 0; i < NLocBuffer; i++)
@@ -3554,7 +3542,7 @@ FlushRelationBuffers(Relation rel)
 
                                PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-                               smgrwrite(rel->rd_smgr,
+                               smgrwrite(RelationGetSmgr(rel),
                                                  bufHdr->tag.forkNum,
                                                  bufHdr->tag.blockNum,
                                                  localpage,
@@ -3595,7 +3583,7 @@ FlushRelationBuffers(Relation rel)
                {
                        PinBuffer_Locked(bufHdr);
                        LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-                       FlushBuffer(bufHdr, rel->rd_smgr);
+                       FlushBuffer(bufHdr, RelationGetSmgr(rel));
                        LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
                        UnpinBuffer(bufHdr, true);
                }
index 8c12dda2380d061f455573363c0bf8876558070b..09d4b16067dec5396903dec763d455eaf7319600 100644 (file)
@@ -265,13 +265,11 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
        uint16          first_removed_slot;
        Buffer          buf;
 
-       RelationOpenSmgr(rel);
-
        /*
         * If no FSM has been created yet for this relation, there's nothing to
         * truncate.
         */
-       if (!smgrexists(rel->rd_smgr, FSM_FORKNUM))
+       if (!smgrexists(RelationGetSmgr(rel), FSM_FORKNUM))
                return InvalidBlockNumber;
 
        /* Get the location in the FSM of the first removed heap block */
@@ -317,7 +315,7 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
        else
        {
                new_nfsmblocks = fsm_logical_to_physical(first_removed_address);
-               if (smgrnblocks(rel->rd_smgr, FSM_FORKNUM) <= new_nfsmblocks)
+               if (smgrnblocks(RelationGetSmgr(rel), FSM_FORKNUM) <= new_nfsmblocks)
                        return InvalidBlockNumber;      /* nothing to do; the FSM was already
                                                                                 * smaller */
        }
@@ -532,8 +530,14 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend)
 {
        BlockNumber blkno = fsm_logical_to_physical(addr);
        Buffer          buf;
+       SMgrRelation reln;
 
-       RelationOpenSmgr(rel);
+       /*
+        * Caution: re-using this smgr pointer could fail if the relcache entry
+        * gets closed.  It's safe as long as we only do smgr-level operations
+        * between here and the last use of the pointer.
+        */
+       reln = RelationGetSmgr(rel);
 
        /*
         * If we haven't cached the size of the FSM yet, check it first.  Also
@@ -541,19 +545,19 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend)
         * value might be stale.  (We send smgr inval messages on truncation, but
         * not on extension.)
         */
-       if (rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber ||
-               blkno >= rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM])
+       if (reln->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber ||
+               blkno >= reln->smgr_cached_nblocks[FSM_FORKNUM])
        {
                /* Invalidate the cache so smgrnblocks asks the kernel. */
-               rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber;
-               if (smgrexists(rel->rd_smgr, FSM_FORKNUM))
-                       smgrnblocks(rel->rd_smgr, FSM_FORKNUM);
+               reln->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber;
+               if (smgrexists(reln, FSM_FORKNUM))
+                       smgrnblocks(reln, FSM_FORKNUM);
                else
-                       rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = 0;
+                       reln->smgr_cached_nblocks[FSM_FORKNUM] = 0;
        }
 
        /* Handle requests beyond EOF */
-       if (blkno >= rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM])
+       if (blkno >= reln->smgr_cached_nblocks[FSM_FORKNUM])
        {
                if (extend)
                        fsm_extend(rel, blkno + 1);
@@ -603,6 +607,7 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
 {
        BlockNumber fsm_nblocks_now;
        PGAlignedBlock pg;
+       SMgrRelation reln;
 
        PageInit((Page) pg.data, BLCKSZ, 0);
 
@@ -618,28 +623,33 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
         */
        LockRelationForExtension(rel, ExclusiveLock);
 
-       /* Might have to re-open if a cache flush happened */
-       RelationOpenSmgr(rel);
+       /*
+        * Caution: re-using this smgr pointer could fail if the relcache entry
+        * gets closed.  It's safe as long as we only do smgr-level operations
+        * between here and the last use of the pointer.
+        */
+       reln = RelationGetSmgr(rel);
 
        /*
         * Create the FSM file first if it doesn't exist.  If
         * smgr_cached_nblocks[FSM_FORKNUM] is positive then it must exist, no
         * need for an smgrexists call.
         */
-       if ((rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == 0 ||
-                rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber) &&
-               !smgrexists(rel->rd_smgr, FSM_FORKNUM))
-               smgrcreate(rel->rd_smgr, FSM_FORKNUM, false);
+       if ((reln->smgr_cached_nblocks[FSM_FORKNUM] == 0 ||
+                reln->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber) &&
+               !smgrexists(reln, FSM_FORKNUM))
+               smgrcreate(reln, FSM_FORKNUM, false);
 
        /* Invalidate cache so that smgrnblocks() asks the kernel. */
-       rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber;
-       fsm_nblocks_now = smgrnblocks(rel->rd_smgr, FSM_FORKNUM);
+       reln->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber;
+       fsm_nblocks_now = smgrnblocks(reln, FSM_FORKNUM);
 
+       /* Extend as needed. */
        while (fsm_nblocks_now < fsm_nblocks)
        {
                PageSetChecksumInplace((Page) pg.data, fsm_nblocks_now);
 
-               smgrextend(rel->rd_smgr, FSM_FORKNUM, fsm_nblocks_now,
+               smgrextend(reln, FSM_FORKNUM, fsm_nblocks_now,
                                   pg.data, false);
                fsm_nblocks_now++;
        }
index 77d176a93482caabf14e667c9905fb3e3c0b4392..b4faa1c12381e9fdb0ec6e60c2a3b4b09bc12fc0 100644 (file)
@@ -24,6 +24,7 @@
 #include "rewrite/prs2lock.h"
 #include "storage/block.h"
 #include "storage/relfilenode.h"
+#include "storage/smgr.h"
 #include "utils/relcache.h"
 #include "utils/reltrigger.h"
 
@@ -53,8 +54,7 @@ typedef LockInfoData *LockInfo;
 typedef struct RelationData
 {
        RelFileNode rd_node;            /* relation physical identifier */
-       /* use "struct" here to avoid needing to include smgr.h: */
-       struct SMgrRelationData *rd_smgr;       /* cached file handle, or NULL */
+       SMgrRelation rd_smgr;           /* cached file handle, or NULL */
        int                     rd_refcnt;              /* reference count */
        BackendId       rd_backend;             /* owning backend id, if temporary relation */
        bool            rd_islocaltemp; /* rel is a temp rel of this session */
@@ -528,14 +528,25 @@ typedef struct ViewOptions
         ((relation)->rd_rel->relfilenode == InvalidOid))
 
 /*
- * RelationOpenSmgr
- *             Open the relation at the smgr level, if not already done.
- */
-#define RelationOpenSmgr(relation) \
-       do { \
-               if ((relation)->rd_smgr == NULL) \
-                       smgrsetowner(&((relation)->rd_smgr), smgropen((relation)->rd_node, (relation)->rd_backend)); \
-       } while (0)
+ * RelationGetSmgr
+ *             Returns smgr file handle for a relation, opening it if needed.
+ *
+ * Very little code is authorized to touch rel->rd_smgr directly.  Instead
+ * use this function to fetch its value.
+ *
+ * Note: since a relcache flush can cause the file handle to be closed again,
+ * it's unwise to hold onto the pointer returned by this function for any
+ * long period.  Recommended practice is to just re-execute RelationGetSmgr
+ * each time you need to access the SMgrRelation.  It's quite cheap in
+ * comparison to whatever an smgr function is going to do.
+ */
+static inline SMgrRelation
+RelationGetSmgr(Relation rel)
+{
+       if (unlikely(rel->rd_smgr == NULL))
+               smgrsetowner(&(rel->rd_smgr), smgropen(rel->rd_node, rel->rd_backend));
+       return rel->rd_smgr;
+}
 
 /*
  * RelationCloseSmgr
@@ -557,7 +568,8 @@ typedef struct ViewOptions
  *             Fetch relation's current insertion target block.
  *
  * Returns InvalidBlockNumber if there is no current target block.  Note
- * that the target block status is discarded on any smgr-level invalidation.
+ * that the target block status is discarded on any smgr-level invalidation,
+ * so there's no need to re-open the smgr handle if it's not currently open.
  */
 #define RelationGetTargetBlock(relation) \
        ( (relation)->rd_smgr != NULL ? (relation)->rd_smgr->smgr_targblock : InvalidBlockNumber )
@@ -568,8 +580,7 @@ typedef struct ViewOptions
  */
 #define RelationSetTargetBlock(relation, targblock) \
        do { \
-               RelationOpenSmgr(relation); \
-               (relation)->rd_smgr->smgr_targblock = (targblock); \
+               RelationGetSmgr(relation)->smgr_targblock = (targblock); \
        } while (0)
 
 /*