[PATCH] btree_gist: add cross-type integer operator support for GiST
Hello hackers,
I'd like to submit a patch that adds cross-type operator support for the
three integer types (int2, int4, int8) to the btree_gist GiST operator
families. The patch also lays a general foundation for cross-type
dispatch that other type families can adopt.
Current problem:
GiST indexes are currently limited to same-type operators: the planner
can only match a query condition against an index column if the
operator's left and right argument types exactly match the types
registered in the index's operator family (pg_amop). This is true for
every data type that backs a GiST opclass.
When a query provides a value whose type is compatible but different
from the column type, the planner cannot use the index for that column.
The operator family lookup fails because no cross-type entry exists.
The planner must then either fall back to a sequential scan,
or in multi-column indexes use only the remaining column(s) and filter
the rest as a post-filter.
As a specific example, consider an int8 (bigint) column indexed with
btree_gist. The query:
SELECT * FROM t WHERE camera_id = 1189;
Here 1189 is evaluated as int4 (integer). The planner resolves the
operator to =(int8,int4), which exists in pg_operator but is not
registered in the gist_int8_ops family. Only =(int8,int8) is
registered, so the column cannot be used as an index condition.
The workaround is to write an explicit cast in every query:
WHERE camera_id = 1189::int8
This is fragile as ORMs, application parameter binding, and even hand-
written SQL queries produce values whose types do not exactly match
the column type.
A couple of self-contained reproduction scripts are included at the
end of this email. They create a table with a GiST index, insert
enough rows to make the plan difference visible, and run
EXPLAIN ANALYZE.
Proposed solution:
I'm working on a patch that extends the three btree_gist integer
operator families (gist_int2_ops, gist_int4_ops, gist_int8_ops)
with cross-type comparison and KNN-distance operators covering the
other two integer types. Concretely:
gist_int2_ops <- operators for (int2, int4) and (int2, int8)
gist_int4_ops <- operators for (int4, int2) and (int4, int8)
gist_int8_ops <- operators for (int8, int2) and (int8, int4)
For each pair I add all the standard btree_gist strategies for the
comparison operators (<, <=, =, >=, >, <>), plus the strategy for
the KNN distance operator (<->) used by ORDER BY.
I propose not to register separate cross-type support functions in
the operator family. GiST's amvalidate requires every support
function to have a matching left and right type, and registering
18 additional support functions (three families x two subtypes x
three strategies) would be verbose and error-prone. Instead, I
propose to dispatch cross-type queries directly inside the existing
consistent and distance functions and use the existing subtype
OID argument.
I'm thinking of introducing a general-purpose cross-type dispatch
table in btree_utils_num:
typedef struct gbt_subtype_info
{
Oid subtype; /* right-hand Oid, e.g. INT4OID */
gbt_cmp_fn lt, le, eq, ge, gt; /* comparison callbacks */
gbt_dist_fn dist; /* KNN distance callback */
} gbt_subtype_info;
Each integer opclass defines a static array of these entries:
static const gbt_subtype_info gbt_int2_subtype_ops[] = {
{INT4OID, ... 6 comparison fns ... , distance fn},
{INT8OID, ... 6 comparison fns ... , distance fn},
{InvalidOid}
};
I'll replace the existing gbt_num_consistent() function with
gbt_num_consistent_x(), which takes a Datum query value and a
subtype Oid. If the subtype is InvalidOid or matches the indexed
type, the same-type path is used (backward compatible). Otherwise,
the function walks the dispatch table, finds the matching subtype
entry, and invokes the corresponding cross-type comparison or
distance callback.
Other btree_gist opclasses (float4/float8, date, timestamp, ...) and
even range-type GiST opfamilies in core can adopt the same pattern by
defining their own subtype dispatch tables and registering cross-type
operators via ALTER OPERATOR FAMILY.
I don't plan adding cross-type support for every data type as that
would result in a very bulky patch difficult to review, but I think
my proposal establishes the infrastructure so that follow-up work
for additional types is straightforward.
Is there interest in this patch? Should I proceed with polishing my
patch and sending it here for a review? I appreciate any feedback!
Thank you
Simple repro example:
DROP TABLE IF EXISTS camera_feeds_simple CASCADE;
CREATE TABLE camera_feeds_simple (
id SERIAL PRIMARY KEY,
camera_id int8 NOT NULL
);
CREATE EXTENSION IF NOT EXISTS btree_gist;
CREATE INDEX idx_camera_feeds_gist
ON camera_feeds_simple
USING GIST (camera_id);
SET enable_seqscan = OFF;
SET enable_bitmapscan = ON;
INSERT INTO camera_feeds_simple (camera_id) VALUES
(1), (1), (1), (1), (2), (2), (2), (2), (3), (3), (3),
(4), (4), (4), (4), (4), (4), (4), (4), (5), (5), (6), (6);
ANALYZE camera_feeds_simple;
\echo 'Triggering current behaviour that fails to use the index'
EXPLAIN (ANALYZE, COSTS, BUFFERS, TIMING, SUMMARY)
SELECT *
FROM camera_feeds_simple
WHERE camera_id = 4;
\echo 'Triggering query with manual cast workaround'
EXPLAIN (ANALYZE, COSTS, BUFFERS, TIMING, SUMMARY)
SELECT *
FROM camera_feeds_simple
WHERE camera_id = 4::int8;
RESET enable_seqscan;
RESET enable_bitmapscan;
Repro example with two columns:
DROP TABLE IF EXISTS camera_feeds_multi CASCADE;
CREATE TABLE camera_feeds_multi (
id SERIAL PRIMARY KEY,
camera_id int8 NOT NULL,
timerange TSTZRANGE NOT NULL
);
CREATE EXTENSION IF NOT EXISTS btree_gist;
CREATE INDEX idx_camera_feeds_gist
ON camera_feeds_multi
USING GIST (camera_id, timerange);
SET enable_seqscan = OFF;
SET enable_bitmapscan = ON;
INSERT INTO camera_feeds_multi (camera_id, timerange) VALUES
(1, '[2026-04-01 06:00:00+00, 2026-04-01 12:00:00+00)'),
(1, '[2026-04-01 12:00:00+00, 2026-04-01 18:00:00+00)'),
(1, '[2026-04-01 18:00:00+00, 2026-04-02 06:00:00+00)'),
(1, '[2026-04-02 06:00:00+00, 2026-04-02 12:00:00+00)'),
(2, '[2026-04-01 07:00:00+00, 2026-04-01 09:00:00+00)'),
(2, '[2026-04-01 08:30:00+00, 2026-04-01 11:00:00+00)'),
(2, '[2026-04-01 10:00:00+00, 2026-04-01 14:00:00+00)'),
(2, '[2026-04-01 13:00:00+00, 2026-04-01 17:00:00+00)'),
(3, '[2026-03-15 00:00:00+00, 2026-03-15 23:59:59+00)'),
(3, '[2026-03-20 08:00:00+00, 2026-03-20 20:00:00+00)'),
(3, '[2026-04-01 00:00:00+00, 2026-04-01 23:59:59+00)'),
(4, '[2026-04-01 00:00:00+00, 2026-04-01 06:00:00+00)'),
(4, '[2026-04-01 06:00:00+00, 2026-04-01 08:00:00+00)'),
(4, '[2026-04-01 08:00:00+00, 2026-04-01 12:00:00+00)'),
(4, '[2026-04-01 12:00:00+00, 2026-04-01 14:00:00+00)'),
(4, '[2026-04-01 14:00:00+00, 2026-04-01 18:00:00+00)'),
(4, '[2026-04-01 18:00:00+00, 2026-04-01 22:00:00+00)'),
(4, '[2026-04-01 22:00:00+00, 2026-04-02 00:00:00+00)'),
(4, '[2026-04-01 09:00:00+00, 2026-04-01 15:00:00+00)'),
(5, '[2026-03-30 00:00:00+00, 2026-04-02 00:00:00+00)'),
(5, '[2026-04-01 10:00:00+00, 2026-04-01 10:30:00+00)'),
(6, '[2026-04-01 12:00:00+00, 2026-04-01 12:00:00+00)'),
(6, '[2026-04-01 12:00:00+00, 2026-04-01 13:00:00+00)');
ANALYZE camera_feeds_multi;
\echo 'Triggering current behaviour that fails to use the index'
EXPLAIN (ANALYZE, COSTS, BUFFERS, TIMING, SUMMARY)
SELECT *
FROM camera_feeds_multi
WHERE camera_id = 4
AND timerange && '[2026-04-01 10:00:00+00, 2026-04-01 16:00:00+00)'::tstzrange
ORDER BY timerange;
\echo 'Triggering query with manual cast workaround'
EXPLAIN (ANALYZE, COSTS, BUFFERS, TIMING, SUMMARY)
SELECT *
FROM camera_feeds_multi
WHERE camera_id = 4::int8
AND timerange && '[2026-04-01 10:00:00+00, 2026-04-01 16:00:00+00)'::tstzrange
ORDER BY timerange;
RESET enable_seqscan;
RESET enable_bitmapscan;
Import Notes
Reply to msg id not found: aac10ffa-a0ca-4c49-846b-3655cbc6b37e@SparkReference msg id not found: aac10ffa-a0ca-4c49-846b-3655cbc6b37e@Spark
Hello hackers,
Following up on my previous message, I’m attaching a partial patch for the
btree_gist cross-type integer operator support.
This patch intentionally contains only the main code changes. It does not yet
include regression tests or documentation updates. I’m sending it this way to
make the core approach easier to review before adding the surrounding test and
documentation work.
I’d appreciate any feedback!
If the approach looks reasonable, I’ll send a more complete version with tests
and documentation.
Thank you
Attachments:
0001-Implement-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+597-37
Hi Alexander!
Thanks for working on this - this looks like useful feature and
btree_gist users will appreciate it.
When a query provides a value whose type is compatible but different
from the column type, the planner cannot use the index for that column.
[...] The workaround is to write an explicit cast in every query:
WHERE camera_id = 1189::int8
This is fragile as ORMs, application parameter binding, and even hand-
written SQL queries produce values whose types do not exactly match
the column type.
This bites particularly hard with composite indexes that mix a PostGIS
column with a plain identifier - a layer number, tenant id, etc.:
CREATE INDEX ON t USING gist (layer_id, geom);
... WHERE geom && :bbox AND layer_id = 42;
Today the integer side forces a cast to be index-usable, which seems ugly
and easy to forget, and the ORM argument above only makes it worse. So
I think this is worth pursuing.
A few things that seem important to me (though discussion may well prove
some of them minor):
1. GiST consistent() is CPU-bound (it runs for every key on every
visited page), so the bar here should be strictly zero speed regression
for existing same-type users. Note sk_subtype is the operator's right
operand type, so for ordinary same-type scans it is the native type,
not InvalidOid - meaning the same-type path now also goes through the
cross-type dispatch and scans the subtype table. Please add some fast
pathm and back it with a microbenchmark on a same-type workload.
2. On the "general foundation" framing:
Other btree_gist opclasses (float4/float8, date, timestamp, ...) and
even range-type GiST opfamilies in core can adopt the same pattern
I'd be cautious here. The scalar int64/fabs() dispatch shape does not
obviously fit range types (not scalar at all), and timestamp/date bring
their own questions (infinity, etc.). It may be better to solve types
as they actually come up and generalize the scaffolding as load on it
grows, rather than commit to a universal framework upfront.
3. The cross-type knowledge lives in two hand-maintained places - the
pg_amop entries in SQL and the C dispatch tables:
I propose [...] to dispatch cross-type queries directly inside the
existing consistent and distance functions and use the existing
subtype OID argument.
They are only reconciled at query time via an elog(), not at
amvalidate/CREATE time. (Version skew between the .so and the catalog
isn't the worry - the new extension version ships with the new major.)
The concern is plain authoring drift: adding an amop without the
matching C entry, or vice versa, passes validation and only fails on a
live query. It would be nicer if we could assert that every cross-type
amop in the family has a corresponding dispatch entry (in amvalidate?).
4. KNN: the distance callbacks compute fabs() in float8, so for int8
values beyond 2^53 the lower-bound distance loses precision. This
matches the existing same-type int8 KNN, so it's not a regression, but
since you're widening the mix it's worth calling out as a known limit.
The missing int_crosstype test files (referenced from REGRESS/meson but
not in the patch) break make check, but I take it that's just the
not-yet-included test round.
Did you register your patch on the commitfest? [0]https://commitfest.postgresql.org/59/
Thank you!
Best regards, Andrey Borodin.
Hey Andrey!
I'm happy to hear you like the proposal!
Let me reply in order:
1)
Oh, good catch! Since the other points require some changes in the patch, I'll
first do them and then I'll make sure to create a benchmark and post the
results.
2)
Agreed, that's a fair point. I was speculating with a generalized "foundation"
because I thought I'd get questions about how I'd support other types if I went
with a simpler patch that covered only ints.
Let me rework that and send a new patch!
3)
I was also worried about this. I wrote some checks against a hardcoded list in
the int_crosstype.sql (which I didn't submit, you noticed), but that is not
checking anything besides the SQL part.
I checked amvalidate(), but I couldn't find a way to check if there are
inconsistencies between what is defined in C and what is defined in SQL.
That function only checks amproc/amop signatures and I wasn't able to see how it
could compare against the C dispatch.
There is a comment in opclasscmds.c, around line 400, that already hints about
it not being able to run such validation.
we have no way to validate that the offered set of operators and
functions are consistent with the AM's expectations. It would be
nice to provide such a check someday
I could create a new core hook, but maybe that would be a larger change that
will expand the scope of this patch, and since neither CREATE nor ALTER ADD
calls it, the hook would still need a test to fire it.
As an alternative, maybe I could use the regression suite? I can put the
knowledge of supported dispatch entries in the code, expose it as a validator
and assert it in the tests. Running "make check" would assert that these entries
agree with pg_amop in both directions.
Concretely: the integer consistent/distance functions would dispatch through a
small static table of supported subtype OIDs. A set-returning C function would
expose that same table to SQL, and a regression test would EXCEPT it against the
cross-type rows in pg_amop for gist_int{2,4,8}_ops in both directions. Adding an
amop without a matching dispatch entry (or vice versa) would show up as a diff
under make check.
Would you prefer the regression suite approach? Or do you think it might be
a better idea to find a way to do it with amvalidate()?
4)
All right, I'll make sure to add it to the next patch.
To sum up, agreeing on 3) means I'd be ready for working on the next version of
the patch. I just need your advice on that bit.
I was not aware of the existence of commitfest, let me register :)
Thank you very much Andrey, best regards!
Hello Andrey!
Following up with the things I owed you: the benchmarks, the consistency check
and adding a note for the 2^53 case.
I added a fast path. Each integer opclass's consistent() / distance() now
detects the "same type" case and calls the original gbt_num_consistent() /
gbt_num_distance() directly.
To confirm there's no regression I ran a microbenchmark on an -O2 build, no
asserts, single client, over a 500k row int4 GiST index, with the following
options:
-c enable_seqscan=off \
-c enable_bitmapscan=off \
-c enable_sort=off \
-c max_parallel_workers_per_gather=0
This is the base for the bench:
CREATE EXTENSION IF NOT EXISTS btree_gist;
DROP TABLE IF EXISTS benchg;
CREATE TABLE benchg (a int4);
INSERT INTO benchg SELECT g FROM generate_series(0, 499999) g;
CREATE INDEX benchg_idx ON benchg USING gist (a);
VACUUM (ANALYZE, FREEZE) benchg;
And the two workloads:
consistent(), full-range index-only count(*):
SELECT count(*) FROM benchg WHERE a >= 0 AND a <= 499999;
distance(), full KNN ordering (ORDER BY a<->k over all rows):
SELECT count(*) FROM (SELECT a FROM benchg ORDER BY a <-> 250000 LIMIT 1000000) q;
The numbers in ms (12 repetitions, 15s each) before
(3e3d7875e95621b02311ea3443e5139e3bce944a) and after my patch:
before consistent min/med/mean = 51.754 52.718 54.137 ms
after consistent min/med/mean = 52.042 52.480 52.572 ms
------------------------------------------------------------------------
before distance min/med/mean = 76.863 77.177 77.395 ms
after distance min/med/mean = 77.357 77.803 77.980 ms
All numbers seem to be within measurement noise, except the consistent-before,
which is probably inflated by one slow rep.
Regarding the other point, I explored the regression suite path I mentioned.
The consistent() / distance() functions dispatch cross-type queries through a
single static table of supported subtype OIDs (gbt_int_crosstype_table in
btree_utils_num.c). I expose that exact table to SQL, in gbt_int_crosstype_subtypes(),
so there is no hand-maintained second copy of the list.
The int_crosstype.sql regression test then builds the set of cross-type
(lefttype, righttype, strategy) entries that should exist in pg_amop from that
function, and EXCEPTs it against the cross-type rows actually present in
gist_int{2,4,8}_ops:
- a pg_amop row whose subtype the C dispatch does not handle shows up as
"unexpected in pg_amop", and
- a dispatch entry without the matching pg_amop rows shows up as
"missing from pg_amop".
Either kind of drift produces a diff under `make check`. So adding an ALTER
OPERATOR FAMILY entry without a matching dispatch entry (or vice versa) fails
the suite (as I mentioned in my previous email, I'm not aware of a way to do
this with amvalidate() without patching core).
I'm attaching the new set of patches (this time I include the tests).
Best regards!
Hi Alexander,
Thank you for bringing this thread to my attention, this is a very
interesting proposal. Personally, I have some doubts about the way
cross-type operators are handled in GiST, but this is another issue.
Within the current framework, this proposal implements things
correctly, so I have no conceptual issue with it.
Concerning the implementation itself, I have a couple comments:
1. In the consistent functions, I believe `subtype` can never be
`InvalidOid` based on the current code. It probably doesn't hurt to
keep it in the condition, but personally I'd drop it. Either way, I
thought it was worth mentioning.
2. I'm not sure I like the approach of casting everything to an
`int64` and using a new `gbt_int_consistent_x` function. It works,
so if others have no problem with it I can accept it, but I have
another suggestion: re-use `gbt_num_consistent` even for cross-type
cases by creating cross-type `gbtree_ninfo` objects with cross-type
`f_gt`, `f_ge`, `f_eq`, etc. functions. I have attached a v3
patchset with patch 0003 containing my suggestions. Feel free to
take them or leave them as you see fit. This suggestion requires
one important change in `gbt_num_consistent`:
```
else
- retval = (tinfo->f_le(key->lower, query, flinfo) &&
+ retval = (tinfo->f_ge(query, key->lower, flinfo) &&
tinfo->f_le(query, key->upper, flinfo));
break;
```This is to make sure that `gbt_num_consistent` call all the
comparison functions with `query` on the left and `key->lower/upper`
on the right, to allow for cross-type comparison functions in `tinfo`.
0003 keeps your SQL/catalog work unchanged, compiles cleanly, and
passes the existing btree_gist regression including the cross-type
cases in 0002 (with the SRF-based catalog-drift check dropped, per
point 3).
3. This is related to my suggestion in 2. as it removes the
`gbt_int_crosstype_subtypes` SQL function (there is no
`gbt_int_crosstype_table` anymore in C). Instead, the `switch`
clause in the consistent functions raises an error in its default
path, so RHS types that are not handled can be caught quickly. As
you mention, I don't think there is a way to catch such issues at
index validation time, so this is the best I could think of.
Note that in theory we could put the `gbtree_ninfo` objects in a
table and then write a function equivalent to
`gbt_int_crosstype_subtypes` to still have the test you suggested,
but I think that switch statements are a bit clearer and perhaps
more efficient than looping through a table for the dispatch.
To me this is similar to adding new same-type operators to a GiST
opclass. Nothing checks at validation time that all the operators
in the opclass are effectively handled by the given consistent
function. So I don't see it as a problem to have the same behaviour
for the cross-type operators.
Other than that, this is a good proposal and definitely something
worth having in postgres.
Please let me know what you think of my suggestions.
Best regards,
Maxime
Attachments:
v3-0001-Implement-cross-type-operators-for-GiST-indexes.patchapplication/applefile; name=v3-0001-Implement-cross-type-operators-for-GiST-indexes.patchDownload
v3-0002-Add-tests-for-cross-type-operators-for-GiST-index.patchapplication/applefile; name=v3-0002-Add-tests-for-cross-type-operators-for-GiST-index.patchDownload
v3-0003-Reuse-gbt_num_consistent-for-cross-type-integer-c.patchapplication/octet-stream; name=v3-0003-Reuse-gbt_num_consistent-for-cross-type-integer-c.patch; x-unix-mode=0644Download+289-343
Hey Maxime!
Thank you for reviewing my changes so far and for proposing the changes in your
patch! And thank you for the kind words!
1)
I think we still need to check `InvalidOid` because execIndexing.c:~800
initializes scan keys ('ScanKeyEntryInitialize') with `InvalidOid` as the
subtype. Running the 'not_equal', 'partitions' and 'without_overlaps' tests
confirms that 'InvalidOid' must be handled. I can add the check again in the
switch. (attaching patch)
2) and 3)
I completely agree, your patch makes it much cleaner.
I applied your patch (with the 'InvalidOid' fix) on top of mine and ran all the
tests and benchmarks that I prepared previously. These are the results before
the entire patchset and after:
duration=15s/run, reps=10, 1 client, interleaved, lower is better
------------------------------------------------------------------------
before consistent min/med/mean = 51.246 51.361 51.378 ms
after consistent min/med/mean = 51.849 51.958 51.967 ms
------------------------------------------------------------------------
before distance min/med/mean = 76.441 76.561 76.567 ms
after distance min/med/mean = 76.568 76.678 76.681 ms
------------------------------------------------------------------------
I added one more commit that adds a small description of this fix in the docs.
The current patchset has been rebased on top of ef01ca6dbca54e9bf3abea01c357b346847ebcf3
Best regards!
Attachments:
v4-0001-Implement-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+501-37
v4-0002-Add-tests-for-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+804-1
v4-0003-Add-docs-for-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+24-1
v4-0004-Reuse-gbt_num_consistent-for-cross-type-integer-comp.patchapplication/octet-streamDownload+289-343
v4-0005-Safe-guard-InvalidOid.patchapplication/octet-streamDownload+12-6
Hi Alexander,
I think we still need to check `InvalidOid` because execIndexing.c:~800
initializes scan keys ('ScanKeyEntryInitialize') with `InvalidOid` as the
subtype. Running the 'not_equal', 'partitions' and 'without_overlaps' tests
confirms that 'InvalidOid' must be handled. I can add the check again in the
switch. (attaching patch)
You are completely right, I missed this while reading the source code.
I completely agree, your patch makes it much cleaner.
I applied your patch (with the 'InvalidOid' fix) on top of mine and ran all the
tests and benchmarks that I prepared previously. [...]
I'm glad you like the suggestion, and thank you for the pgindent
fixes that I missed.
Since this implementation now replaces the initial one in 0001, it
might be worth squashing 0001 and 0004 (and the 0005 fixes) into a
single 0001 patch, unless you want to keep the history of this work
in the patch series.
I added one more commit that adds a small description of this fix in the docs.
This patch also looks good to me, although I may not be the best
person to review doc changes.
One last tiny thing: 0001 adds int_crosstype to the regress tests
but the test file is only added in 0002, so 0001 on its own fails
`make check`. Might be worth moving the Makefile/meson.build
registration into 0002 so each patch passes its own tests.
I think that one additional thing needed before it's ready would be
to add commit messages to each patch.
Other than that it looks good, I don't have any additional code
comments.
Best,
Maxime
Hey Maxime!
I squashed your code-related changes on top of 0001, and the test-related changes into 0002.
I also moved my test-related changes (Makefile / meson.build) from 0001 to 0002.
I squashed the remaining fixes (the InvalidOid and the pgindent; 0005) into 0001 as well, so now we have:
- 0001 - code-related changes
- 0002 - test-related changes
- 0003 - docs changes
I also added one more test to 0002 ("Width-sensitivity guard") and added the commit messages.
Last but not least, I preserved your changes as co-author in 0001 and 0002.
I hope I didn't miss anything!
Best regards!
Attachments:
v5-0001-Implement-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+547-39
v5-0002-Add-tests-for-cross-type-operators-for-GiST-indexes.patchapplication/octet-streamDownload+809-2
v5-0003-doc-Document-cross-type-operator-support-in-btree_gi.patchapplication/octet-streamDownload+24-1
Hi Alexander,
Thank you for this, everything looks good to me now.
I'm not sure what the convention is exactly, but unless you're
waiting on another review we can probably set this patch to
"Ready for Committer" in the commitfest.
Thank you for the great work.
Best,
Maxime
Hey Maxime!
Fantastic!! From what I understood, this will land in PG20. Is there any date limit (merge window)
for that? If "no", let's give Andrey a chance to have another look at the patchset (he told me
he is currently overloaded). If "yes" (and we're close to the end of it), let's set it to "Ready" in
order to avoid slipping outside of the merge window.
By the way, should I set the status?
Thank you for the review and the feedback!
Best regards
On 26 Jun 2026, at 01:07, Alexander Nestorov <alexandernst@gmail.com> wrote:
Fantastic!! From what I understood, this will land in PG20. Is there any date limit (merge window)
for that? If "no", let's give Andrey a chance to have another look at the patchset (he told me
he is currently overloaded). If "yes" (and we're close to the end of it), let's set it to "Ready" in
order to avoid slipping outside of the merge window.
By the way, should I set the status?
The decision to commit something is up to the committers. They normally look
for useful new patches in the Commitfest, our app for tracking proposed contributions.
Any community reviewer - like Maxime or me - can move an entry to "Ready for Committer";
sometimes that attracts a committer's attention, sometimes it doesn't. You can change
the status yourself, but "Ready for Committer" is best set by a reviewer once they're
happy with the patch. Personally, I consider setting "RfC" on my side as a contract
for "I'll show up to fix stuff if we break something by committing this". But there are
no written rules AFAIK.
I'm a bit overloaded to do another review round right this moment (but I still skim
pgsql-hackers - that's my way of filling the gaps between managerial meetings). I plan
to get to this patchset soon, as soon as I'm back to regular hacking; I have two more
prioritized hacking tasks ahead of it, though.
Thanks!
Best regards, Andrey Borodin.
Hey all!
I noticed in the commitfest tracker that the patchset stopped passing all the
tests. It seems that several commits that were recently merged are conflicting
with this patchset. Specifically:
- 4b808ed77cd "Sync signatures of gbt_var_consistent() and gbt_num_consistent()"
gbt_num_consistent() now takes the strategy by value instead of by pointer, so
I had to change the call sites in my code.
- eef644e57c3 "Fix btree_gist's NotEqual strategy on internal index pages"
splits the <> handling into separate leaf and internal cases.
- fea9c1884b2 "Tighten up btree_gist's handling of truncated bounds" (and its
follow-up b82d69abf64)
restructured gbt_num_consistent() around the new lower_is_below_query() /
upper_is_above_query() helpers and calls callbacks as f(key, query), but my code
did it the other way around (f(query, key)).
I kept upstream's new leaf/internal split, the NotEqual fix and the two helper
macros verbatim, and only flipped each comparison to query-first. The result is
semantically identical to upstream for the same-type case, while preserving the
fixed argument order the cross-type callbacks depend on. On the caller side, the
int2/int4/int8 consistent() functions now pass the strategy by value to match
the new signature.
There are no functional changes to the patchset itself and all tests pass.
Attaching v6, rebased on top of a8c2547e.
PD: Adding Tom Lane (tgl) to CC as I noticed he added himself as a committer.
Best regards!