diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 1a0606704ab..26272ef8666 100644 --- 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 4b2186b8dda..f77a4c8ffa3 100644 --- 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 13d4e3b4a36..19923bc3e08 100644 --- 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 f3deb47a97b..7dbc103f4da 100644 --- 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 1372bb638fd..e4eb0eb34ae 100644 --- 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 ecef0ff0724..73485c58191 100644 --- 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 11f21db992f..866bfbfb31e 100644 --- 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 6aecc143e7b..922744e04db 100644 --- 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 8d9bc8f4403..8d44b932317 100644 --- 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 489c72582d3..799ea8ea196 100644 --- 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 64dfe06b261..e872eb70b77 100644 --- 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 01684685ea4..c2e83d3ee5e 100644 --- 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 d177b9f2338..ff0b4fb5c2d 100644 --- 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 b40bd440cf0..4d3cb2e5dbd 100644 --- 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 faab1616759..ceccbdb8feb 100644 --- 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 0403fc086ab..6dcf1602a6a 100644 --- 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 5c76e7a39ba..3525a771af5 100644 --- 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 9f4d1da2fe3..a57fe611877 100644 --- 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 303f82aa233..e256c005a7a 100644 --- 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 c17b3f49dd0..f4c78dbfe4f 100644 --- 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 243f3c72af9..1531d18dadb 100644 --- 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) /*