Replace RelationOpenSmgr() with RelationGetSmgr().

This is a back-patch of the v15-era commit f10f0ae42 into older
supported branches.  The idea is to design out bugs in which an
ill-timed relcache flush clears rel->rd_smgr partway through
some code sequence that wasn't expecting that.  We had another
report today of a corner case that reliably crashes v14 under
debug_discard_caches (nee CLOBBER_CACHE_ALWAYS), and therefore
would crash once in a blue moon in the field.  We're unlikely
to get rid of all such code paths unless we adopt the more
rigorous coding rules instituted by f10f0ae42.  Therefore,
even though this is a bit invasive, it's time to back-patch.
Some comfort can be taken in the fact that f10f0ae42 has been
in v15 for 16 months without problems.

I left the RelationOpenSmgr macro present in the back branches,
even though no core code should use it anymore, in order to not break
third-party extensions in minor releases.  Such extensions might opt
to start using RelationGetSmgr instead, to reduce their code
differential between v15 and earlier branches.  This carries a hazard
of failing to compile against headers from existing minor releases.
However, once compiled the extension should work fine even with such
releases, because RelationGetSmgr is a "static inline" function so
it creates no link-time dependency.  So depending on distribution
practices, that might be an OK tradeoff.

Per report from Spyridon Dimitrios Agathos.  Original patch
by Amul Sul.

Discussion: https://postgr.es/m/CAFM5RaqdgyusQvmWkyPYaWMwoK5gigdtW-7HcgHgOeAw7mqJ_Q@mail.gmail.com
Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 1a0606704ab3e39a3bde9a1d7ae1cd5f758af744..26272ef86669a676037ca8a351fb7f1fef2f943f 100644 (file)
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -301,8 +301,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
    {
        bool    heapkeyspace;
 
-       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",

diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 4b2186b8ddaf47f721587f309a310548c7ff3b0d..f77a4c8ffa3b645a22882329a50df97b56d20246 100644 (file)
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -179,9 +179,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);
 
    /*
@@ -189,7 +189,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);
 }
 
 /*

diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index 13d4e3b4a36e1e2df219153b905b0358f5756251..19923bc3e08f428ff5c7ca2949e8072965a78c5b 100644 (file)
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -518,15 +518,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;

diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c
index f3deb47a97b71b9fd6521b66ff0d676c6f0105a2..7dbc103f4dadeb921a97edd7898e94da3653eb11 100644 (file)
--- a/contrib/pg_prewarm/pg_prewarm.c
+++ b/contrib/pg_prewarm/pg_prewarm.c
@@ -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;
        }
    }

diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 1372bb638fd0c21efa986f42d94864ea7c62cc75..e4eb0eb34ae6d4963b0d2ec5bfc5b1f432cdedc0 100644 (file)
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -389,8 +389,8 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
    /* Only some relkinds have a visibility map */
    check_relation_relkind(rel);
 
-   RelationOpenSmgr(rel);
-   rel->rd_smgr->smgr_vm_nblocks = InvalidBlockNumber;
+   /* Forcibly reset cached file size */
+   RelationGetSmgr(rel)->smgr_vm_nblocks = InvalidBlockNumber;
 
    visibilitymap_truncate(rel, 0);
 

diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index ecef0ff0724ae2ce0d9e6b41060631cee55bf93e..73485c58191b8e1ed2aaedddab8c55bb1a166a6a 100644 (file)
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -514,7 +514,7 @@ gistBuildCallback(Relation index,
     */
    if ((buildstate->bufferingMode == 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->bufferingMode == GIST_BUFFERING_STATS &&
         buildstate->indtuples >= BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET))
    {

diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 11f21db992fa50a4df0a3ae5e888efd52bbf393e..866bfbfb31edf3a6f37af1b971de84df18b54881 100644 (file)
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -1031,9 +1031,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;
 }

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 6aecc143e7b2d6b75b5d1e351ce642d678542d7f..922744e04db46db8926804be91e283448ec9953f 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -9098,8 +9098,7 @@ heap_sync(Relation rel)
 
    /* main heap */
    FlushRelationBuffers(rel);
-   /* FlushRelationBuffers will have opened rd_smgr */
-   smgrimmedsync(rel->rd_smgr, MAIN_FORKNUM);
+   smgrimmedsync(RelationGetSmgr(rel), MAIN_FORKNUM);
 
    /* FSM is not critical, don't bother syncing it */
 
@@ -9110,7 +9109,7 @@ heap_sync(Relation rel)
 
        toastrel = table_open(rel->rd_rel->reltoastrelid, AccessShareLock);
        FlushRelationBuffers(toastrel);
-       smgrimmedsync(toastrel->rd_smgr, MAIN_FORKNUM);
+       smgrimmedsync(RelationGetSmgr(toastrel), MAIN_FORKNUM);
        table_close(toastrel, AccessShareLock);
    }
 }

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 8d9bc8f4403d06ae2506d393cc0d9fb62b58a027..8d44b932317172f61ae5528f62fad83882b20ecd 100644 (file)
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -643,7 +643,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,
@@ -663,14 +662,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);
 
@@ -682,7 +681,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);
        }
    }
@@ -2045,18 +2044,23 @@ static uint64
 heapam_relation_size(Relation rel, ForkNumber forkNumber)
 {
    uint64      nblocks = 0;
+   SMgrRelation reln;
 
-   /* Open it at the smgr level if not already done */
-   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);
 
    /* 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(reln, i);
    }
    else
-       nblocks = smgrnblocks(rel->rd_smgr, forkNumber);
+       nblocks = smgrnblocks(reln, forkNumber);
 
    return nblocks * BLCKSZ;
 }

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 489c72582d3bcc2c77f5d4ca9827a7d21bef1ace..799ea8ea196b575dc1cd032e68e11d04d6dcd866 100644 (file)
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -336,11 +336,10 @@ end_heap_rewrite(RewriteState state)
                        state->rs_blockno,
                        state->rs_buffer,
                        true);
-       RelationOpenSmgr(state->rs_new_rel);
 
        PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
 
-       smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno,
+       smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM, state->rs_blockno,
                   (char *) state->rs_buffer, true);
    }
 
@@ -708,11 +707,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
             * 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++;

diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 64dfe06b261f04c120e283098bd0527b04389203..e872eb70b772198585222e444b59b07260380e8b 100644 (file)
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -452,13 +452,11 @@ visibilitymap_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;
 
    /*
@@ -525,14 +523,14 @@ visibilitymap_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;
    }
 
    /* Truncate the unused VM pages, and send smgr inval message */
-   smgrtruncate(rel->rd_smgr, VISIBILITYMAP_FORKNUM, newnblocks);
+   smgrtruncate(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM, newnblocks);
 
    /*
     * We might as well update the local smgr_vm_nblocks setting. smgrtruncate
@@ -554,31 +552,30 @@ 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_vm_nblocks == InvalidBlockNumber)
+   if (reln->smgr_vm_nblocks == InvalidBlockNumber)
    {
-       if (smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM))
-           rel->rd_smgr->smgr_vm_nblocks = smgrnblocks(rel->rd_smgr,
-                                                       VISIBILITYMAP_FORKNUM);
+       if (smgrexists(reln, VISIBILITYMAP_FORKNUM))
+           reln->smgr_vm_nblocks = smgrnblocks(reln,
+                                               VISIBILITYMAP_FORKNUM);
        else
-           rel->rd_smgr->smgr_vm_nblocks = 0;
+           reln->smgr_vm_nblocks = 0;
    }
 
    /* Handle requests beyond EOF */
-   if (blkno >= rel->rd_smgr->smgr_vm_nblocks)
+   if (blkno >= reln->smgr_vm_nblocks)
    {
        if (extend)
            vm_extend(rel, blkno + 1);
@@ -626,6 +623,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
 {
    BlockNumber vm_nblocks_now;
    PGAlignedBlock pg;
+   SMgrRelation reln;
 
    PageInit((Page) pg.data, BLCKSZ, 0);
 
@@ -641,26 +639,30 @@ 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_vm_nblocks == 0 ||
-        rel->rd_smgr->smgr_vm_nblocks == InvalidBlockNumber) &&
-       !smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM))
-       smgrcreate(rel->rd_smgr, VISIBILITYMAP_FORKNUM, false);
+   if ((reln->smgr_vm_nblocks == 0 ||
+        reln->smgr_vm_nblocks == InvalidBlockNumber) &&
+       !smgrexists(reln, VISIBILITYMAP_FORKNUM))
+       smgrcreate(reln, VISIBILITYMAP_FORKNUM, false);
 
-   vm_nblocks_now = smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM);
+   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,
+       smgrextend(reln, VISIBILITYMAP_FORKNUM, vm_nblocks_now,
                   pg.data, false);
        vm_nblocks_now++;
    }
@@ -672,10 +674,10 @@ 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);
 
    /* Update local cache with the up-to-date size */
-   rel->rd_smgr->smgr_vm_nblocks = vm_nblocks_now;
+   reln->smgr_vm_nblocks = vm_nblocks_now;
 
    UnlockRelationForExtension(rel, ExclusiveLock);
 }

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 01684685ea4068a630ef1c423ca8f28405ef8869..c2e83d3ee5e95f38733ea175c379f35278fe75e9 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -170,9 +170,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);
 
    /*
@@ -180,7 +180,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);
 }
 
 /*

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index d177b9f233811b72e0e588688d1c1e6a21f34640..ff0b4fb5c2d0378d1c16b3e81f98691ec649eed9 100644 (file)
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -655,9 +655,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)
    {
@@ -677,7 +674,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);
@@ -692,14 +689,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);
    }
 
@@ -1305,10 +1302,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
     * occurs.
     */
    if (RelationNeedsWAL(wstate->index))
-   {
-       RelationOpenSmgr(wstate->index);
-       smgrimmedsync(wstate->index->rd_smgr, MAIN_FORKNUM);
-   }
+       smgrimmedsync(RelationGetSmgr(wstate->index), MAIN_FORKNUM);
 }
 
 /*

diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c
index b40bd440cf09e1723e2515711f193dfc70e801a2..4d3cb2e5dbd549156400a0dccb78cae9828c89c6 100644 (file)
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -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);
 }
 
 /*

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index faab16167594c07b3f007c6fda6f47a347c53e3f..ceccbdb8feb360244a7dd3f2a9c25dea1df0e35b 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -415,8 +415,6 @@ heap_create(const char *relname,
     */
    if (create_storage)
    {
-       RelationOpenSmgr(rel);
-
        switch (rel->rd_rel->relkind)
        {
            case RELKIND_VIEW:

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 0403fc086ab382cb65e15c2f57946fc9a4514013..6dcf1602a6a913bfb8df7ae136e17b8ba030ace3 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3009,10 +3009,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);
    }
 

diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 5c76e7a39baf1c76bde686fc0c12fc2b156634b1..3525a771af5e02c08253c5e316c2a78a01906838 100644 (file)
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -232,24 +232,24 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
 {
    bool        fsm;
    bool        vm;
-
-   /* 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 here.)
     */
-   rel->rd_smgr->smgr_targblock = InvalidBlockNumber;
-   rel->rd_smgr->smgr_fsm_nblocks = InvalidBlockNumber;
-   rel->rd_smgr->smgr_vm_nblocks = InvalidBlockNumber;
+   reln = RelationGetSmgr(rel);
+   reln->smgr_targblock = InvalidBlockNumber;
+   reln->smgr_fsm_nblocks = InvalidBlockNumber;
+   reln->smgr_vm_nblocks = InvalidBlockNumber;
 
    /* Truncate the FSM first if it exists */
-   fsm = smgrexists(rel->rd_smgr, FSM_FORKNUM);
+   fsm = smgrexists(reln, FSM_FORKNUM);
    if (fsm)
        FreeSpaceMapTruncateRel(rel, nblocks);
 
    /* Truncate the visibility map too if it exists. */
-   vm = smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM);
+   vm = smgrexists(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM);
    if (vm)
        visibilitymap_truncate(rel, nblocks);
 
