[Patch] Implement TODOs for index operator strategy number validation

Started by Priyanka Sabout 1 month ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253305
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 16, 2026 at 08:33 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253305_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253305_1 && git checkout t253305_1

Patchset v1 (message #1) is on t253305_1

Jump to latest
#1Priyanka S
developerette@gmail.com

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

Attachments:

t253305_1
0001-Fix-TODOs-for-index-opclass-strategy-number-validati.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-TODOs-for-index-opclass-strategy-number-validati.patchDownload+21-15
#2Jinqing Kuang
kuangjinqingcn@gmail.com
In reply to: Priyanka S (#1)
Re: [Patch] Implement TODOs for index operator strategy number validation

On Aug 3, 2026, at 22:35, Priyanka S <developerette@gmail.com> wrote:

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
<0001-Fix-TODOs-for-index-opclass-strategy-number-validati.patch>

Hi Priyanka,

Thanks for the patch. I have a concern about the new strategy-number
bounds.

GiST, SP-GiST and BRIN all set amstrategies = 0, meaning that strategy
numbers are opclass-defined (see amapi.h and DefineOpClass()).

For example, a custom SP-GiST opclass can use strategy 31 if its support
functions understand it, but this patch would make amvalidate() reject
it. RTMaxStrategyNumber isn't a general limit for BRIN/SP-GiST, and
the Geo* constants are specific to the built-in GiST point implementation.

Regards,
Jinqing

#3Priyanka S
developerette@gmail.com
In reply to: Jinqing Kuang (#2)
Re: [Patch] Implement TODOs for index operator strategy number validation

Hi Jinqing,

Thanks for your explanation. I was not aware of the possibility of defining
custom strategies for GiST, SP-GiST and BRIN. I'll withdraw the patch for
rework.

Thanks & regards,
Priyanka

On Tue, Sep 8, 2026 at 1:47 PM Jinqing Kuang <kuangjinqingcn@gmail.com>
wrote:

Show quoted text

On Aug 3, 2026, at 22:35, Priyanka S <developerette@gmail.com> wrote:

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
<0001-Fix-TODOs-for-index-opclass-strategy-number-validati.patch>

Hi Priyanka,

Thanks for the patch. I have a concern about the new strategy-number
bounds.

GiST, SP-GiST and BRIN all set amstrategies = 0, meaning that strategy
numbers are opclass-defined (see amapi.h and DefineOpClass()).

For example, a custom SP-GiST opclass can use strategy 31 if its support
functions understand it, but this patch would make amvalidate() reject
it. RTMaxStrategyNumber isn't a general limit for BRIN/SP-GiST, and
the Geo* constants are specific to the built-in GiST point implementation.

Regards,
Jinqing