Re: Fix small typo, use InvalidRelFileNumber instead of InvalidOid

On 2024-Nov-08, Dilip Kumar wrote:

> It appears that we are passing InvalidOid instead of InvalidRelFileNumber
> when calling index_create(). While this isn’t technically a bug, since
> InvalidRelFileNumber is defined as InvalidOid, it’s preferable to use the
> correct identifier for clarity and consistency.

Oh ugh, this is quite a widespread problem actually, and it's just
because RelFileNumber is a typedef to Oid that the compiler doesn't
complain. In a very quick desultory scan, I found a bunch of similar
issues elsewhere, such as below (also the assignment to relNumber in
GetNewRelFileNumber). This isn't exhaustive by any means nor did I try
to compile it ... are you motivated to search for this more
exhaustively? You could try (temporarily) making RelFileNumber a
typedef that tricks the compiler into creating a warning or error for
every mischief, such as a struct, fix all the warnings/errors, then
revert the temporary change.

diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c
index 275615877eb..e2a73d88408 100644
--- a/src/backend/backup/basebackup_incremental.c
+++ b/src/backend/backup/basebackup_incremental.c
@@ -740,7 +740,7 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path,
*/
rlocator.spcOid = spcoid;
rlocator.dbOid = dboid;
- rlocator.relNumber = 0;
+ rlocator.relNumber = InvalidRelFileNumber;
if (BlockRefTableGetEntry(ib->brtab, &rlocator, MAIN_FORKNUM,
&limit_block) != NULL)
{
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 73a7592fb71..2f7f06a126f 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -206,7 +206,7 @@ Boot_CreateStmt:
PG_CATALOG_NAMESPACE,
shared_relation ? GLOBALTABLESPACE_OID : 0,
$3,
- InvalidOid,
+ InvalidRelFileNumber,
HEAP_TABLE_AM_OID,
tupdesc,
RELKIND_RELATION,
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f9bb721c5fe..f8d4824a3f8 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3740,7 +3740,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
if (set_tablespace)
{
/* Update its pg_class row */
- SetRelationTableSpace(iRel, params->tablespaceOid, InvalidOid);
+ SetRelationTableSpace(iRel, params->tablespaceOid, InvalidRelFileNumber);

/*
* Schedule unlinking of the old index storage at transaction commit.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index eaa81424270..8505cc8c1b5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15441,7 +15441,7 @@ ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
}

/* Update can be done, so change reltablespace */
- SetRelationTableSpace(rel, newTableSpace, InvalidOid);
+ SetRelationTableSpace(rel, newTableSpace, InvalidRelFileNumber);

InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);

diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 0fc2c093b0d..bffbb98779e 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -54,7 +54,7 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat

/* identifier of physical storage file */
/* relfilenode == 0 means it is a "mapped" relation, see relmapper.c */
- Oid relfilenode BKI_DEFAULT(0);
+ RelFileNumber relfilenode BKI_DEFAULT(0);

/* identifier of table space for relation (0 means default for database) */
Oid reltablespace BKI_DEFAULT(0) BKI_LOOKUP_OPT(pg_tablespace);

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/