Skip to content

Commit 3b08511

Browse files
Priyanka Sangamhackorum
authored andcommitted
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.
1 parent 0ec3f04 commit 3b08511

6 files changed

Lines changed: 21 additions & 14 deletions

File tree

src/backend/access/brin/brin_validate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ brinvalidate(Oid opclassoid)
142142
Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
143143

144144
/* Check that only allowed strategy numbers exist */
145-
if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
145+
if (oprform->amopstrategy < 1 || oprform->amopstrategy > RTMaxStrategyNumber)
146146
{
147147
ereport(INFO,
148148
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),

src/backend/access/gin/ginvalidate.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ ginvalidate(Oid opclassoid)
165165
HeapTuple oprtup = &oprlist->members[i]->tuple;
166166
Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
167167

168-
/* TODO: Check that only allowed strategy numbers exist */
169-
if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
168+
/* Check that only allowed strategy numbers exist */
169+
if (oprform->amopstrategy < 1 || oprform->amopstrategy > MaxStrategyNumber)
170170
{
171171
ereport(INFO,
172172
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),

src/backend/access/gist/gistproc.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,12 +1330,6 @@ gist_point_consistent_internal(StrategyNumber strategy,
13301330
return result;
13311331
}
13321332

1333-
#define GeoStrategyNumberOffset 20
1334-
#define PointStrategyNumberGroup 0
1335-
#define BoxStrategyNumberGroup 1
1336-
#define PolygonStrategyNumberGroup 2
1337-
#define CircleStrategyNumberGroup 3
1338-
13391333
Datum
13401334
gist_point_consistent(PG_FUNCTION_ARGS)
13411335
{

src/backend/access/gist/gistvalidate.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,9 @@ gistvalidate(Oid opclassoid)
174174
Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
175175
Oid op_rettype;
176176

177-
/* TODO: Check that only allowed strategy numbers exist */
178-
if (oprform->amopstrategy < 1)
177+
/* Check that only allowed strategy numbers exist */
178+
if (oprform->amopstrategy < 1 ||
179+
oprform->amopstrategy >= (GeoStrategyNumberOffset * (GeoMaxStrategyNumberGroup + 1)))
179180
{
180181
ereport(INFO,
181182
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),

src/backend/access/spgist/spgvalidate.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ spgvalidate(Oid opclassoid)
205205
Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
206206
Oid op_rettype;
207207

208-
/* TODO: Check that only allowed strategy numbers exist */
209-
if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
208+
/* Check that only allowed strategy numbers exist */
209+
if (oprform->amopstrategy < 1 || oprform->amopstrategy > RTMaxStrategyNumber)
210210
{
211211
ereport(INFO,
212212
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),

src/include/access/stratnum.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ typedef uint16 StrategyNumber;
4242

4343
#define HTMaxStrategyNumber 1
4444

45+
/*
46+
* Strategy number groups used by GIST, one for each GIS object type. Each number
47+
* group can contain up to 'GeoStrategyNumberOffset' strategies.
48+
*/
49+
#define GeoStrategyNumberOffset 20
50+
#define PointStrategyNumberGroup 0
51+
#define BoxStrategyNumberGroup 1
52+
#define PolygonStrategyNumberGroup 2
53+
#define CircleStrategyNumberGroup 3
54+
55+
#define GeoMaxStrategyNumberGroup 3
56+
4557
/*
4658
* Strategy numbers common to (some) GiST, SP-GiST and BRIN opclasses.
4759
*
@@ -80,6 +92,6 @@ typedef uint16 StrategyNumber;
8092
#define RTOldAboveStrategyNumber 30 /* for old spelling of |>> */
8193

8294
#define RTMaxStrategyNumber 30
83-
95+
#define MaxStrategyNumber 63
8496

8597
#endif /* STRATNUM_H */

0 commit comments

Comments
 (0)