Re: Fix small typo, use InvalidRelFileNumber instead of InvalidOid - Mailing list pgsql-hackers

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/