]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Replace RelationOpenSmgr() with RelationGetSmgr().
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 17 Nov 2022 21:54:31 +0000 (16:54 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 17 Nov 2022 21:54:31 +0000 (16:54 -0500)
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

20 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.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/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 4c4ca02891f588a070cdb369852237f86abf4a2c..5b75c891efcd68ed52f914d99bc474ac25946498 100644 (file)
@@ -278,8 +278,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed)
 
        if (btree_index_mainfork_expected(indrel))
        {
-               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 9f223d3b2a7bd4c4247016b11557850b8c25b9df..cd85703c1be764cc6fbca90f690ade3c0e514076 100644 (file)
@@ -178,9 +178,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);
 
        /*
@@ -188,7 +188,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 33b907f13bb9b84dc1e034d324cfa957aa3b871a..5b2a6e2d3ac8042c69df7d656d790daa6642bd3b 100644 (file)
@@ -522,15 +522,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 1f4bfb8c0d4b56fec8bcfa93dfaea350a3e95c2e..c89d3b1390dc10981d8a5d5b8c2ab9275c8e1892 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 944dea66fc8641bb744fe7d520014116721da2a5..8009bb381ccbb3eaeff73f1cb807d456d0afb68f 100644 (file)
@@ -387,8 +387,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);
 
index b9c4e27e1a5ff881293595b93574fa68d5fe67c6..e84bc2e4c150d5ca571d742d742461a1666085b8 100644 (file)
@@ -513,7 +513,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))
        {
index 1d2d3c0134466ac8c1906258b0a0cad1f2b09b1f..f698c5fc058e5b997ed4c91fb7853164b0028e22 100644 (file)
@@ -1041,9 +1041,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 f17d434c9d5eb63429a763dba68893238a61dfd3..848062375e75ac69036751b8644f31636080918d 100644 (file)
@@ -9538,8 +9538,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 */
 
@@ -9550,7 +9549,7 @@ heap_sync(Relation rel)
 
                toastrel = heap_open(rel->rd_rel->reltoastrelid, AccessShareLock);
                FlushRelationBuffers(toastrel);
-               smgrimmedsync(toastrel->rd_smgr, MAIN_FORKNUM);
+               smgrimmedsync(RelationGetSmgr(toastrel), MAIN_FORKNUM);
                heap_close(toastrel, AccessShareLock);
        }
 }
index b4092bb73252dc3dfa19751099c388fdd869d654..b5b9c189b718360452985520b7a5f6b0b5db0428 100644 (file)
@@ -337,11 +337,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);
        }
 
@@ -709,11 +708,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++;
index 695567b4b0d5edea59e800fb5754c420b6e2bbfb..62bb9e971d5eb3c64d8fb8e93b796a9da875e7f6 100644 (file)
@@ -472,13 +472,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;
 
        /*
@@ -545,14 +543,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
@@ -574,31 +572,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);
@@ -646,6 +643,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
 {
        BlockNumber vm_nblocks_now;
        PGAlignedBlock pg;
+       SMgrRelation reln;
 
        PageInit((Page) pg.data, BLCKSZ, 0);
 
@@ -661,26 +659,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++;
        }
@@ -692,10 +694,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);
 }
index fcb36ebcc0547d0f346dd77e847a278d2d8a3e44..18b370376dbe2034cd4649ce314ffc185a430c8f 100644 (file)
@@ -168,9 +168,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);
 
        /*
@@ -178,7 +178,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 9025c5c8c67a2f897d4f5de1aa9062281ccbe9cb..038aefe745a2924efecc05aeb77f7cb8007bccee 100644 (file)
@@ -610,9 +610,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)
        {
@@ -632,7 +629,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);
@@ -647,14 +644,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);
        }
 
@@ -1208,10 +1205,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);
 }
 
 /*
index 7dd0d61fbbca6365dae442c4978555574368d712..4b0103b2937d270d0e0a9732cf9a8c3b317c03c6 100644 (file)
@@ -177,27 +177,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);
 
        /*
@@ -205,7 +205,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 6a5a08199297bcde3842b76d2e970899708d4461..846c530154a7204bb4cf796407017bc5e14c8ba3 100644 (file)
@@ -371,10 +371,7 @@ heap_create(const char *relname,
         * demand.
         */
        if (create_storage)