@@ -312,7 +312,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
     * longer exist after truncation is complete, and then truncate the
     * corresponding files on disk.
     */
-   smgrtruncate(rel->rd_smgr, MAIN_FORKNUM, nblocks);
+   smgrtruncate(RelationGetSmgr(rel), MAIN_FORKNUM, nblocks);
 
    /* We've done all the critical work, so checkpoints are OK now. */
    MyProc->delayChkptEnd = false;
@@ -324,6 +324,12 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
  * 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,
@@ -364,13 +370,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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9f4d1da2fe3ed4f74aec032e0d4134becd2292eb..a57fe6118775372e81b66c826bb11bbc46fff433 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13317,7 +13317,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,
@@ -13337,14 +13336,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);
 
@@ -13356,7 +13355,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);
        }
    }

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 303f82aa233c7a640edf601256a6d76e1c2478d1..e256c005a7ae4db004f0714eab2d8cac8e705032 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -533,9 +533,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 */
@@ -545,7 +542,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)
                     errmsg("cannot access temporary tables of other sessions")));
 
        /* pass it off to localbuf.c */
-       LocalPrefetchBuffer(reln->rd_smgr, forkNum, blockNum);
+       LocalPrefetchBuffer(RelationGetSmgr(reln), forkNum, blockNum);
    }
    else
    {
@@ -555,7 +552,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)
        int         buf_id;
 
        /* create a tag so we can lookup the buffer */
-       INIT_BUFFERTAG(newTag, reln->rd_smgr->smgr_rnode.node,
+       INIT_BUFFERTAG(newTag, RelationGetSmgr(reln)->smgr_rnode.node,
                       forkNum, blockNum);
 
        /* determine its hash code and partition lock ID */
@@ -569,7 +566,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)
 
        /* If not in buffers, initiate prefetch */
        if (buf_id < 0)
-           smgrprefetch(reln->rd_smgr, forkNum, blockNum);
+           smgrprefetch(RelationGetSmgr(reln), forkNum, blockNum);
 
        /*
         * If the block *is* in buffers, we do nothing.  This is not really
@@ -645,9 +642,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
@@ -663,7 +657,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);
@@ -2814,10 +2808,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:
@@ -3204,9 +3195,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++)
@@ -3231,7 +3219,7 @@ FlushRelationBuffers(Relation rel)
 
                PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-               smgrwrite(rel->rd_smgr,
+               smgrwrite(RelationGetSmgr(rel),
                          bufHdr->tag.forkNum,
                          bufHdr->tag.blockNum,
                          localpage,
@@ -3272,7 +3260,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);
        }

diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index c17b3f49dd0ef9f14dca4542711a8554aa6f64e5..f4c78dbfe4f1ca46bb5db43fb279369a9f04c0f7 100644 (file)
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -263,13 +263,11 @@ FreeSpaceMapTruncateRel(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;
 
    /* Get the location in the FSM of the first removed heap block */
@@ -314,12 +312,12 @@ FreeSpaceMapTruncateRel(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;             /* nothing to do; the FSM was already smaller */
    }
 
    /* Truncate the unused FSM pages, and send smgr inval message */
