public inbox for [email protected]
help / color / mirror / Atom feedFrom: Maxime Schoemans <[email protected]>
To: Alexander Nestorov <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: pgsql-hackers mailing list <[email protected]>
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for GiST
Date: Mon, 22 Jun 2026 14:20:37 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <80ef3b41-1a71-47a4-a320-29e118d7092c@Spark>
References: <aac10ffa-a0ca-4c49-846b-3655cbc6b37e@Spark>
<36b4f67d-5975-452c-a6b8-b6407f0924ee@Spark>
<[email protected]>
<18e88767-6c31-402a-887e-37c38b366a6a@Spark>
<80ef3b41-1a71-47a4-a320-29e118d7092c@Spark>
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:
[application/applefile] v3-0001-Implement-cross-type-operators-for-GiST-indexes.patch (121B, 2-v3-0001-Implement-cross-type-operators-for-GiST-indexes.patch)
download | inline diff:
2
< = v3-0001-Implement-cross-type-operators-for-GiST-indexes.patch
[application/applefile] v3-0002-Add-tests-for-cross-type-operators-for-GiST-index.patch (123B, 3-v3-0002-Add-tests-for-cross-type-operators-for-GiST-index.patch)
download | inline diff:
2
< ? v3-0002-Add-tests-for-cross-type-operators-for-GiST-index.patch
[application/octet-stream] v3-0003-Reuse-gbt_num_consistent-for-cross-type-integer-c.patch (29.8K, 4-v3-0003-Reuse-gbt_num_consistent-for-cross-type-integer-c.patch)
download | inline diff:
From 347ed8923363a2ea0a7d4e348603825c6783cbf7 Mon Sep 17 00:00:00 2001
From: Maxime Schoemans <[email protected]>
Date: Mon, 22 Jun 2026 13:31:41 +0200
Subject: [PATCH v3 3/3] Reuse gbt_num_consistent for cross-type integer
comparisons
---
contrib/btree_gist/btree_gist--1.9--1.10.sql | 13 +-
contrib/btree_gist/btree_int2.c | 98 +++++++---
contrib/btree_gist/btree_int4.c | 98 +++++++---
contrib/btree_gist/btree_int8.c | 98 +++++++---
contrib/btree_gist/btree_utils_num.c | 169 +-----------------
contrib/btree_gist/btree_utils_num.h | 55 ++++--
contrib/btree_gist/expected/int_crosstype.out | 63 +------
contrib/btree_gist/sql/int_crosstype.sql | 60 +------
8 files changed, 289 insertions(+), 365 deletions(-)
diff --git a/contrib/btree_gist/btree_gist--1.9--1.10.sql b/contrib/btree_gist/btree_gist--1.9--1.10.sql
index c9ff1955204..9cd57455086 100644
--- a/contrib/btree_gist/btree_gist--1.9--1.10.sql
+++ b/contrib/btree_gist/btree_gist--1.9--1.10.sql
@@ -10,8 +10,8 @@
-- left/right input types, so the catalog additions below are deliberately
-- pg_amop-only. The existing consistent/distance support functions dispatch
-- on the subtype OID: same-type queries take the normal path, while mixed-width
--- integer queries are promoted to int64 and compared by the integer cross-type
--- helpers in btree_utils_num.c.
+-- integer queries select a cross-type comparison callback that reads the query
+-- and key sides at their own widths (see btree_int{2,4,8}.c).
CREATE FUNCTION int2_int4_dist(int2, int4)
RETURNS int4
@@ -43,15 +43,6 @@ RETURNS int8
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
--- Introspection helper exposing the integer query subtypes understood by the C
--- cross-type dispatch (gbt_int_crosstype_table in btree_utils_num.c). The
--- regression tests use it to assert that the cross-type pg_amop entries below
--- never drift from what the dispatch can handle.
-CREATE FUNCTION gbt_int_crosstype_subtypes()
-RETURNS SETOF oid
-AS 'MODULE_PATHNAME'
-LANGUAGE C IMMUTABLE PARALLEL SAFE;
-
CREATE OPERATOR <-> (
LEFTARG = int2,
RIGHTARG = int4,
diff --git a/contrib/btree_gist/btree_int2.c b/contrib/btree_gist/btree_int2.c
index d00b24d9a57..ea9c206e552 100644
--- a/contrib/btree_gist/btree_int2.c
+++ b/contrib/btree_gist/btree_int2.c
@@ -91,6 +91,72 @@ static const gbtree_ninfo tinfo =
gbt_int2_dist
};
+/*
+ * Cross-type GiST callbacks: the indexed key is int2, the query is int4 or
+ * int8. Both reuse gbt_num_consistent()/gbt_num_distance() via a tinfo whose
+ * comparison/distance callbacks read the query (left) and key (right) sides at
+ * their own widths. f_cmp is unused on these paths and left NULL.
+ */
+GBT_INT_CMP_FNS(gbt_int2_q4_, int32, int16)
+GBT_INT_CMP_FNS(gbt_int2_q8_, int64, int16)
+
+static const gbtree_ninfo tinfo_q4 =
+{
+ gbt_t_int2,
+ sizeof(int16),
+ 4,
+ gbt_int2_q4_gt,
+ gbt_int2_q4_ge,
+ gbt_int2_q4_eq,
+ gbt_int2_q4_le,
+ gbt_int2_q4_lt,
+ NULL,
+ gbt_int2_q4_dist
+};
+
+static const gbtree_ninfo tinfo_q8 =
+{
+ gbt_t_int2,
+ sizeof(int16),
+ 4,
+ gbt_int2_q8_gt,
+ gbt_int2_q8_ge,
+ gbt_int2_q8_eq,
+ gbt_int2_q8_le,
+ gbt_int2_q8_lt,
+ NULL,
+ gbt_int2_q8_dist
+};
+
+/*
+ * Cross-type dispatch shared by gbt_int2_consistent and gbt_int2_distance:
+ * select the tinfo for the query subtype and read the query value at its own
+ * width into caller-owned storage.
+ */
+static const gbtree_ninfo *
+gbt_int2_crosstype(Oid subtype, Datum d, gbt_intkey *q, const void **qp)
+{
+ switch (subtype)
+ {
+ case INT2OID:
+ q->i2 = DatumGetInt16(d);
+ *qp = &q->i2;
+ return &tinfo;
+ case INT4OID:
+ q->i4 = DatumGetInt32(d);
+ *qp = &q->i4;
+ return &tinfo_q4;
+ case INT8OID:
+ q->i8 = DatumGetInt64(d);
+ *qp = &q->i8;
+ return &tinfo_q8;
+ default:
+ elog(ERROR, "unrecognized subtype %u for btree_gist int2 cross-type comparison",
+ subtype);
+ return NULL; /* keep compiler quiet */
+ }
+}
+
PG_FUNCTION_INFO_V1(int2_dist);
Datum
@@ -176,7 +242,9 @@ gbt_int2_consistent(PG_FUNCTION_ARGS)
Oid subtype = PG_GETARG_OID(3);
bool *recheck = (bool *) PG_GETARG_POINTER(4);
int16KEY *kkk = (int16KEY *) DatumGetPointer(entry->key);
- int16 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
/* All cases served by this function are exact */
@@ -185,17 +253,10 @@ gbt_int2_consistent(PG_FUNCTION_ARGS)
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT2OID))
- {
- query = DatumGetInt16(queryDatum);
- PG_RETURN_BOOL(gbt_num_consistent(&key, &query, &strategy,
- GIST_LEAF(entry), &tinfo,
- fcinfo->flinfo));
- }
+ ti = gbt_int2_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_BOOL(gbt_int_consistent_x((int64) kkk->lower, (int64) kkk->upper,
- queryDatum, subtype, &strategy,
- GIST_LEAF(entry)));
+ PG_RETURN_BOOL(gbt_num_consistent(&key, qp, &strategy, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
@@ -205,21 +266,18 @@ gbt_int2_distance(PG_FUNCTION_ARGS)
Datum queryDatum = PG_GETARG_DATUM(1);
Oid subtype = PG_GETARG_OID(3);
int16KEY *kkk = (int16KEY *) DatumGetPointer(entry->key);
- int16 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT2OID))
- {
- query = DatumGetInt16(queryDatum);
- PG_RETURN_FLOAT8(gbt_num_distance(&key, &query, GIST_LEAF(entry),
- &tinfo, fcinfo->flinfo));
- }
+ ti = gbt_int2_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_FLOAT8(gbt_int_distance_x((int64) kkk->lower, (int64) kkk->upper,
- queryDatum, subtype));
+ PG_RETURN_FLOAT8(gbt_num_distance(&key, qp, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
diff --git a/contrib/btree_gist/btree_int4.c b/contrib/btree_gist/btree_int4.c
index 4df91f2057c..7e566890143 100644
--- a/contrib/btree_gist/btree_int4.c
+++ b/contrib/btree_gist/btree_int4.c
@@ -89,6 +89,72 @@ static const gbtree_ninfo tinfo =
gbt_int4_dist
};
+/*
+ * Cross-type GiST callbacks: the indexed key is int4, the query is int2 or
+ * int8. Both reuse gbt_num_consistent()/gbt_num_distance() via a tinfo whose
+ * comparison/distance callbacks read the query (left) and key (right) sides at
+ * their own widths. f_cmp is unused on these paths and left NULL.
+ */
+GBT_INT_CMP_FNS(gbt_int4_q2_, int16, int32)
+GBT_INT_CMP_FNS(gbt_int4_q8_, int64, int32)
+
+static const gbtree_ninfo tinfo_q2 =
+{
+ gbt_t_int4,
+ sizeof(int32),
+ 8,
+ gbt_int4_q2_gt,
+ gbt_int4_q2_ge,
+ gbt_int4_q2_eq,
+ gbt_int4_q2_le,
+ gbt_int4_q2_lt,
+ NULL,
+ gbt_int4_q2_dist
+};
+
+static const gbtree_ninfo tinfo_q8 =
+{
+ gbt_t_int4,
+ sizeof(int32),
+ 8,
+ gbt_int4_q8_gt,
+ gbt_int4_q8_ge,
+ gbt_int4_q8_eq,
+ gbt_int4_q8_le,
+ gbt_int4_q8_lt,
+ NULL,
+ gbt_int4_q8_dist
+};
+
+/*
+ * Cross-type dispatch shared by gbt_int4_consistent and gbt_int4_distance:
+ * select the tinfo for the query subtype and read the query value at its own
+ * width into caller-owned storage.
+ */
+static const gbtree_ninfo *
+gbt_int4_crosstype(Oid subtype, Datum d, gbt_intkey *q, const void **qp)
+{
+ switch (subtype)
+ {
+ case INT2OID:
+ q->i2 = DatumGetInt16(d);
+ *qp = &q->i2;
+ return &tinfo_q2;
+ case INT4OID:
+ q->i4 = DatumGetInt32(d);
+ *qp = &q->i4;
+ return &tinfo;
+ case INT8OID:
+ q->i8 = DatumGetInt64(d);
+ *qp = &q->i8;
+ return &tinfo_q8;
+ default:
+ elog(ERROR, "unrecognized subtype %u for btree_gist int4 cross-type comparison",
+ subtype);
+ return NULL; /* keep compiler quiet */
+ }
+}
+
PG_FUNCTION_INFO_V1(int4_dist);
Datum
@@ -174,7 +240,9 @@ gbt_int4_consistent(PG_FUNCTION_ARGS)
Oid subtype = PG_GETARG_OID(3);
bool *recheck = (bool *) PG_GETARG_POINTER(4);
int32KEY *kkk = (int32KEY *) DatumGetPointer(entry->key);
- int32 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
/* All cases served by this function are exact */
@@ -183,17 +251,10 @@ gbt_int4_consistent(PG_FUNCTION_ARGS)
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT4OID))
- {
- query = DatumGetInt32(queryDatum);
- PG_RETURN_BOOL(gbt_num_consistent(&key, &query, &strategy,
- GIST_LEAF(entry), &tinfo,
- fcinfo->flinfo));
- }
+ ti = gbt_int4_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_BOOL(gbt_int_consistent_x((int64) kkk->lower, (int64) kkk->upper,
- queryDatum, subtype, &strategy,
- GIST_LEAF(entry)));
+ PG_RETURN_BOOL(gbt_num_consistent(&key, qp, &strategy, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
@@ -203,21 +264,18 @@ gbt_int4_distance(PG_FUNCTION_ARGS)
Datum queryDatum = PG_GETARG_DATUM(1);
Oid subtype = PG_GETARG_OID(3);
int32KEY *kkk = (int32KEY *) DatumGetPointer(entry->key);
- int32 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT4OID))
- {
- query = DatumGetInt32(queryDatum);
- PG_RETURN_FLOAT8(gbt_num_distance(&key, &query, GIST_LEAF(entry),
- &tinfo, fcinfo->flinfo));
- }
+ ti = gbt_int4_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_FLOAT8(gbt_int_distance_x((int64) kkk->lower, (int64) kkk->upper,
- queryDatum, subtype));
+ PG_RETURN_FLOAT8(gbt_num_distance(&key, qp, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
diff --git a/contrib/btree_gist/btree_int8.c b/contrib/btree_gist/btree_int8.c
index ae01273b709..81c8bb364cf 100644
--- a/contrib/btree_gist/btree_int8.c
+++ b/contrib/btree_gist/btree_int8.c
@@ -91,6 +91,72 @@ static const gbtree_ninfo tinfo =
gbt_int8_dist
};
+/*
+ * Cross-type GiST callbacks: the indexed key is int8, the query is int2 or
+ * int4. Both reuse gbt_num_consistent()/gbt_num_distance() via a tinfo whose
+ * comparison/distance callbacks read the query (left) and key (right) sides at
+ * their own widths. f_cmp is unused on these paths and left NULL.
+ */
+GBT_INT_CMP_FNS(gbt_int8_q2_, int16, int64)
+GBT_INT_CMP_FNS(gbt_int8_q4_, int32, int64)
+
+static const gbtree_ninfo tinfo_q2 =
+{
+ gbt_t_int8,
+ sizeof(int64),
+ 16,
+ gbt_int8_q2_gt,
+ gbt_int8_q2_ge,
+ gbt_int8_q2_eq,
+ gbt_int8_q2_le,
+ gbt_int8_q2_lt,
+ NULL,
+ gbt_int8_q2_dist
+};
+
+static const gbtree_ninfo tinfo_q4 =
+{
+ gbt_t_int8,
+ sizeof(int64),
+ 16,
+ gbt_int8_q4_gt,
+ gbt_int8_q4_ge,
+ gbt_int8_q4_eq,
+ gbt_int8_q4_le,
+ gbt_int8_q4_lt,
+ NULL,
+ gbt_int8_q4_dist
+};
+
+/*
+ * Cross-type dispatch shared by gbt_int8_consistent and gbt_int8_distance:
+ * select the tinfo for the query subtype and read the query value at its own
+ * width into caller-owned storage.
+ */
+static const gbtree_ninfo *
+gbt_int8_crosstype(Oid subtype, Datum d, gbt_intkey *q, const void **qp)
+{
+ switch (subtype)
+ {
+ case INT2OID:
+ q->i2 = DatumGetInt16(d);
+ *qp = &q->i2;
+ return &tinfo_q2;
+ case INT4OID:
+ q->i4 = DatumGetInt32(d);
+ *qp = &q->i4;
+ return &tinfo_q4;
+ case INT8OID:
+ q->i8 = DatumGetInt64(d);
+ *qp = &q->i8;
+ return &tinfo;
+ default:
+ elog(ERROR, "unrecognized subtype %u for btree_gist int8 cross-type comparison",
+ subtype);
+ return NULL; /* keep compiler quiet */
+ }
+}
+
PG_FUNCTION_INFO_V1(int8_dist);
Datum
@@ -176,7 +242,9 @@ gbt_int8_consistent(PG_FUNCTION_ARGS)
Oid subtype = PG_GETARG_OID(3);
bool *recheck = (bool *) PG_GETARG_POINTER(4);
int64KEY *kkk = (int64KEY *) DatumGetPointer(entry->key);
- int64 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
/* All cases served by this function are exact */
@@ -185,17 +253,10 @@ gbt_int8_consistent(PG_FUNCTION_ARGS)
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT8OID))
- {
- query = DatumGetInt64(queryDatum);
- PG_RETURN_BOOL(gbt_num_consistent(&key, &query, &strategy,
- GIST_LEAF(entry), &tinfo,
- fcinfo->flinfo));
- }
+ ti = gbt_int8_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_BOOL(gbt_int_consistent_x(kkk->lower, kkk->upper,
- queryDatum, subtype, &strategy,
- GIST_LEAF(entry)));
+ PG_RETURN_BOOL(gbt_num_consistent(&key, qp, &strategy, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
@@ -205,21 +266,18 @@ gbt_int8_distance(PG_FUNCTION_ARGS)
Datum queryDatum = PG_GETARG_DATUM(1);
Oid subtype = PG_GETARG_OID(3);
int64KEY *kkk = (int64KEY *) DatumGetPointer(entry->key);
- int64 query;
+ const gbtree_ninfo *ti;
+ gbt_intkey query;
+ const void *qp;
GBT_NUMKEY_R key;
key.lower = (GBT_NUMKEY *) &kkk->lower;
key.upper = (GBT_NUMKEY *) &kkk->upper;
- if (likely(subtype == InvalidOid || subtype == INT8OID))
- {
- query = DatumGetInt64(queryDatum);
- PG_RETURN_FLOAT8(gbt_num_distance(&key, &query, GIST_LEAF(entry),
- &tinfo, fcinfo->flinfo));
- }
+ ti = gbt_int8_crosstype(subtype, queryDatum, &query, &qp);
- PG_RETURN_FLOAT8(gbt_int_distance_x(kkk->lower, kkk->upper,
- queryDatum, subtype));
+ PG_RETURN_FLOAT8(gbt_num_distance(&key, qp, GIST_LEAF(entry),
+ ti, fcinfo->flinfo));
}
Datum
diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c
index cff47e2d876..91f253d880c 100644
--- a/contrib/btree_gist/btree_utils_num.c
+++ b/contrib/btree_gist/btree_utils_num.c
@@ -5,8 +5,6 @@
#include "btree_gist.h"
#include "btree_utils_num.h"
-#include "catalog/pg_type.h"
-#include "funcapi.h"
#include "utils/cash.h"
#include "utils/date.h"
#include "utils/timestamp.h"
@@ -271,6 +269,12 @@ gbt_num_consistent(const GBT_NUMKEY_R *key,
{
bool retval;
+ /*
+ * Every comparison callback is invoked as f_xx(query, key): the query value
+ * is always the left argument and the indexed key bound the right. The
+ * integer opclasses rely on this fixed order so their cross-type callbacks
+ * can read each side at its own width.
+ */
switch (*strategy)
{
case BTLessEqualStrategyNumber:
@@ -286,7 +290,7 @@ gbt_num_consistent(const GBT_NUMKEY_R *key,
if (is_leaf)
retval = tinfo->f_eq(query, key->lower, flinfo);
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;
case BTGreaterStrategyNumber:
@@ -309,139 +313,6 @@ gbt_num_consistent(const GBT_NUMKEY_R *key,
return retval;
}
-/*
- * Cross-type dispatch table for the integer opclasses.
- *
- * This is the single source of truth for which query subtypes the integer
- * cross-type path understands. gbt_int_query_to_int64() promotes exactly the
- * subtypes listed here, and the SQL-visible gbt_int_crosstype_subtypes()
- * exposes the same list so the regression tests can assert that the cross-type
- * pg_amop entries in gist_int{2,4,8}_ops never drift from what the C code can
- * actually handle.
- */
-typedef int64 (*gbt_int_promote_fn) (Datum query);
-
-static int64
-gbt_int2_to_int64(Datum query)
-{
- return (int64) DatumGetInt16(query);
-}
-
-static int64
-gbt_int4_to_int64(Datum query)
-{
- return (int64) DatumGetInt32(query);
-}
-
-static int64
-gbt_int8_to_int64(Datum query)
-{
- return DatumGetInt64(query);
-}
-
-typedef struct
-{
- Oid subtype;
- gbt_int_promote_fn promote;
-} gbt_int_crosstype;
-
-static const gbt_int_crosstype gbt_int_crosstype_table[] = {
- {INT2OID, gbt_int2_to_int64},
- {INT4OID, gbt_int4_to_int64},
- {INT8OID, gbt_int8_to_int64},
-};
-
-/*
- * Promote a cross-type integer query value to int64.
- *
- * A subtype outside gbt_int_crosstype_table means the C dispatch is out of sync
- * with the operator-family registrations in pg_amop, so we treat it as an
- * internal error. The regression tests assert this never happens.
- */
-static int64
-gbt_int_query_to_int64(Datum query, Oid subtype)
-{
- int i;
-
- for (i = 0; i < lengthof(gbt_int_crosstype_table); i++)
- {
- if (gbt_int_crosstype_table[i].subtype == subtype)
- return gbt_int_crosstype_table[i].promote(query);
- }
-
- elog(ERROR, "unrecognized subtype %u for btree_gist integer cross-type comparison",
- subtype);
- return 0; /* keep compiler quiet */
-}
-
-/*
- * gbt_int_crosstype_subtypes
- *
- * Expose the contents of gbt_int_crosstype_table to SQL as a set of type OIDs.
- * The btree_gist regression tests use this to check that the cross-type pg_amop
- * entries in gist_int{2,4,8}_ops stay in agreement with the C dispatch.
- */
-PG_FUNCTION_INFO_V1(gbt_int_crosstype_subtypes);
-Datum
-gbt_int_crosstype_subtypes(PG_FUNCTION_ARGS)
-{
- FuncCallContext *funcctx;
-
- if (SRF_IS_FIRSTCALL())
- funcctx = SRF_FIRSTCALL_INIT();
-
- funcctx = SRF_PERCALL_SETUP();
-
- if (funcctx->call_cntr < lengthof(gbt_int_crosstype_table))
- {
- Oid subtype = gbt_int_crosstype_table[funcctx->call_cntr].subtype;
-
- SRF_RETURN_NEXT(funcctx, ObjectIdGetDatum(subtype));
- }
-
- SRF_RETURN_DONE(funcctx);
-}
-
-/*
- * Cross-type consistent method for the integer opclasses.
- *
- * The key range [lower, upper] and the query value are all compared as int64.
- * The strategy logic mirrors the same-type path in gbt_num_consistent(): the
- * query value keeps its own width (no narrowing to the indexed column type),
- * so out-of-range constants behave according to normal integer comparison.
- */
-bool
-gbt_int_consistent_x(int64 lower, int64 upper, Datum query, Oid subtype,
- const StrategyNumber *strategy, bool is_leaf)
-{
- int64 q = gbt_int_query_to_int64(query, subtype);
-
- switch (*strategy)
- {
- case BTLessEqualStrategyNumber:
- /* some k in [lower,upper] has k <= q iff lower <= q */
- return lower <= q;
- case BTLessStrategyNumber:
- /* leaf: key < q. internal: lower <= q (loose) */
- return is_leaf ? (lower < q) : (lower <= q);
- case BTEqualStrategyNumber:
- if (is_leaf)
- return lower == q;
- /* internal: lower <= q <= upper */
- return (lower <= q && q <= upper);
- case BTGreaterStrategyNumber:
- /* leaf: key > q. internal: upper >= q (loose) */
- return is_leaf ? (upper > q) : (upper >= q);
- case BTGreaterEqualStrategyNumber:
- return upper >= q;
- case BtreeGistNotEqualStrategyNumber:
- return !(lower == q && upper == q);
- default:
- return false;
- }
-}
-
-
/*
* The GiST distance method (for KNN-Gist)
*/
@@ -468,32 +339,6 @@ gbt_num_distance(const GBT_NUMKEY_R *key,
return retval;
}
-/*
- * Cross-type distance method for the integer opclasses.
- *
- * The distance from the query value to the key range is computed in int64 and
- * returned as a float8, matching the same-type integer distance behaviour.
- *
- * Note that the subtraction is performed in float8, so when the key bound and
- * the query value differ by more than 2^53 the returned distance loses
- * precision. This only affects the ordering of KNN results that are nearly
- * equidistant at that scale; it never changes which rows are returned.
- */
-float8
-gbt_int_distance_x(int64 lower, int64 upper, Datum query, Oid subtype)
-{
- int64 q = gbt_int_query_to_int64(query, subtype);
-
- /* query below the range: distance to lower bound */
- if (lower >= q)
- return fabs((float8) lower - (float8) q);
- /* query above the range: distance to upper bound */
- if (upper <= q)
- return fabs((float8) upper - (float8) q);
- /* query inside the range */
- return 0.0;
-}
-
GIST_SPLITVEC *
gbt_num_picksplit(const GistEntryVector *entryvec, GIST_SPLITVEC *v,
diff --git a/contrib/btree_gist/btree_utils_num.h b/contrib/btree_gist/btree_utils_num.h
index d378b0df37e..be09bdb2581 100644
--- a/contrib/btree_gist/btree_utils_num.h
+++ b/contrib/btree_gist/btree_utils_num.h
@@ -27,6 +27,18 @@ typedef struct
GBT_NUMKEY *t;
} Nsrt;
+/*
+ * Query-value storage for the integer opclasses' cross-type path. The caller
+ * owns one of these and passes its address to the per-type cross-type helper,
+ * which fills the right width and points the query pointer at it.
+ */
+typedef union
+{
+ int16 i2;
+ int32 i4;
+ int64 i8;
+} gbt_intkey;
+
/* type description */
@@ -86,7 +98,32 @@ typedef struct
(ivp)->day * (24.0 * SECS_PER_HOUR) + \
(ivp)->month * (30.0 * SECS_PER_DAY))
-#define GET_FLOAT_DISTANCE(t, arg1, arg2) fabs( ((float8) *((const t *) (arg1))) - ((float8) *((const t *) (arg2))) )
+#define GET_FLOAT_DISTANCE2(t1, t2, arg1, arg2) fabs( ((float8) *((const t1 *) (arg1))) - ((float8) *((const t2 *) (arg2))) )
+#define GET_FLOAT_DISTANCE(t, arg1, arg2) GET_FLOAT_DISTANCE2(t, t, (arg1), (arg2))
+
+/*
+ * Generate the comparison/distance callbacks for a gbtree_ninfo whose query
+ * and key sides may be different (integer) types. gbt_num_consistent() and
+ * gbt_num_distance() always invoke the callbacks as f_xx(query, key), so the
+ * first argument has the query type QT (the operator's right-hand subtype) and
+ * the second has the indexed key type KT. Integer widening is value-preserving,
+ * so the comparisons need no explicit cast; the distance widens to float8 to
+ * avoid overflow in the subtraction. Invoked with QT == KT this also generates
+ * the ordinary same-type callbacks.
+ */
+#define GBT_INT_CMP_FNS(prefix, QT, KT) \
+static bool prefix##gt(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return *((const QT *) a) > *((const KT *) b); } \
+static bool prefix##ge(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return *((const QT *) a) >= *((const KT *) b); } \
+static bool prefix##eq(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return *((const QT *) a) == *((const KT *) b); } \
+static bool prefix##le(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return *((const QT *) a) <= *((const KT *) b); } \
+static bool prefix##lt(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return *((const QT *) a) < *((const KT *) b); } \
+static float8 prefix##dist(const void *a, const void *b, FmgrInfo *flinfo) \
+{ return GET_FLOAT_DISTANCE2(QT, KT, a, b); }
extern Interval *abs_interval(Interval *a);
@@ -98,22 +135,6 @@ extern bool gbt_num_consistent(const GBT_NUMKEY_R *key, const void *query,
extern float8 gbt_num_distance(const GBT_NUMKEY_R *key, const void *query,
bool is_leaf, const gbtree_ninfo *tinfo, FmgrInfo *flinfo);
-/*
- * Cross-type consistent/distance helpers for the integer opclasses
- * (int2/int4/int8). All three integer widths promote losslessly to int64, so
- * a cross-type query reduces to a plain int64 comparison and there is no need
- * for the per-type callback machinery the same-type path uses. These helpers
- * are deliberately integer-specific rather than a general cross-type framework;
- * other type families should grow their own helpers shaped to their needs as
- * the need arises.
- */
-extern bool gbt_int_consistent_x(int64 lower, int64 upper,
- Datum query, Oid subtype,
- const StrategyNumber *strategy, bool is_leaf);
-
-extern float8 gbt_int_distance_x(int64 lower, int64 upper,
- Datum query, Oid subtype);
-
extern GIST_SPLITVEC *gbt_num_picksplit(const GistEntryVector *entryvec, GIST_SPLITVEC *v,
const gbtree_ninfo *tinfo, FmgrInfo *flinfo);
diff --git a/contrib/btree_gist/expected/int_crosstype.out b/contrib/btree_gist/expected/int_crosstype.out
index bd2b90bd3f6..af03cf84a40 100644
--- a/contrib/btree_gist/expected/int_crosstype.out
+++ b/contrib/btree_gist/expected/int_crosstype.out
@@ -6,66 +6,11 @@
-- index, and (c) values outside the smaller subtype's range are handled
-- according to normal comparison semantics, without narrowing or erroring.
--
--- Catalog invariant: the cross-type pg_amop entries in gist_int{2,4,8}_ops must
--- agree, in both directions, with what the C cross-type dispatch can handle.
--- The set of supported query subtypes is read from the C side via
--- gbt_int_crosstype_subtypes(), so there is no hand-maintained second copy of
--- the list here: registering a pg_amop row whose subtype the dispatch does not
--- handle (or dropping a dispatch entry while its pg_amop rows remain, or vice
--- versa) shows up as a diff below. Cross-type pg_amproc rows must also stay
--- absent, since the dispatch reuses the same-type support functions.
+-- The integer cross-type support is handled inside the existing
+-- consistent/distance support functions (which dispatch on the subtype OID),
+-- so the operator families must not contain any cross-type (different
+-- left/right input type) support-function entries.
--
-WITH dispatch_subtypes(typ) AS (
- SELECT s.subtype::regtype
- FROM gbt_int_crosstype_subtypes() AS s(subtype)
-),
-gist_int_opclasses(opfamily, indextype) AS (
- SELECT opc.opcname::text, opc.opcintype::regtype
- FROM pg_opclass opc
- JOIN pg_am am ON am.oid = opc.opcmethod
- WHERE am.amname = 'gist'
- AND opc.opcname IN ('gist_int2_ops', 'gist_int4_ops', 'gist_int8_ops')
-),
-expected_pairs(opfamily, lefttype, righttype) AS (
- SELECT oc.opfamily, oc.indextype, ds.typ
- FROM gist_int_opclasses oc
- CROSS JOIN dispatch_subtypes ds
- WHERE ds.typ <> oc.indextype
-),
-expected_amop(opfamily, lefttype, righttype, strategy, purpose) AS (
- SELECT opfamily, lefttype, righttype, strategy, purpose
- FROM expected_pairs
- CROSS JOIN (VALUES
- (1, 's'), (2, 's'), (3, 's'), (4, 's'),
- (5, 's'), (6, 's'), (15, 'o')
- ) AS strategy_purposes(strategy, purpose)
-),
-actual_amop AS (
- SELECT opf.opfname::text AS opfamily,
- amop.amoplefttype::regtype AS lefttype,
- amop.amoprighttype::regtype AS righttype,
- amop.amopstrategy::int AS strategy,
- amop.amoppurpose::text AS purpose
- FROM pg_amop amop
- JOIN pg_opfamily opf ON opf.oid = amop.amopfamily
- JOIN pg_am am ON am.oid = opf.opfmethod
- WHERE am.amname = 'gist'
- AND opf.opfname IN ('gist_int2_ops', 'gist_int4_ops', 'gist_int8_ops')
- AND amop.amoplefttype <> amop.amoprighttype
-)
-SELECT *
-FROM (
- SELECT 'missing from pg_amop' AS status, *
- FROM (SELECT * FROM expected_amop EXCEPT SELECT * FROM actual_amop) missing
- UNION ALL
- SELECT 'unexpected in pg_amop' AS status, *
- FROM (SELECT * FROM actual_amop EXCEPT SELECT * FROM expected_amop) unexpected
-) diff
-ORDER BY status, opfamily, lefttype::text, righttype::text, strategy, purpose;
- status | opfamily | lefttype | righttype | strategy | purpose
---------+----------+----------+-----------+----------+---------
-(0 rows)
-
SELECT opf.opfname AS opfamily,
amproc.amproclefttype::regtype AS lefttype,
amproc.amprocrighttype::regtype AS righttype,
diff --git a/contrib/btree_gist/sql/int_crosstype.sql b/contrib/btree_gist/sql/int_crosstype.sql
index d38084f1941..b337e26a1af 100644
--- a/contrib/btree_gist/sql/int_crosstype.sql
+++ b/contrib/btree_gist/sql/int_crosstype.sql
@@ -7,63 +7,11 @@
-- according to normal comparison semantics, without narrowing or erroring.
--
--- Catalog invariant: the cross-type pg_amop entries in gist_int{2,4,8}_ops must
--- agree, in both directions, with what the C cross-type dispatch can handle.
--- The set of supported query subtypes is read from the C side via
--- gbt_int_crosstype_subtypes(), so there is no hand-maintained second copy of
--- the list here: registering a pg_amop row whose subtype the dispatch does not
--- handle (or dropping a dispatch entry while its pg_amop rows remain, or vice
--- versa) shows up as a diff below. Cross-type pg_amproc rows must also stay
--- absent, since the dispatch reuses the same-type support functions.
+-- The integer cross-type support is handled inside the existing
+-- consistent/distance support functions (which dispatch on the subtype OID),
+-- so the operator families must not contain any cross-type (different
+-- left/right input type) support-function entries.
--
-WITH dispatch_subtypes(typ) AS (
- SELECT s.subtype::regtype
- FROM gbt_int_crosstype_subtypes() AS s(subtype)
-),
-gist_int_opclasses(opfamily, indextype) AS (
- SELECT opc.opcname::text, opc.opcintype::regtype
- FROM pg_opclass opc
- JOIN pg_am am ON am.oid = opc.opcmethod
- WHERE am.amname = 'gist'
- AND opc.opcname IN ('gist_int2_ops', 'gist_int4_ops', 'gist_int8_ops')
-),
-expected_pairs(opfamily, lefttype, righttype) AS (
- SELECT oc.opfamily, oc.indextype, ds.typ
- FROM gist_int_opclasses oc
- CROSS JOIN dispatch_subtypes ds
- WHERE ds.typ <> oc.indextype
-),
-expected_amop(opfamily, lefttype, righttype, strategy, purpose) AS (
- SELECT opfamily, lefttype, righttype, strategy, purpose
- FROM expected_pairs
- CROSS JOIN (VALUES
- (1, 's'), (2, 's'), (3, 's'), (4, 's'),
- (5, 's'), (6, 's'), (15, 'o')
- ) AS strategy_purposes(strategy, purpose)
-),
-actual_amop AS (
- SELECT opf.opfname::text AS opfamily,
- amop.amoplefttype::regtype AS lefttype,
- amop.amoprighttype::regtype AS righttype,
- amop.amopstrategy::int AS strategy,
- amop.amoppurpose::text AS purpose
- FROM pg_amop amop
- JOIN pg_opfamily opf ON opf.oid = amop.amopfamily
- JOIN pg_am am ON am.oid = opf.opfmethod
- WHERE am.amname = 'gist'
- AND opf.opfname IN ('gist_int2_ops', 'gist_int4_ops', 'gist_int8_ops')
- AND amop.amoplefttype <> amop.amoprighttype
-)
-SELECT *
-FROM (
- SELECT 'missing from pg_amop' AS status, *
- FROM (SELECT * FROM expected_amop EXCEPT SELECT * FROM actual_amop) missing
- UNION ALL
- SELECT 'unexpected in pg_amop' AS status, *
- FROM (SELECT * FROM actual_amop EXCEPT SELECT * FROM expected_amop) unexpected
-) diff
-ORDER BY status, opfamily, lefttype::text, righttype::text, strategy, purpose;
-
SELECT opf.opfname AS opfamily,
amproc.amproclefttype::regtype AS lefttype,
amproc.amprocrighttype::regtype AS righttype,
--
2.50.1 (Apple Git-155)
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for GiST
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox