At Mon, 11 Jul 2022 13:51:12 -0400, Robert Haas <robertmh...@gmail.com> wrote
in
> On Fri, Jul 8, 2022 at 1:59 AM Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote:
> > The function CreateAndCopyRelationData exists since before b0a55e4329
> > but renamed since it takes RelFileLocators.
>
> I'm not very sold on this. I think that the places where you've
> replaced RelFileLocator with just RelFile in various functions might
> be an improvement, but the places where you've replaced Relation with
> RelFile seem to me to be worse. I don't really see that there's
> anything wrong with names like CreateAndCopyRelationData or
> FlushRelationsAllBuffers, and in general I prefer function names that
> are made up of whole words rather than parts of words.
Fair enough. My first thought was that Relation can represent both
Relation and "RelFile" but in the patch I choosed to make distinction
between them by associating respectively to the types of the primary
parameter (Relation or RelFileLocator). So I'm fine with Relation
instead since I see it more intuitive than RelFileLocator in the
function names.
The attached is that.
> > While working on this, I found that the following coment is wrong.
..
> I committed this.
Thanks!
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From dbb3dbebbea6aae29588bfccdbda28d0cf15a6fe Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Fri, 8 Jul 2022 13:20:31 +0900
Subject: [PATCH v2] Rename some *RelFileLocator* functions
A recent commit b0a55e4329 changed the term RelFileNode to
RelFileLocator. Accordingly the names of some functions were
changed. However, since RelFileLocator doesn't fully cover what
RelFileNode represented, some of the function names have gotten less
intuitive.
This commit replaces all uses of RelFileLocator in function names with
Relation because that uses of RelFileLocator actually mean storage
object.
---
src/backend/storage/buffer/bufmgr.c | 63 +++++++++++++--------------
src/backend/storage/buffer/localbuf.c | 16 +++----
src/backend/storage/smgr/smgr.c | 4 +-
src/include/storage/buf_internals.h | 7 +--
src/include/storage/bufmgr.h | 10 ++---
5 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e257ae23e4..c7d7abcd73 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -121,7 +121,7 @@ typedef struct CkptTsStatus
* Type for array used to sort SMgrRelations
*
* FlushRelationsAllBuffers shares the same comparator function with
- * DropRelFileLocatorsAllBuffers. Pointer to this struct and RelFileLocator must be
+ * DropRelationsAllBuffers. Pointer to this struct and RelFileLocator must be
* compatible.
*/
typedef struct SMgrSortArray
@@ -483,10 +483,10 @@ static BufferDesc *BufferAlloc(SMgrRelation smgr,
BufferAccessStrategy strategy,
bool *foundPtr);
static void FlushBuffer(BufferDesc *buf, SMgrRelation reln);
-static void FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator,
- ForkNumber forkNum,
- BlockNumber nForkBlock,
- BlockNumber firstDelBlock);
+static void FindAndDropRelationBuffers(RelFileLocator rlocator,
+ ForkNumber forkNum,
+ BlockNumber nForkBlock,
+ BlockNumber firstDelBlock);
static void RelationCopyStorageUsingBuffer(Relation src, Relation dst,
ForkNumber forkNum,
bool isunlogged);
@@ -3026,7 +3026,7 @@ BufferGetLSNAtomic(Buffer buffer)
}
/* ---------------------------------------------------------------------
- * DropRelFileLocatorBuffers
+ * DropRelationBuffers
*
* This function removes from the buffer pool all the pages of the
* specified relation forks that have block numbers >= firstDelBlock.
@@ -3047,8 +3047,8 @@ BufferGetLSNAtomic(Buffer buffer)
* --------------------------------------------------------------------
*/
void
-DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
- int nforks, BlockNumber *firstDelBlock)
+DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
+ int nforks, BlockNumber *firstDelBlock)
{
int i;
int j;
@@ -3064,8 +3064,8 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
if (rlocator.backend == MyBackendId)
{
for (j = 0; j < nforks; j++)
- DropRelFileLocatorLocalBuffers(rlocator.locator, forkNum[j],
- firstDelBlock[j]);
+ DropRelationLocalBuffers(rlocator.locator, forkNum[j],
+ firstDelBlock[j]);
}
return;
}
@@ -3115,8 +3115,8 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)
{
for (j = 0; j < nforks; j++)
- FindAndDropRelFileLocatorBuffers(rlocator.locator, forkNum[j],
- nForkBlock[j], firstDelBlock[j]);
+ FindAndDropRelationBuffers(rlocator.locator, forkNum[j],
+ nForkBlock[j], firstDelBlock[j]);
return;
}
@@ -3162,16 +3162,15 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
}
/* ---------------------------------------------------------------------
- * DropRelFileLocatorsAllBuffers
+ * DropRelationsAllBuffers
*
* This function removes from the buffer pool all the pages of all
* forks of the specified relations. It's equivalent to calling
- * DropRelFileLocatorBuffers once per fork per relation with
- * firstDelBlock = 0.
- * --------------------------------------------------------------------
+ * DropRelationBuffers once per fork per relation with firstDelBlock = 0.
+ * --------------------------------------------------------------------
*/
void
-DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
+DropRelationsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
{
int i;
int j;
@@ -3194,7 +3193,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
if (RelFileLocatorBackendIsTemp(smgr_reln[i]->smgr_rlocator))
{
if (smgr_reln[i]->smgr_rlocator.backend == MyBackendId)
- DropRelFileLocatorAllLocalBuffers(smgr_reln[i]->smgr_rlocator.locator);
+ DropRelationAllLocalBuffers(smgr_reln[i]->smgr_rlocator.locator);
}
else
rels[n++] = smgr_reln[i];
@@ -3219,7 +3218,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
/*
* We can avoid scanning the entire buffer pool if we know the exact size
- * of each of the given relation forks. See DropRelFileLocatorBuffers.
+ * of each of the given relation forks. See DropRelationBuffers.
*/
for (i = 0; i < n && cached; i++)
{
@@ -3257,8 +3256,8 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
continue;
/* drop all the buffers for a particular relation fork */
- FindAndDropRelFileLocatorBuffers(rels[i]->smgr_rlocator.locator,
- j, block[i][j], 0);
+ FindAndDropRelationBuffers(rels[i]->smgr_rlocator.locator,
+ j, block[i][j], 0);
}
}
@@ -3291,7 +3290,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelationBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
@@ -3331,7 +3330,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
}
/* ---------------------------------------------------------------------
- * FindAndDropRelFileLocatorBuffers
+ * FindAndDropRelationBuffers
*
* This function performs look up in BufMapping table and removes from the
* buffer pool all the pages of the specified relation fork that has block
@@ -3340,9 +3339,9 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
* --------------------------------------------------------------------
*/
static void
-FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber nForkBlock,
- BlockNumber firstDelBlock)
+FindAndDropRelationBuffers(RelFileLocator rlocator, ForkNumber forkNum,
+ BlockNumber nForkBlock,
+ BlockNumber firstDelBlock)
{
BlockNumber curBlock;
@@ -3397,7 +3396,7 @@ FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator, ForkNumber forkNum,
* bothering to write them out first. This is used when we destroy a
* database, to avoid trying to flush data to disk when the directory
* tree no longer exists. Implementation is pretty similar to
- * DropRelFileLocatorBuffers() which is for destroying just one relation.
+ * DropRelationBuffers() which is for destroying just one relation.
* --------------------------------------------------------------------
*/
void
@@ -3416,7 +3415,7 @@ DropDatabaseBuffers(Oid dbid)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelationBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (bufHdr->tag.rlocator.dbOid != dbid)
@@ -3561,7 +3560,7 @@ FlushRelationBuffers(Relation rel)
bufHdr = GetBufferDescriptor(i);
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelationBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (!RelFileLocatorEquals(bufHdr->tag.rlocator, rel->rd_locator))
@@ -3616,7 +3615,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
/*
* Save the bsearch overhead for low number of relations to sync. See
- * DropRelFileLocatorsAllBuffers for details.
+ * DropRelationsAllBuffers for details.
*/
use_bsearch = nrels > RELS_BSEARCH_THRESHOLD;
@@ -3634,7 +3633,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelationBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
@@ -3864,7 +3863,7 @@ FlushDatabaseBuffers(Oid dbid)
bufHdr = GetBufferDescriptor(i);
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelationBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (bufHdr->tag.rlocator.dbOid != dbid)
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 41a08076b3..9c038851d7 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -312,7 +312,7 @@ MarkLocalBufferDirty(Buffer buffer)
}
/*
- * DropRelFileLocatorLocalBuffers
+ * DropRelationLocalBuffers
* This function removes from the buffer pool all the pages of the
* specified relation that have block numbers >= firstDelBlock.
* (In particular, with firstDelBlock = 0, all pages are removed.)
@@ -320,11 +320,11 @@ MarkLocalBufferDirty(Buffer buffer)
* out first. Therefore, this is NOT rollback-able, and so should be
* used only with extreme caution!
*
- * See DropRelFileLocatorBuffers in bufmgr.c for more notes.
+ * See DropRelationBuffers in bufmgr.c for more notes.
*/
void
-DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber firstDelBlock)
+DropRelationLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
+ BlockNumber firstDelBlock)
{
int i;
@@ -363,14 +363,14 @@ DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
}
/*
- * DropRelFileLocatorAllLocalBuffers
+ * DropRelationAllLocalBuffers
* This function removes from the buffer pool all pages of all forks
* of the specified relation.
*
- * See DropRelFileLocatorsAllBuffers in bufmgr.c for more notes.
+ * See DropRelationsAllBuffers in bufmgr.c for more notes.
*/
void
-DropRelFileLocatorAllLocalBuffers(RelFileLocator rlocator)
+DropRelationAllLocalBuffers(RelFileLocator rlocator)
{
int i;
@@ -589,7 +589,7 @@ AtProcExit_LocalBuffers(void)
{
/*
* We shouldn't be holding any remaining pins; if we are, and assertions
- * aren't enabled, we'll fail later in DropRelFileLocatorBuffers while
+ * aren't enabled, we'll fail later in DropRelationBuffers while
* trying to drop the temp rels.
*/
CheckForLocalBufferLeaks();
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index b21d8c3822..c1a5febcbf 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -430,7 +430,7 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
* Get rid of any remaining buffers for the relations. bufmgr will just
* drop them without bothering to write the contents.
*/
- DropRelFileLocatorsAllBuffers(rels, nrels);
+ DropRelationsAllBuffers(rels, nrels);
/*
* create an array which contains all relations to be dropped, and close
@@ -631,7 +631,7 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nb
* Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will
* just drop them without bothering to write the contents.
*/
- DropRelFileLocatorBuffers(reln, forknum, nforks, nblocks);
+ DropRelationBuffers(reln, forknum, nforks, nblocks);
/*
* Send a shared-inval message to force other backends to close any smgr
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index aded5e8f7e..69e45900ba 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -337,9 +337,10 @@ extern PrefetchBufferResult PrefetchLocalBuffer(SMgrRelation smgr,
extern BufferDesc *LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum,
BlockNumber blockNum, bool *foundPtr);
extern void MarkLocalBufferDirty(Buffer buffer);
-extern void DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber firstDelBlock);
-extern void DropRelFileLocatorAllLocalBuffers(RelFileLocator rlocator);
+extern void DropRelationLocalBuffers(RelFileLocator rlocator,
+ ForkNumber forkNum,
+ BlockNumber firstDelBlock);
+extern void DropRelationAllLocalBuffers(RelFileLocator rlocator);
extern void AtEOXact_LocalBuffers(bool isCommit);
#endif /* BUFMGR_INTERNALS_H */
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 7bcfaac272..d7ced19c14 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -208,11 +208,11 @@ extern void CreateAndCopyRelationData(RelFileLocator src_rlocator,
RelFileLocator dst_rlocator,
bool permanent);
extern void FlushDatabaseBuffers(Oid dbid);
-extern void DropRelFileLocatorBuffers(struct SMgrRelationData *smgr_reln,
- ForkNumber *forkNum,
- int nforks, BlockNumber *firstDelBlock);
-extern void DropRelFileLocatorsAllBuffers(struct SMgrRelationData **smgr_reln,
- int nlocators);
+extern void DropRelationBuffers(struct SMgrRelationData *smgr_reln,
+ ForkNumber *forkNum,
+ int nforks, BlockNumber *firstDelBlock);
+extern void DropRelationsAllBuffers(struct SMgrRelationData **smgr_reln,
+ int nlocators);
extern void DropDatabaseBuffers(Oid dbid);
#define RelationGetNumberOfBlocks(reln) \
--
2.31.1
