Hi,
I noticed that some asserts and cycles use magic numbers 1 and 0
instead of BTLessStrategyNumber and InvalidStrategy.
At the same time, the BTMaxStrategyNumber macro is used there.
I suggest using appropriate macros for 1 and 0 values.
Please, see attached patch (targeted on the master branch).
--
Best regards,
Daniil Davydov
From 67d2125240021f3f0f25d56630dad1d70940b40d Mon Sep 17 00:00:00 2001
From: Daniil Davidov <d.davy...@postgrespro.ru>
Date: Mon, 30 Jun 2025 10:10:24 +0700
Subject: [PATCH v1] Get rid of magic numbers
---
src/backend/access/brin/brin_minmax.c | 4 ++--
src/backend/access/brin/brin_minmax_multi.c | 4 ++--
src/backend/access/nbtree/nbtpreprocesskeys.c | 4 ++--
src/backend/access/nbtree/nbtvalidate.c | 2 +-
src/backend/partitioning/partprune.c | 4 +++-
5 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index d21ab3a668c..19e953a4fd4 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -263,7 +263,7 @@ minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
{
MinmaxOpaque *opaque;
- Assert(strategynum >= 1 &&
+ Assert(strategynum >= BTLessStrategyNumber &&
strategynum <= BTMaxStrategyNumber);
opaque = (MinmaxOpaque *) bdesc->bd_info[attno - 1]->oi_opaque;
@@ -277,7 +277,7 @@ minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
{
uint16 i;
- for (i = 1; i <= BTMaxStrategyNumber; i++)
+ for (i = BTLessStrategyNumber; i <= BTMaxStrategyNumber; i++)
opaque->strategy_procinfos[i - 1].fn_oid = InvalidOid;
opaque->cached_subtype = subtype;
}
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 0d1507a2a36..f1dc40b10e7 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -2901,7 +2901,7 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
{
MinmaxMultiOpaque *opaque;
- Assert(strategynum >= 1 &&
+ Assert(strategynum >= BTLessStrategyNumber &&
strategynum <= BTMaxStrategyNumber);
opaque = (MinmaxMultiOpaque *) bdesc->bd_info[attno - 1]->oi_opaque;
@@ -2915,7 +2915,7 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
{
uint16 i;
- for (i = 1; i <= BTMaxStrategyNumber; i++)
+ for (i = BTLessStrategyNumber; i <= BTMaxStrategyNumber; i++)
opaque->strategy_procinfos[i - 1].fn_oid = InvalidOid;
opaque->cached_subtype = subtype;
}
diff --git a/src/backend/access/nbtree/nbtpreprocesskeys.c b/src/backend/access/nbtree/nbtpreprocesskeys.c
index a136e4bbfdf..b030833e370 100644
--- a/src/backend/access/nbtree/nbtpreprocesskeys.c
+++ b/src/backend/access/nbtree/nbtpreprocesskeys.c
@@ -360,7 +360,7 @@ _bt_preprocess_keys(IndexScanDesc scan)
Assert(OidIsValid(orderproc->fn_oid));
}
- for (j = BTMaxStrategyNumber; --j >= 0;)
+ for (j = BTMaxStrategyNumber; --j >= InvalidStrategy;)
{
ScanKey chk = xform[j].inkey;
@@ -438,7 +438,7 @@ _bt_preprocess_keys(IndexScanDesc scan)
* sufficient to form an unbroken series of "=" constraints on all
* attrs prior to the attr from the final scan->keyData[] key.
*/
- for (j = BTMaxStrategyNumber; --j >= 0;)
+ for (j = BTMaxStrategyNumber; --j >= InvalidStrategy;)
{
if (xform[j].inkey)
{
diff --git a/src/backend/access/nbtree/nbtvalidate.c b/src/backend/access/nbtree/nbtvalidate.c
index 817ad358f0c..03729892cde 100644
--- a/src/backend/access/nbtree/nbtvalidate.c
+++ b/src/backend/access/nbtree/nbtvalidate.c
@@ -140,7 +140,7 @@ btvalidate(Oid opclassoid)
Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
/* Check that only allowed strategy numbers exist */
- if (oprform->amopstrategy < 1 ||
+ if (oprform->amopstrategy < BTLessStrategyNumber ||
oprform->amopstrategy > BTMaxStrategyNumber)
{
ereport(INFO,
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 48a35f763e9..0d9ac307b17 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1526,7 +1526,9 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
* combinations of expressions of different keys, which
* get_steps_using_prefix takes care of for us.
*/
- for (strat = 1; strat <= BTMaxStrategyNumber; strat++)
+ for (strat = BTLessStrategyNumber;
+ strat <= BTMaxStrategyNumber;
+ strat++)
{
foreach(lc, btree_clauses[strat])
{
--
2.43.0
