Hi, This patch implements TODOs in the BRIN, GIN, GIST and SP-GIST index code. Operator class validation is modified to add strategy number bounds checking specific to each index type. 1) For BRIN and SP-GIST, the maximum strategy number from the common RTStrategy list is used as the upper limit. 2) For SP-GIST, the number of GIS object types and the strategy numbers per type are used to calculate the maximum strategy number. 3) As far as I can understand from the code, GIN may have custom strategies, so the existing limit of 63 is retained.
I have tested this patch using 'make check' and 'make worldcheck'. All the tests pass. This is my first contribution, so any input would be appreciated. Thanks & regards, Priyanka
From 5c2ffa3eb6f0ffc608f0b7f93687d23d88e03876 Mon Sep 17 00:00:00 2001 From: Priyanka Sangam <[email protected]> Date: Sun, 2 Aug 2026 20:09:03 +0530 Subject: [PATCH] TODOs for index opclass strategy number validation This patch addresses TODOs in the BRIN, GIN, GIST and SP-GIST index code. Opclass validation is modified to add strategy number bounds checking specific to each index type. 1) For BRIN and SP-GIST, the maximum strategy number from the list of strategies common to these index types is used as the limit. 2) For SP-GIST, the number of GIS object types and the strategy numbers per type are used to calculate the maximum strategy number. 3) For GIN, custom strategy numbers may be defined, so the highest possible limit of 63 is retained. --- src/backend/access/brin/brin_validate.c | 2 +- src/backend/access/gin/ginvalidate.c | 4 ++-- src/backend/access/gist/gistproc.c | 6 ------ src/backend/access/gist/gistvalidate.c | 5 +++-- src/backend/access/spgist/spgvalidate.c | 4 ++-- src/include/access/stratnum.h | 14 +++++++++++++- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c index 3f5be426b78..2bc21dd1aca 100644 --- a/src/backend/access/brin/brin_validate.c +++ b/src/backend/access/brin/brin_validate.c @@ -142,7 +142,7 @@ brinvalidate(Oid opclassoid) Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); /* Check that only allowed strategy numbers exist */ - if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63) + if (oprform->amopstrategy < 1 || oprform->amopstrategy > RTMaxStrategyNumber) { ereport(INFO, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), diff --git a/src/backend/access/gin/ginvalidate.c b/src/backend/access/gin/ginvalidate.c index ac82ce922d2..b501f5abd87 100644 --- a/src/backend/access/gin/ginvalidate.c +++ b/src/backend/access/gin/ginvalidate.c @@ -165,8 +165,8 @@ ginvalidate(Oid opclassoid) HeapTuple oprtup = &oprlist->members[i]->tuple; Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); - /* TODO: Check that only allowed strategy numbers exist */ - if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63) + /* Check that only allowed strategy numbers exist */ + if (oprform->amopstrategy < 1 || oprform->amopstrategy > MaxStrategyNumber) { ereport(INFO, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c index bb84030b23d..2ad2bf943b7 100644 --- a/src/backend/access/gist/gistproc.c +++ b/src/backend/access/gist/gistproc.c @@ -1330,12 +1330,6 @@ gist_point_consistent_internal(StrategyNumber strategy, return result; } -#define GeoStrategyNumberOffset 20 -#define PointStrategyNumberGroup 0 -#define BoxStrategyNumberGroup 1 -#define PolygonStrategyNumberGroup 2 -#define CircleStrategyNumberGroup 3 - Datum gist_point_consistent(PG_FUNCTION_ARGS) { diff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c index 56feb8d8400..e90032e81bc 100644 --- a/src/backend/access/gist/gistvalidate.c +++ b/src/backend/access/gist/gistvalidate.c @@ -174,8 +174,9 @@ gistvalidate(Oid opclassoid) Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); Oid op_rettype; - /* TODO: Check that only allowed strategy numbers exist */ - if (oprform->amopstrategy < 1) + /* Check that only allowed strategy numbers exist */ + if (oprform->amopstrategy < 1 || + oprform->amopstrategy >= (GeoStrategyNumberOffset * (GeoMaxStrategyNumberGroup + 1))) { ereport(INFO, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), diff --git a/src/backend/access/spgist/spgvalidate.c b/src/backend/access/spgist/spgvalidate.c index 27c855921e6..c2606fe9eb8 100644 --- a/src/backend/access/spgist/spgvalidate.c +++ b/src/backend/access/spgist/spgvalidate.c @@ -205,8 +205,8 @@ spgvalidate(Oid opclassoid) Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); Oid op_rettype; - /* TODO: Check that only allowed strategy numbers exist */ - if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63) + /* Check that only allowed strategy numbers exist */ + if (oprform->amopstrategy < 1 || oprform->amopstrategy > RTMaxStrategyNumber) { ereport(INFO, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), diff --git a/src/include/access/stratnum.h b/src/include/access/stratnum.h index 812ecd2ef8a..b409320d7d9 100644 --- a/src/include/access/stratnum.h +++ b/src/include/access/stratnum.h @@ -42,6 +42,18 @@ typedef uint16 StrategyNumber; #define HTMaxStrategyNumber 1 +/* + * Strategy number groups used by GIST, one for each GIS object type. Each number + * group can contain up to 'GeoStrategyNumberOffset' strategies. + */ +#define GeoStrategyNumberOffset 20 +#define PointStrategyNumberGroup 0 +#define BoxStrategyNumberGroup 1 +#define PolygonStrategyNumberGroup 2 +#define CircleStrategyNumberGroup 3 + +#define GeoMaxStrategyNumberGroup 3 + /* * Strategy numbers common to (some) GiST, SP-GiST and BRIN opclasses. * @@ -80,6 +92,6 @@ typedef uint16 StrategyNumber; #define RTOldAboveStrategyNumber 30 /* for old spelling of |>> */ #define RTMaxStrategyNumber 30 - +#define MaxStrategyNumber 63 #endif /* STRATNUM_H */ -- 2.43.0
