pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedReplace literal 0 values with the appropriate Invalid* constants
3+ messages / 2 participants
[nested] [flat]
* Replace literal 0 values with the appropriate Invalid* constants
@ 2026-02-12 10:15 Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Bertrand Drouvot @ 2026-02-12 10:15 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi hackers,
There are some places where we are using literal 0 instead of the proper Invalid*
constant.
I think it's better to use the proper constant instead: It improves code clarity
by making it explicit that these are invalid values rather than ambiguous zero
literals.
We already did the exercise for InvalidXLogRecPtr in ec317440716 and the same has
been proposed for InvalidReplOriginId in [1].
In this patch series I focused on the output of 'git grep "#define Invalid" "*.h"'
where invalid values are defined as 0.
That gives:
0001: Dealing with InvalidMultiXactId
That's not a lot of noise:
1 file changed, 1 insertion(+), 1 deletion(-)
0002: Dealing with InvalidOffsetNumber
That's not a lot of noise:
6 files changed, 7 insertions(+), 7 deletions(-)
0003: Dealing with InvalidAttrNumber
A bit more noise but I think it's still manageable to review:
19 files changed, 38 insertions(+), 37 deletions(-)
0004: Dealing with InvalidDsaPointer
That's not a lot of noise:
1 file changed, 1 insertion(+), 1 deletion(-)
0005: Dealing with InvalidRelFileNumber
That's not a lot of noise:
3 files changed, 5 insertions(+), 5 deletions(-)
Overall that's:
30 files changed, 52 insertions(+), 51 deletions(-)
If we think those changes are worth it, then I think that's not that much noise
(and that could be merged at an interval of choice if we feel that's too much noise).
FWIW, the same kind of Coccinelle script that lead to ec317440716 has been used
to generate those patches.
[1]: https://postgr.es/m/tencent_FED33DE297DEA5EC5E888D3B9F7E50D6EF07%40qq.com
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v1-0001-Replace-literal-0-with-InvalidMultiXactId-for-Mul.patch (1.2K, ../../aY2oKlSpikgO9m+X@ip-10-97-1-34.eu-west-3.compute.internal/2-v1-0001-Replace-literal-0-with-InvalidMultiXactId-for-Mul.patch)
download | inline diff:
From 554e5369df50f565168172e2539ddf7038db2225 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 12 Feb 2026 09:08:15 +0000
Subject: [PATCH v1 1/5] Replace literal 0 with InvalidMultiXactId for
MultiXactId assignments
Use the proper constant InvalidMultiXactId instead of literal 0 when
assigning MultiXactId variables and struct fields.
This improves code clarity by making it explicit that these are
invalid MultiXactId values rather than ambiguous zero literals.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion:
---
src/bin/pg_resetwal/pg_resetwal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
100.0% src/bin/pg_resetwal/
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index 85dc43d4cdb..697bed096bd 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -86,7 +86,7 @@ static Oid next_oid_val;
static bool mxids_given = false;
static MultiXactId next_mxid_val;
-static MultiXactId oldest_mxid_val = 0;
+static MultiXactId oldest_mxid_val = InvalidMultiXactId;
static bool next_mxoff_given = false;
static MultiXactOffset next_mxoff_val;
--
2.34.1
[text/x-diff] v1-0002-Replace-literal-0-with-InvalidOffsetNumber-for-Of.patch (3.8K, ../../aY2oKlSpikgO9m+X@ip-10-97-1-34.eu-west-3.compute.internal/3-v1-0002-Replace-literal-0-with-InvalidOffsetNumber-for-Of.patch)
download | inline diff:
From 3998beb9cb2064d993ec29a77132a5cdd97dab0c Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 12 Feb 2026 09:12:17 +0000
Subject: [PATCH v1 2/5] Replace literal 0 with InvalidOffsetNumber for
OffsetNumber assignments
Use the proper constant InvalidOffsetNumber instead of literal 0 when
assigning OffsetNumber variables and struct fields.
This improves code clarity by making it explicit that these are
invalid OffsetNumber values rather than ambiguous zero literals.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion:
---
src/backend/access/gin/ginfast.c | 4 ++--
src/backend/access/gin/ginget.c | 2 +-
src/backend/access/gin/gininsert.c | 2 +-
src/backend/access/gin/ginxlog.c | 2 +-
src/backend/access/nbtree/nbtsearch.c | 2 +-
src/backend/storage/page/itemptr.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
71.7% src/backend/access/gin/
19.8% src/backend/access/nbtree/
8.4% src/backend/storage/page/
diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 7a6b177977b..dde0f534613 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -108,7 +108,7 @@ writeListPage(Relation index, Buffer buffer,
}
else
{
- GinPageGetOpaque(page)->maxoff = 0;
+ GinPageGetOpaque(page)->maxoff = InvalidOffsetNumber;
}
MarkBufferDirty(buffer);
@@ -720,7 +720,7 @@ processPendingPage(BuildAccumulator *accum, KeyArray *ka,
maxoff = PageGetMaxOffsetNumber(page);
Assert(maxoff >= FirstOffsetNumber);
ItemPointerSetInvalid(&heapptr);
- attrnum = 0;
+ attrnum = InvalidOffsetNumber;
for (i = startoff; i <= maxoff; i = OffsetNumberNext(i))
{
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 6b148e69a8e..9f4be6d2be3 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -859,7 +859,7 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
* matchResult is lossy. So, on next call we will get next
* result from TIDBitmap.
*/
- entry->offset = 0;
+ entry->offset = InvalidOffsetNumber;
}
if (entry->isFinished)
break;
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 0d63fb4ba27..ae9a8736a8f 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -1562,7 +1562,7 @@ GinBufferReset(GinBuffer *buffer)
*/
buffer->key = (Datum) 0;
- buffer->attnum = 0;
+ buffer->attnum = InvalidOffsetNumber;
buffer->category = 0;
buffer->keylen = 0;
buffer->nitems = 0;
diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
index b1fee3c281f..5d24862405c 100644
--- a/src/backend/access/gin/ginxlog.c
+++ b/src/backend/access/gin/ginxlog.c
@@ -643,7 +643,7 @@ ginRedoInsertListPage(XLogReaderState *record)
}
else
{
- GinPageGetOpaque(page)->maxoff = 0;
+ GinPageGetOpaque(page)->maxoff = InvalidOffsetNumber;
}
payload = XLogRecGetBlockData(record, 0, &totaltupsize);
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index 32ae0bda892..3711efb97e0 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -2223,7 +2223,7 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
else
{
elog(ERROR, "invalid scan direction: %d", (int) dir);
- start = 0; /* keep compiler quiet */
+ start = InvalidOffsetNumber; /* keep compiler quiet */
}
/*
diff --git a/src/backend/storage/page/itemptr.c b/src/backend/storage/page/itemptr.c
index 546874ebc5f..4843ac84f01 100644
--- a/src/backend/storage/page/itemptr.c
+++ b/src/backend/storage/page/itemptr.c
@@ -90,7 +90,7 @@ ItemPointerInc(ItemPointer pointer)
{
if (blk != InvalidBlockNumber)
{
- off = 0;
+ off = InvalidOffsetNumber;
blk++;
}
}
--
2.34.1
[text/x-diff] v1-0003-Replace-literal-0-with-InvalidAttrNumber-for-Attr.patch (15.9K, ../../aY2oKlSpikgO9m+X@ip-10-97-1-34.eu-west-3.compute.internal/4-v1-0003-Replace-literal-0-with-InvalidAttrNumber-for-Attr.patch)
download | inline diff:
From 50d97e0d84f41e98e0ec2846d1d43d6aa1bf1272 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 12 Feb 2026 09:16:48 +0000
Subject: [PATCH v1 3/5] Replace literal 0 with InvalidAttrNumber for
AttrNumber assignments
Use the proper constant InvalidAttrNumber instead of literal 0 when
assigning AttrNumber variables and struct fields.
This improves code clarity by making it explicit that these are
invalid AttrNumber values rather than ambiguous zero literals.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion:
---
contrib/pg_visibility/pg_visibility.c | 2 +-
contrib/postgres_fdw/postgres_fdw.c | 6 +++---
src/backend/access/common/printtup.c | 2 +-
src/backend/access/common/tupdesc.c | 2 +-
src/backend/catalog/heap.c | 2 +-
src/backend/commands/copyfrom.c | 2 +-
src/backend/commands/indexcmds.c | 2 +-
src/backend/commands/sequence.c | 2 +-
src/backend/commands/tablecmds.c | 9 +++++----
src/backend/executor/execExpr.c | 4 ++--
src/backend/executor/execJunk.c | 2 +-
src/backend/executor/execMain.c | 2 +-
src/backend/executor/execTuples.c | 22 +++++++++++-----------
src/backend/executor/nodeFunctionscan.c | 2 +-
src/backend/executor/nodeIndexscan.c | 2 +-
src/backend/statistics/attribute_stats.c | 2 +-
src/backend/statistics/dependencies.c | 4 ++--
src/backend/utils/adt/pg_dependencies.c | 4 ++--
src/backend/utils/adt/selfuncs.c | 2 +-
19 files changed, 38 insertions(+), 37 deletions(-)
7.7% contrib/postgres_fdw/
3.9% src/backend/access/common/
23.1% src/backend/commands/
45.2% src/backend/executor/
7.0% src/backend/statistics/
8.1% src/backend/utils/adt/
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 9bc3a784bf7..ae0ebc0d05a 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -454,7 +454,7 @@ pg_visibility_tupdesc(bool include_blkno, bool include_pd)
{
TupleDesc tupdesc;
AttrNumber maxattr = 2;
- AttrNumber a = 0;
+ AttrNumber a = InvalidAttrNumber;
if (include_blkno)
++maxattr;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 3572689e33b..506836f3645 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -7536,7 +7536,7 @@ make_tuple_from_result_row(PGresult *res,
/*
* Set up and install callback to report where conversion error occurs.
*/
- errpos.cur_attno = 0;
+ errpos.cur_attno = InvalidAttrNumber;
errpos.rel = rel;
errpos.fsstate = fsstate;
errcallback.callback = conversion_error_callback;
@@ -7587,7 +7587,7 @@ make_tuple_from_result_row(PGresult *res,
ctid = (ItemPointer) DatumGetPointer(datum);
}
}
- errpos.cur_attno = 0;
+ errpos.cur_attno = InvalidAttrNumber;
j++;
}
@@ -7664,7 +7664,7 @@ conversion_error_callback(void *arg)
/* ForeignScan case */
ForeignScan *fsplan = castNode(ForeignScan, fsstate->ss.ps.plan);
int varno = 0;
- AttrNumber colno = 0;
+ AttrNumber colno = InvalidAttrNumber;
if (fsplan->scan.scanrelid > 0)
{
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 616bdafd395..f5672e74edd 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -224,7 +224,7 @@ SendRowDescriptionMessage(StringInfo buf, TupleDesc typeinfo,
{
/* No info available, so send zeroes */
resorigtbl = 0;
- resorigcol = 0;
+ resorigcol = InvalidAttrNumber;
}
if (formats)
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index b69d10f0a45..c318d91c0a3 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -1051,7 +1051,7 @@ BuildDescFromLists(const List *names, const List *types, const List *typmods, co
*/
desc = CreateTemplateTupleDesc(natts);
- attnum = 0;
+ attnum = InvalidAttrNumber;
forfour(l1, names, l2, types, l3, typmods, l4, collations)
{
char *attname = strVal(lfirst(l1));
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index a6ed9849e77..9238043857d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2597,7 +2597,7 @@ AddRelationNewConstraints(Relation rel,
cooked->contype = CONSTR_CHECK;
cooked->conoid = constrOid;
cooked->name = ccname;
- cooked->attnum = 0;
+ cooked->attnum = InvalidAttrNumber;
cooked->expr = expr;
cooked->is_enforced = cdef->is_enforced;
cooked->skip_validation = cdef->skip_validation;
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 25ee20b23db..5a7005a921c 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1740,7 +1740,7 @@ BeginCopyFrom(ParseState *pstate,
cstate->rteperminfos = pstate->p_rteperminfos;
}
- num_defaults = 0;
+ num_defaults = InvalidAttrNumber;
volatile_defexprs = false;
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 635679cc1f2..702a396b605 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2009,7 +2009,7 @@ ComputeIndexAttrs(ParseState *pstate,
}
else
{
- indexInfo->ii_IndexAttrNumbers[attn] = 0; /* marks expression */
+ indexInfo->ii_IndexAttrNumbers[attn] = InvalidAttrNumber; /* marks expression */
indexInfo->ii_Expressions = lappend(indexInfo->ii_Expressions,
expr);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index e1b808bbb60..925213ee8c9 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1615,7 +1615,7 @@ process_owned_by(Relation seqrel, List *owned_by, bool for_identity)
errmsg("invalid OWNED BY option"),
errhint("Specify OWNED BY table.column or OWNED BY NONE.")));
tablerel = NULL;
- attnum = 0;
+ attnum = InvalidAttrNumber;
}
else
{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..4d035ae2558 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -991,7 +991,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
*/
rawDefaults = NIL;
cookedDefaults = NIL;
- attnum = 0;
+ attnum = InvalidAttrNumber;
foreach(listptr, stmt->tableElts)
{
@@ -1391,7 +1391,7 @@ BuildDescForRelation(const List *columns)
natts = list_length(columns);
desc = CreateTemplateTupleDesc(natts);
- attnum = 0;
+ attnum = InvalidAttrNumber;
foreach(l, columns)
{
@@ -9168,7 +9168,7 @@ SetIndexStorageProperties(Relation rel, Relation attrelation,
{
Oid indexoid = lfirst_oid(lc);
Relation indrel;
- AttrNumber indattnum = 0;
+ AttrNumber indattnum = InvalidAttrNumber;
HeapTuple tuple;
indrel = index_open(indexoid, lockmode);
@@ -19958,7 +19958,8 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu
}
else
{
- partattrs[attn] = 0; /* marks the column as expression */
+ partattrs[attn] = InvalidAttrNumber; /* marks the column as
+ * expression */
*partexprs = lappend(*partexprs, expr);
/*
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 088eca24021..01c09b8899f 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -396,7 +396,7 @@ ExecBuildProjectionInfo(List *targetList,
{
TargetEntry *tle = lfirst_node(TargetEntry, lc);
Var *variable = NULL;
- AttrNumber attnum = 0;
+ AttrNumber attnum = InvalidAttrNumber;
bool isSafeVar = false;
/*
@@ -4141,7 +4141,7 @@ ExecBuildHash32FromAttrs(TupleDesc desc, const TupleTableSlotOps *ops,
ExprEvalStep scratch = {0};
NullableDatum *iresult = NULL;
intptr_t opcode;
- AttrNumber last_attnum = 0;
+ AttrNumber last_attnum = InvalidAttrNumber;
Assert(numCols >= 0);
diff --git a/src/backend/executor/execJunk.c b/src/backend/executor/execJunk.c
index 3736bea3b3c..79a7e60c342 100644
--- a/src/backend/executor/execJunk.c
+++ b/src/backend/executor/execJunk.c
@@ -94,7 +94,7 @@ ExecInitJunkFilter(List *targetList, TupleTableSlot *slot)
ListCell *t;
cleanMap = (AttrNumber *) palloc(cleanLength * sizeof(AttrNumber));
- cleanResno = 0;
+ cleanResno = InvalidAttrNumber;
foreach(t, targetList)
{
TargetEntry *tle = lfirst(t);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index bfd3ebc601e..4912989eac1 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1284,7 +1284,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_FdwRoutine = NULL;
/* The following fields are set later if needed */
- resultRelInfo->ri_RowIdAttNo = 0;
+ resultRelInfo->ri_RowIdAttNo = InvalidAttrNumber;
resultRelInfo->ri_extraUpdatedCols = NULL;
resultRelInfo->ri_projectNew = NULL;
resultRelInfo->ri_newTupleSlot = NULL;
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index b768eae9e53..528ac05f7d9 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -117,7 +117,7 @@ tts_virtual_clear(TupleTableSlot *slot)
slot->tts_flags &= ~TTS_FLAG_SHOULDFREE;
}
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
slot->tts_flags |= TTS_FLAG_EMPTY;
ItemPointerSetInvalid(&slot->tts_tid);
}
@@ -335,7 +335,7 @@ tts_heap_clear(TupleTableSlot *slot)
slot->tts_flags &= ~TTS_FLAG_SHOULDFREE;
}
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
slot->tts_flags |= TTS_FLAG_EMPTY;
ItemPointerSetInvalid(&slot->tts_tid);
hslot->off = 0;
@@ -413,7 +413,7 @@ tts_heap_materialize(TupleTableSlot *slot)
* Have to deform from scratch, otherwise tts_values[] entries could point
* into the non-materialized tuple (which might be gone when accessed).
*/
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
hslot->off = 0;
if (!hslot->tuple)
@@ -490,7 +490,7 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree)
tts_heap_clear(slot);
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
hslot->tuple = tuple;
hslot->off = 0;
slot->tts_flags &= ~(TTS_FLAG_EMPTY | TTS_FLAG_SHOULDFREE);
@@ -533,7 +533,7 @@ tts_minimal_clear(TupleTableSlot *slot)
slot->tts_flags &= ~TTS_FLAG_SHOULDFREE;
}
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
slot->tts_flags |= TTS_FLAG_EMPTY;
ItemPointerSetInvalid(&slot->tts_tid);
mslot->off = 0;
@@ -601,7 +601,7 @@ tts_minimal_materialize(TupleTableSlot *slot)
* Have to deform from scratch, otherwise tts_values[] entries could point
* into the non-materialized tuple (which might be gone when accessed).
*/
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
mslot->off = 0;
if (!mslot->mintuple)
@@ -689,7 +689,7 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree
Assert(TTS_EMPTY(slot));
slot->tts_flags &= ~TTS_FLAG_EMPTY;
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
mslot->off = 0;
mslot->mintuple = mtup;
@@ -739,7 +739,7 @@ tts_buffer_heap_clear(TupleTableSlot *slot)
if (BufferIsValid(bslot->buffer))
ReleaseBuffer(bslot->buffer);
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
slot->tts_flags |= TTS_FLAG_EMPTY;
ItemPointerSetInvalid(&slot->tts_tid);
bslot->base.tuple = NULL;
@@ -819,7 +819,7 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
* into the non-materialized tuple (which might be gone when accessed).
*/
bslot->base.off = 0;
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
if (!bslot->base.tuple)
{
@@ -956,7 +956,7 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple,
}
slot->tts_flags &= ~TTS_FLAG_EMPTY;
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
bslot->base.tuple = tuple;
bslot->base.off = 0;
slot->tts_tid = tuple->t_self;
@@ -1327,7 +1327,7 @@ MakeTupleTableSlot(TupleDesc tupleDesc,
slot->tts_flags |= TTS_FLAG_FIXED;
slot->tts_tupleDescriptor = tupleDesc;
slot->tts_mcxt = CurrentMemoryContext;
- slot->tts_nvalid = 0;
+ slot->tts_nvalid = InvalidAttrNumber;
if (tupleDesc != NULL)
{
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index 63e605e1f81..1caa8046ac6 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -457,7 +457,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
}
else
{
- AttrNumber attno = 0;
+ AttrNumber attno = InvalidAttrNumber;
if (node->funcordinality)
natts++;
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index a616abff04c..c38163b4519 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -1045,7 +1045,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
/* See cmp_orderbyvals() comments on NULLS LAST */
orderbysort->ssup_nulls_first = false;
/* ssup_attno is unused here and elsewhere */
- orderbysort->ssup_attno = 0;
+ orderbysort->ssup_attno = InvalidAttrNumber;
/* No abbreviation */
orderbysort->abbreviate = false;
PrepareSortSupportFromOrderingOp(orderbyop, orderbysort);
diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index a6b118a8d72..2aadf4fdfac 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -219,7 +219,7 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("must specify either \"%s\" or \"%s\"", "attname", "attnum")));
attname = NULL; /* keep compiler quiet */
- attnum = 0;
+ attnum = InvalidAttrNumber;
}
if (attnum < 0)
diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index e3a2f5817e0..b2718f64604 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -174,7 +174,7 @@ DependencyGenerator_init(int n, int k)
state = palloc0_object(DependencyGeneratorData);
state->dependencies = palloc_array(AttrNumber, k);
- state->ndependencies = 0;
+ state->ndependencies = InvalidAttrNumber;
state->current = 0;
state->k = k;
state->n = n;
@@ -1476,7 +1476,7 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
if (unique_exprs_cnt > 0)
attnum_offset = (unique_exprs_cnt + 1);
else
- attnum_offset = 0;
+ attnum_offset = InvalidAttrNumber;
/*
* Now that we know how many expressions there are, we can offset the
diff --git a/src/backend/utils/adt/pg_dependencies.c b/src/backend/utils/adt/pg_dependencies.c
index d8bd1c4a168..1e7e1c2fd8d 100644
--- a/src/backend/utils/adt/pg_dependencies.c
+++ b/src/backend/utils/adt/pg_dependencies.c
@@ -236,7 +236,7 @@ dependencies_object_end(void *state)
*/
list_free(parse->attnum_list);
parse->attnum_list = NIL;
- parse->dependency = 0;
+ parse->dependency = InvalidAttrNumber;
parse->degree = 0.0;
parse->found_attributes = false;
parse->found_dependency = false;
@@ -760,7 +760,7 @@ pg_dependencies_in(PG_FUNCTION_ARGS)
parse_state.state = DEPS_EXPECT_START;
parse_state.dependency_list = NIL;
parse_state.attnum_list = NIL;
- parse_state.dependency = 0;
+ parse_state.dependency = InvalidAttrNumber;
parse_state.degree = 0.0;
parse_state.found_attributes = false;
parse_state.found_dependency = false;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 29fec655593..bd9024cb99e 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4693,7 +4693,7 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel,
if (matched_info->exprs)
attnum_offset = (list_length(matched_info->exprs) + 1);
else
- attnum_offset = 0;
+ attnum_offset = InvalidAttrNumber;
/* see what actually matched */
foreach(lc2, *varinfos)
--
2.34.1
[text/x-diff] v1-0004-Replace-literal-0-with-InvalidDsaPointer-for-dsa_.patch (1.2K, ../../aY2oKlSpikgO9m+X@ip-10-97-1-34.eu-west-3.compute.internal/5-v1-0004-Replace-literal-0-with-InvalidDsaPointer-for-dsa_.patch)
download | inline diff:
From e42af89df713357054f7f8836c33b59279f6951a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 12 Feb 2026 09:23:58 +0000
Subject: [PATCH v1 4/5] Replace literal 0 with InvalidDsaPointer for
dsa_pointer assignments
Use the proper constant InvalidDsaPointer instead of literal 0 when
assigning dsa_pointer variables and struct fields.
This improves code clarity by making it explicit that these are
invalid dsa_pointer values rather than ambiguous zero literals.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion:
---
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
100.0% src/backend/executor/
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c68c26cbf38..2e16442c4ec 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -499,7 +499,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
if (node->ss.ps.instrument && pcxt->nworkers > 0)
sinstrument = (SharedBitmapHeapInstrumentation *) ptr;
- pstate->tbmiterator = 0;
+ pstate->tbmiterator = InvalidDsaPointer;
/* Initialize the mutex */
SpinLockInit(&pstate->mutex);
--
2.34.1
[text/x-diff] v1-0005-Replace-literal-0-with-InvalidRelFileNumber-for-R.patch (3.3K, ../../aY2oKlSpikgO9m+X@ip-10-97-1-34.eu-west-3.compute.internal/6-v1-0005-Replace-literal-0-with-InvalidRelFileNumber-for-R.patch)
download | inline diff:
From 743079c9ad4f7d01df38a5344ae87db42e6d75e3 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 12 Feb 2026 09:28:15 +0000
Subject: [PATCH v1 5/5] Replace literal 0 with InvalidRelFileNumber for
RelFileNumber assignments
Use the proper constant InvalidRelFileNumber instead of literal 0 when
assigning RelFileNumber variables and struct fields.
This improves code clarity by making it explicit that these are
invalid RelFileNumber values rather than ambiguous zero literals.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion:
---
src/backend/backup/basebackup_incremental.c | 2 +-
src/backend/postmaster/walsummarizer.c | 6 +++---
src/backend/storage/smgr/md.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
19.6% src/backend/backup/
60.6% src/backend/postmaster/
19.6% src/backend/storage/smgr/
diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c
index f58ed9b198a..eb3f0fdf347 100644
--- a/src/backend/backup/basebackup_incremental.c
+++ b/src/backend/backup/basebackup_incremental.c
@@ -738,7 +738,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/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index 2d8f57099fd..31e2ff01ec7 100644
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -1278,7 +1278,7 @@ SummarizeDbaseRecord(XLogReaderState *xlogreader, BlockRefTable *brtab)
(xl_dbase_create_file_copy_rec *) XLogRecGetData(xlogreader);
rlocator.spcOid = xlrec->tablespace_id;
rlocator.dbOid = xlrec->db_id;
- rlocator.relNumber = 0;
+ rlocator.relNumber = InvalidRelFileNumber;
BlockRefTableSetLimitBlock(brtab, &rlocator, MAIN_FORKNUM, 0);
}
else if (info == XLOG_DBASE_CREATE_WAL_LOG)
@@ -1289,7 +1289,7 @@ SummarizeDbaseRecord(XLogReaderState *xlogreader, BlockRefTable *brtab)
xlrec = (xl_dbase_create_wal_log_rec *) XLogRecGetData(xlogreader);
rlocator.spcOid = xlrec->tablespace_id;
rlocator.dbOid = xlrec->db_id;
- rlocator.relNumber = 0;
+ rlocator.relNumber = InvalidRelFileNumber;
BlockRefTableSetLimitBlock(brtab, &rlocator, MAIN_FORKNUM, 0);
}
else if (info == XLOG_DBASE_DROP)
@@ -1300,7 +1300,7 @@ SummarizeDbaseRecord(XLogReaderState *xlogreader, BlockRefTable *brtab)
xlrec = (xl_dbase_drop_rec *) XLogRecGetData(xlogreader);
rlocator.dbOid = xlrec->db_id;
- rlocator.relNumber = 0;
+ rlocator.relNumber = InvalidRelFileNumber;
for (i = 0; i < xlrec->ntablespaces; ++i)
{
rlocator.spcOid = xlrec->tablespace_ids[i];
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 443434e4ea8..52d69e15864 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -1597,7 +1597,7 @@ ForgetDatabaseSyncRequests(Oid dbid)
rlocator.dbOid = dbid;
rlocator.spcOid = 0;
- rlocator.relNumber = 0;
+ rlocator.relNumber = InvalidRelFileNumber;
INIT_MD_FILETAG(tag, rlocator, InvalidForkNumber, InvalidBlockNumber);
--
2.34.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Replace literal 0 values with the appropriate Invalid* constants
@ 2026-02-13 00:51 Andres Freund <andres@anarazel.de>
parent: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2026-02-13 00:51 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
Hi,
On 2026-02-12 10:15:06 +0000, Bertrand Drouvot wrote:
> There are some places where we are using literal 0 instead of the proper Invalid*
> constant.
>
> I think it's better to use the proper constant instead: It improves code clarity
> by making it explicit that these are invalid values rather than ambiguous zero
> literals.
I think we should reject these and other similar patches, they generate way
more noise than they are improving the code. If you edit the code for other
reasons and update the initialization in the course of that, ok, but these
wholesale searches for initializing with 0 use up bandwidth that we could use
for much more worthwhile things.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Replace literal 0 values with the appropriate Invalid* constants
@ 2026-02-13 06:42 Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 3+ messages in thread
From: Bertrand Drouvot @ 2026-02-13 06:42 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
Hi,
On Thu, Feb 12, 2026 at 07:51:18PM -0500, Andres Freund wrote:
> Hi,
>
> On 2026-02-12 10:15:06 +0000, Bertrand Drouvot wrote:
> > There are some places where we are using literal 0 instead of the proper Invalid*
> > constant.
> >
> > I think it's better to use the proper constant instead: It improves code clarity
> > by making it explicit that these are invalid values rather than ambiguous zero
> > literals.
>
> I think we should reject these and other similar patches, they generate way
> more noise than they are improving the code. If you edit the code for other
> reasons and update the initialization in the course of that, ok, but these
> wholesale searches for initializing with 0 use up bandwidth that we could use
> for much more worthwhile things.
Thanks for sharing your thoughts! I'll keep using Coccinelle but only for things
that provide more "added" value to the code.
Though I thought that for things like the ones in this thread (that I agree don't
provide a lot of added value), proceding like:
> > (and that could be merged at an interval of choice if we feel that's too much noise).
could have been a valuable option.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-02-13 06:42 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-12 10:15 Replace literal 0 values with the appropriate Invalid* constants Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2026-02-13 00:51 ` Andres Freund <andres@anarazel.de>
2026-02-13 06:42 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox