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);