-       {
-               RelationOpenSmgr(rel);
                RelationCreateStorage(rel->rd_node, relpersistence);
-       }
 
        /*
         * If a tablespace is specified, removal of that tablespace is normally
@@ -1411,10 +1408,9 @@ heap_create_init_fork(Relation rel)
        Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
                   rel->rd_rel->relkind == RELKIND_MATVIEW ||
                   rel->rd_rel->relkind == RELKIND_TOASTVALUE);
-       RelationOpenSmgr(rel);
-       smgrcreate(rel->rd_smgr, INIT_FORKNUM, false);
-       log_smgrcreate(&rel->rd_smgr->smgr_rnode.node, INIT_FORKNUM);
-       smgrimmedsync(rel->rd_smgr, INIT_FORKNUM);
+       smgrcreate(RelationGetSmgr(rel), INIT_FORKNUM, false);
+       log_smgrcreate(&(RelationGetSmgr(rel)->smgr_rnode.node), INIT_FORKNUM);
+       smgrimmedsync(RelationGetSmgr(rel), INIT_FORKNUM);
 }
 
 /*
index 234bf7f8548ecf703161c6b5b7919aff7f20abe7..6c71d41d65bbd2652321cbeb3f684f128a4c21d9 100644 (file)
@@ -2424,10 +2424,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_amroutine->ambuildempty(indexRelation);
        }
 
index 76a75a46b823cb9068838b5eaa136cda49eec148..65962148dd60b82c26a45e0a3b6ae99b13b2c907 100644 (file)
@@ -228,24 +228,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);
 
@@ -308,7 +308,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;
index 14faa61380acf836e34982726084f41a81a82156..f56219973e1b1ea6fd7e2249ed536310f0cedf01 100644 (file)
@@ -11955,8 +11955,6 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
        newrnode.spcNode = newTableSpace;
        dstrel = smgropen(newrnode, rel->rd_backend);
 
-       RelationOpenSmgr(rel);
-
        /*
         * Create and copy all forks of the relation, and schedule unlinking of
         * old physical files.
@@ -11967,13 +11965,13 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
        RelationCreateStorage(newrnode, rel->rd_rel->relpersistence);
 
        /* copy main fork */
-       copy_relation_data(rel->rd_smgr, dstrel, MAIN_FORKNUM,
+       copy_relation_data(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM,
                                           rel->rd_rel->relpersistence);
 
        /* copy those extra forks that exist */
        for (forkNum = MAIN_FORKNUM + 1; forkNum <= MAX_FORKNUM; forkNum++)
        {
-               if (smgrexists(rel->rd_smgr, forkNum))
+               if (smgrexists(RelationGetSmgr(rel), forkNum))
                {
                        smgrcreate(dstrel, forkNum, false);
 
@@ -11985,7 +11983,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
                                (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
                                 forkNum == INIT_FORKNUM))
                                log_smgrcreate(&newrnode, forkNum);
-                       copy_relation_data(rel->rd_smgr, dstrel, forkNum,
+                       copy_relation_data(RelationGetSmgr(rel), dstrel, forkNum,
                                                           rel->rd_rel->relpersistence);
                }
        }
index f0f3f02b2e20e9b3cb92e22d70551f28c9d982e1..5ea6bd6221d4a8cc6b698352f961b0a97948d6e3 100644 (file)
@@ -532,9 +532,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 */
@@ -544,7 +541,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
        {
@@ -554,7 +551,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 */
@@ -568,7 +565,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
@@ -644,9 +641,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
@@ -662,7 +656,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);
@@ -2798,10 +2792,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln)
 BlockNumber
 RelationGetNumberOfBlocksInFork(Relation relation, ForkNumber forkNum)
 {
-       /* Open it at the smgr level if not already done */
-       RelationOpenSmgr(relation);
-
-       return smgrnblocks(relation->rd_smgr, forkNum);
+       return smgrnblocks(RelationGetSmgr(relation), forkNum);
 }
 
 /*
@@ -3161,9 +3152,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++)
@@ -3188,7 +3176,7 @@ FlushRelationBuffers(Relation rel)
 
                                PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-                               smgrwrite(rel->rd_smgr,
+                               smgrwrite(RelationGetSmgr(rel),
                                                  bufHdr->tag.forkNum,
                                                  bufHdr->tag.blockNum,
                                                  localpage,
@@ -3229,7 +3217,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 7c4ad1c44947351dab3c713e2f2cfbf062e518fc..e4d84ca38c4248b1e8fabacd097c2bec5f8ecad2 100644 (file)
@@ -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);
 }
index 11a635964fd0861357059c8519e42dd90dda497a..40cc47ad25429c1065f8e2d202ae7337c2e7438a 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 */
@@ -468,9 +468,33 @@ typedef struct ViewOptions
 #define RelationIsMapped(relation) \
        ((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 { \
@@ -498,7 +522,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 )
@@ -509,8 +534,7 @@ typedef struct ViewOptions
  */
 #define RelationSetTargetBlock(relation, targblock) \
        do { \
-               RelationOpenSmgr(relation); \
-               (relation)->rd_smgr->smgr_targblock = (targblock); \
+               RelationGetSmgr(relation)->smgr_targblock = (targblock); \
        } while (0)
 
 /*