-   smgrtruncate(rel->rd_smgr, FSM_FORKNUM, new_nfsmblocks);
+   smgrtruncate(RelationGetSmgr(rel), FSM_FORKNUM, new_nfsmblocks);
 
    /*
     * We might as well update the local smgr_fsm_nblocks setting.
@@ -546,8 +544,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
@@ -555,18 +559,18 @@ 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_fsm_nblocks == InvalidBlockNumber ||
-       blkno >= rel->rd_smgr->smgr_fsm_nblocks)
+   if (reln->smgr_fsm_nblocks == InvalidBlockNumber ||
+       blkno >= reln->smgr_fsm_nblocks)
    {
-       if (smgrexists(rel->rd_smgr, FSM_FORKNUM))
-           rel->rd_smgr->smgr_fsm_nblocks = smgrnblocks(rel->rd_smgr,
-                                                        FSM_FORKNUM);
+       if (smgrexists(reln, FSM_FORKNUM))
+           reln->smgr_fsm_nblocks = smgrnblocks(reln,
+                                                FSM_FORKNUM);
        else
-           rel->rd_smgr->smgr_fsm_nblocks = 0;
+           reln->smgr_fsm_nblocks = 0;
    }
 
    /* Handle requests beyond EOF */
-   if (blkno >= rel->rd_smgr->smgr_fsm_nblocks)
+   if (blkno >= reln->smgr_fsm_nblocks)
    {
        if (extend)
            fsm_extend(rel, blkno + 1);
@@ -616,6 +620,7 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
 {
    BlockNumber fsm_nblocks_now;
    PGAlignedBlock pg;
+   SMgrRelation reln;
 
    PageInit((Page) pg.data, BLCKSZ, 0);
 
@@ -631,31 +636,35 @@ 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_fsm_nblocks is
     * positive then it must exist, no need for an smgrexists call.
     */
-   if ((rel->rd_smgr->smgr_fsm_nblocks == 0 ||
-        rel->rd_smgr->smgr_fsm_nblocks == InvalidBlockNumber) &&
-       !smgrexists(rel->rd_smgr, FSM_FORKNUM))
-       smgrcreate(rel->rd_smgr, FSM_FORKNUM, false);
+   if ((reln->smgr_fsm_nblocks == 0 ||
+        reln->smgr_fsm_nblocks == InvalidBlockNumber) &&
+       !smgrexists(reln, FSM_FORKNUM))
+       smgrcreate(reln, FSM_FORKNUM, false);
 
-   fsm_nblocks_now = smgrnblocks(rel->rd_smgr, FSM_FORKNUM);
+   fsm_nblocks_now = smgrnblocks(reln, FSM_FORKNUM);
 
    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++;
    }
 
    /* Update local cache with the up-to-date size */
-   rel->rd_smgr->smgr_fsm_nblocks = fsm_nblocks_now;
+   reln->smgr_fsm_nblocks = fsm_nblocks_now;
 
    UnlockRelationForExtension(rel, ExclusiveLock);
 }

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 243f3c72af9f77495baba8e8c8be8e13870c41d9..1531d18dadbbc4e7ad0cd994d51ddf22b83bf9e4 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -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 */
@@ -472,9 +472,33 @@ typedef struct ViewOptions
    (RELKIND_HAS_STORAGE((relation)->rd_rel->relkind) && \
     ((relation)->rd_rel->relfilenode == InvalidOid))
 
+/*
+ * 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;
+}
+
 /*
  * RelationOpenSmgr
  *     Open the relation at the smgr level, if not already done.
+ *
+ * XXX this is now deprecated, and should not be used in new code.
+ * Instead, call RelationGetSmgr in place of fetching rd_smgr directly.
  */
 #define RelationOpenSmgr(relation) \
    do { \
@@ -502,7 +526,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 )
@@ -513,8 +538,7 @@ typedef struct ViewOptions
  */
 #define RelationSetTargetBlock(relation, targblock) \
    do { \
-       RelationOpenSmgr(relation); \
-       (relation)->rd_smgr->smgr_targblock = (targblock); \
+       RelationGetSmgr(relation)->smgr_targblock = (targblock); \
    } while (0)
 
 /*