Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wfQRO-005RbL-2a for pgsql-bugs@arkaria.postgresql.org; Thu, 02 Jul 2026 23:03:31 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wfQQN-003ko2-1i for pgsql-bugs@arkaria.postgresql.org; Thu, 02 Jul 2026 23:02:27 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wfQQN-003knu-05 for pgsql-bugs@lists.postgresql.org; Thu, 02 Jul 2026 23:02:27 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1wfQQJ-00000001GJc-1eI3 for pgsql-bugs@postgresql.org; Thu, 02 Jul 2026 23:02:26 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.18.1/8.18.1) with ESMTP id 662N2Hss3793781; Thu, 2 Jul 2026 19:02:17 -0400 From: Tom Lane To: Ayush Tiwari cc: =?UTF-8?B?546L6LeD5p6X?= , pgsql-bugs , 3764353996 <3764353996@qq.com> Subject: Re: Fw:Re: Fw: gbt_var_consistent in contrib/btree_gist/btree_utils_var.c has internal-node type confusion on the <> strategy, bypassing exclusion constraints In-reply-to: References: <3748108.1783003096@sss.pgh.pa.us> <3756776.1783011257@sss.pgh.pa.us> Comments: In-reply-to Ayush Tiwari message dated "Thu, 02 Jul 2026 23:16:04 +0530" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <3793655.1783033301.0@sss.pgh.pa.us> Date: Thu, 02 Jul 2026 19:02:17 -0400 Message-ID: <3793780.1783033337@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <3793655.1783033301.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable Ayush Tiwari writes: > On Thu, 2 Jul 2026 at 22:24, Tom Lane wrote: >> Anyway, I think part of our work here has got to be to raise the >> level of documentation of this code. I've had it with having >> to reverse-engineer details as fundamental as these. > I've added some documentation in code, patch attached. I had something considerably more ambitious in mind, so I had a go at that --- and was glad that I did so, because the review turned up an additional bug as well as some other opportunities for improvement. v3-0001 is a documentation-improvement patch. I only touched the files associated with btree_utils_var.c. Maybe it's worth doing something similar for the btree_utils_num.c group, but I have the impression that those are way simpler. v3-0002 is your patch, editorialized a bit. I left out the test case because it seemed fairly expensive for something that won't expose a bug in normal builds. (BTW, I wonder if rather than documenting the comparisons as being reversed, we should just flip them all.) v3-0003 fixes a bug I identified: index builds that use the gbt_bit_ssup_cmp infrastructure will sort the entries in a way that doesn't match the bit types' actual ordering, leading to what's probably a very inefficient index. v3-0004 and v3-0005 clean up some things that seem like dead code to me. 0003 through 0005 could use more eyeballs on them. Also, it occurs to me after another look at 0002 that what was perhaps intended was something like this for the non-leaf case: if (is_leaf) retval =3D !(tinfo->f_eq(query, key->lower, collation, fli= nfo)); else retval =3D tinfo->trnc || !(tinfo->f_cmp(query, key->lower, collation, flinfo) =3D= =3D 0 && tinfo->f_cmp(query, key->upper, collation, flinfo) =3D= =3D 0); That is, if we're looking at non-truncated keys and lower =3D=3D upper, then we know that all the keys below this node are exactly that value, so we can avoid descending if that value is equal to the query. Most of the time this would not pay off in any savings, but if you had an index on a column with only a few values, maybe there would be leaf pages like that? I would think that the planner would avoid choosing to implement a <> query with an indexscan unless there was a pretty large fraction of the table that <> would reject, so maybe this actually is worth doing. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v3-0001-Reverse-engineer-some-documentation-for-btree_gis.p"; name*1="atch"; charset="us-ascii" Content-ID: <3793655.1783033301.2@sss.pgh.pa.us> Content-Description: v3-0001-Reverse-engineer-some-documentation-for-btree_gis.patch Content-Transfer-Encoding: quoted-printable =46rom 16f010e5853ee0cc9baf4e44753eda04c6fabe32 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Jul 2026 16:51:35 -0400 Subject: [PATCH v3 1/5] Reverse-engineer some documentation for btree_gist= 's varlena modules. There were a number of rather subtle points about the behavior of this code, which its original authors did not deign to document. Try to improve that. In particular, explain how internal and leaf keys can differ and what the restrictions are on that. This work arose from trying to fix some bugs, and in the process I believe I've identified some more, but this patch does not attempt to fix anything, only document it. I did make a few purely cosmetic code changes, such as removing dead (and confusing!) initializations of variables and choosing more appropriate types for some pointers. Author: Tom Lane Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.H= mail.3020001251@tju.edu.cn --- contrib/btree_gist/btree_bit.c | 51 ++++++++++++++++++++------ contrib/btree_gist/btree_bytea.c | 4 +- contrib/btree_gist/btree_numeric.c | 9 ++++- contrib/btree_gist/btree_text.c | 12 +++++- contrib/btree_gist/btree_utils_var.c | 55 ++++++++++++++++++++-------- contrib/btree_gist/btree_utils_var.h | 38 ++++++++++++++++++- 6 files changed, 136 insertions(+), 33 deletions(-) diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit= .c index 2b9c18a586f..dcfe6ed17e4 100644 --- a/contrib/btree_gist/btree_bit.c +++ b/contrib/btree_gist/btree_bit.c @@ -1,5 +1,11 @@ /* * contrib/btree_gist/btree_bit.c + * + * Support for bit and varbit types (which act the same for our purposes)= . + * + * Leaf-page keys are bit/varbit values, but internal-page keys are just + * bytea values containing the first N bytes of the represented bitstring= . + * (In particular, they lack the bit_len field of a bit/varbit Datum.) */ #include "postgres.h" = @@ -63,6 +69,10 @@ gbt_bitlt(const void *a, const void *b, Oid collation, = FmgrInfo *flinfo) PointerGetDatum(b))); } = +/* + * Notice we use byteacmp here, not bitcmp as you might expect. + * That's because internal-page keys are bytea. + */ static int32 gbt_bitcmp(const void *a, const void *b, Oid collation, FmgrInfo *flinfo) { @@ -72,10 +82,25 @@ gbt_bitcmp(const void *a, const void *b, Oid collation= , FmgrInfo *flinfo) } = = +/* + * Convert a leaf-page bit/varbit value to internal-page form. + * + * The important change here is to remove the bit_len field so that + * what we have left looks like a bytea. + * + * The business with padding to INTALIGN length appears entirely historic= al, + * but we can't remove that without breaking on-disk compatibility. + * For example, if the lower bound of some leaf page is the 20-bit string + * of all ones, with data contents FFFFF0, this code pads it to bytea FFF= FF000 + * in the internal key representing that page. If, when we search the in= dex + * for that value, we did not again pad to FFFFF000, then byteacmp would + * say that the query is strictly less than the lower bound so we would n= ot + * descend to that leaf page. + */ static bytea * -gbt_bit_xfrm(bytea *leaf) +gbt_bit_xfrm(VarBit *leaf) { - bytea *out =3D leaf; + bytea *out; int sz =3D VARBITBYTES(leaf) + VARHDRSZ; int padded_sz =3D INTALIGN(sz); = @@ -88,17 +113,19 @@ gbt_bit_xfrm(bytea *leaf) return out; } = - - - +/* + * Convert a GBT_VARKEY representation of a leaf key to a palloc'd + * GBT_VARKEY representation of an internal key. + * We assume lower =3D=3D upper since it's a leaf key. + */ static GBT_VARKEY * gbt_bit_l2n(GBT_VARKEY *leaf, FmgrInfo *flinfo) { - GBT_VARKEY *out =3D leaf; + GBT_VARKEY *out; GBT_VARKEY_R r =3D gbt_var_key_readable(leaf); bytea *o; = - o =3D gbt_bit_xfrm(r.lower); + o =3D gbt_bit_xfrm((VarBit *) r.lower); r.upper =3D r.lower =3D o; out =3D gbt_var_key_copy(&r); pfree(o); @@ -110,14 +137,14 @@ static const gbtree_vinfo tinfo =3D { gbt_t_bit, 0, - true, + true, /* internal keys can be truncated */ gbt_bitgt, gbt_bitge, gbt_biteq, gbt_bitle, gbt_bitlt, gbt_bitcmp, - gbt_bit_l2n + gbt_bit_l2n /* leaf to internal transformation */ }; = = @@ -137,7 +164,7 @@ Datum gbt_bit_consistent(PG_FUNCTION_ARGS) { GISTENTRY *entry =3D (GISTENTRY *) PG_GETARG_POINTER(0); - void *query =3D DatumGetByteaP(PG_GETARG_DATUM(1)); + VarBit *query =3D PG_GETARG_VARBIT_P(1); StrategyNumber strategy =3D (StrategyNumber) PG_GETARG_UINT16(2); #ifdef NOT_USED Oid subtype =3D PG_GETARG_OID(3); @@ -155,7 +182,8 @@ gbt_bit_consistent(PG_FUNCTION_ARGS) true, &tinfo, fcinfo->flinfo); else { - bytea *q =3D gbt_bit_xfrm((bytea *) query); + /* Must convert to internal form to compare to internal-page entries */ + bytea *q =3D gbt_bit_xfrm(query); = retval =3D gbt_var_consistent(&r, q, strategy, PG_GET_COLLATION(), false, &tinfo, fcinfo->flinfo); @@ -217,6 +245,7 @@ gbt_bit_ssup_cmp(Datum x, Datum y, SortSupport ssup) Datum result; = /* for leaf items we expect lower =3D=3D upper, so only compare lower */ + /* XXX shouldn't this use bitcmp() ? */ result =3D DirectFunctionCall2(byteacmp, PointerGetDatum(arg1.lower), PointerGetDatum(arg2.lower)); diff --git a/contrib/btree_gist/btree_bytea.c b/contrib/btree_gist/btree_b= ytea.c index 50bb24308e9..8411598a99b 100644 --- a/contrib/btree_gist/btree_bytea.c +++ b/contrib/btree_gist/btree_bytea.c @@ -1,5 +1,7 @@ /* * contrib/btree_gist/btree_bytea.c + * + * Support for bytea data type. */ #include "postgres.h" = @@ -72,7 +74,7 @@ static const gbtree_vinfo tinfo =3D { gbt_t_bytea, 0, - true, + true, /* internal keys can be truncated */ gbt_byteagt, gbt_byteage, gbt_byteaeq, diff --git a/contrib/btree_gist/btree_numeric.c b/contrib/btree_gist/btree= _numeric.c index dba04c3a1b3..8a506865b17 100644 --- a/contrib/btree_gist/btree_numeric.c +++ b/contrib/btree_gist/btree_numeric.c @@ -1,5 +1,7 @@ /* * contrib/btree_gist/btree_numeric.c + * + * Support for numeric data type. */ #include "postgres.h" = @@ -73,11 +75,16 @@ gbt_numeric_cmp(const void *a, const void *b, Oid coll= ation, FmgrInfo *flinfo) } = = +/* + * We could conceivably support internal-key truncation here, but it woul= d + * require custom truncation code, and most values wouldn't be long enoug= h + * to make it worthwhile. + */ static const gbtree_vinfo tinfo =3D { gbt_t_numeric, 0, - false, + false, /* no truncation permitted */ gbt_numeric_gt, gbt_numeric_ge, gbt_numeric_eq, diff --git a/contrib/btree_gist/btree_text.c b/contrib/btree_gist/btree_te= xt.c index 2ac12f1cab2..e7269ba3e7c 100644 --- a/contrib/btree_gist/btree_text.c +++ b/contrib/btree_gist/btree_text.c @@ -1,5 +1,7 @@ /* * contrib/btree_gist/btree_text.c + * + * Support for text and bpchar types. */ #include "postgres.h" = @@ -78,11 +80,17 @@ gbt_textcmp(const void *a, const void *b, Oid collatio= n, FmgrInfo *flinfo) PointerGetDatum(b))); } = +/* + * Originally this module permitted truncation of internal keys, but that + * tends to result in wrong comparison answers if the collation is any + * more complicated than C. The prefix-match hack used for simpler types + * can't fix it, either. + */ static gbtree_vinfo tinfo =3D { gbt_t_text, 0, - false, + false, /* no truncation permitted */ gbt_textgt, gbt_textge, gbt_texteq, @@ -152,7 +160,7 @@ static gbtree_vinfo bptinfo =3D { gbt_t_bpchar, 0, - false, + false, /* as above, no truncation permitted */ gbt_bpchargt, gbt_bpcharge, gbt_bpchareq, diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btr= ee_utils_var.c index 25c3bbe8eac..bceadf5ae60 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -1,5 +1,7 @@ /* * contrib/btree_gist/btree_utils_var.c + * + * Common routines for btree_gist code working with varlena indexed data = types. */ #include "postgres.h" = @@ -37,6 +39,7 @@ gbt_var_decompress(PG_FUNCTION_ARGS) GISTENTRY *entry =3D (GISTENTRY *) PG_GETARG_POINTER(0); GBT_VARKEY *key =3D (GBT_VARKEY *) PG_DETOAST_DATUM(entry->key); = + /* We only need a new GISTENTRY if detoasting did something */ if (key !=3D (GBT_VARKEY *) DatumGetPointer(entry->key)) { GISTENTRY *retval =3D palloc_object(GISTENTRY); @@ -51,7 +54,10 @@ gbt_var_decompress(PG_FUNCTION_ARGS) PG_RETURN_POINTER(entry); } = -/* Returns a better readable representation of variable key ( sets pointe= r ) */ +/* + * Extract a "readable" representation of a GBT_VARKEY, containing direct + * pointers to the contained lower and upper datums. + */ GBT_VARKEY_R gbt_var_key_readable(const GBT_VARKEY *k) { @@ -83,7 +89,10 @@ gbt_var_key_from_datum(const varlena *u) } = /* - * Create an entry to store in the index, from lower and upper bound. + * Create a key entry to store in the index, from lower and upper bound. + * + * This code assumes that none of the types we work with require more + * than INTALIGN alignment. */ GBT_VARKEY * gbt_var_key_copy(const GBT_VARKEY_R *u) @@ -100,16 +109,17 @@ gbt_var_key_copy(const GBT_VARKEY_R *u) return r; } = - +/* + * Convert a GBT_VARKEY in leaf form to a GBT_VARKEY in internal form. + * No-op if the data type doesn't require a transformation. + */ static GBT_VARKEY * gbt_var_leaf2node(GBT_VARKEY *leaf, const gbtree_vinfo *tinfo, FmgrInfo *= flinfo) { - GBT_VARKEY *out =3D leaf; - if (tinfo->f_l2n) - out =3D tinfo->f_l2n(leaf, flinfo); - - return out; + return tinfo->f_l2n(leaf, flinfo); + else + return leaf; } = = @@ -173,7 +183,10 @@ gbt_var_node_cp_len(const GBT_VARKEY *node, const gbt= ree_vinfo *tinfo) = = /* - * returns true, if query matches prefix ( common prefix ) + * returns true if query matches prefix up to the length of the prefix + * + * We need this to avoid edge-case problems when the "prefix" is a trunca= ted + * datum; see discussion in btree_utils_var.h. */ static bool gbt_bytea_pf_match(const bytea *pf, const bytea *query, const gbtree_vinf= o *tinfo) @@ -195,7 +208,14 @@ gbt_bytea_pf_match(const bytea *pf, const bytea *quer= y, const gbtree_vinfo *tinf = = /* - * returns true, if query matches node using common prefix + * returns true if query matches node according to common-prefix rule + * + * If the data type is truncatable, then a shortened upper bound must be + * considered to include all values that match it up to its own length, + * even though longer values would normally be considered larger. + * + * XXX isn't the check against node->lower useless? + * A truncated lower bound would already be less than all included values= . */ static bool gbt_var_node_pf_match(const GBT_VARKEY_R *node, const bytea *query, const= gbtree_vinfo *tinfo) @@ -207,13 +227,16 @@ gbt_var_node_pf_match(const GBT_VARKEY_R *node, cons= t bytea *query, const gbtree = = /* - * truncates / compresses the node key - * cpf_length .. common prefix length + * truncates / compresses the node key + * + * cpf_length is the common prefix length of the lower and upper values. + * We truncate to that plus one byte, so that the node represents a range + * of leaf values but doesn't have undue specificity. */ static GBT_VARKEY * gbt_var_node_truncate(const GBT_VARKEY *node, int32 cpf_length, const gbt= ree_vinfo *tinfo) { - GBT_VARKEY *out =3D NULL; + GBT_VARKEY *out; GBT_VARKEY_R r =3D gbt_var_key_readable(node); int32 len1 =3D VARSIZE(r.lower) - VARHDRSZ; int32 len2 =3D VARSIZE(r.upper) - VARHDRSZ; @@ -246,7 +269,7 @@ gbt_var_bin_union(Datum *u, GBT_VARKEY *e, Oid collati= on, GBT_VARKEY_R eo =3D gbt_var_key_readable(e); GBT_VARKEY_R nr; = - if (eo.lower =3D=3D eo.upper) /* leaf */ + if (eo.lower =3D=3D eo.upper) /* if leaf, convert to internal form */ { GBT_VARKEY *tmp; = @@ -355,7 +378,7 @@ gbt_var_union(const GistEntryVector *entryvec, int32 *= size, Oid collation, if (tinfo->trnc) { int32 plen; - GBT_VARKEY *trc =3D NULL; + GBT_VARKEY *trc; = plen =3D gbt_var_node_cp_len((GBT_VARKEY *) DatumGetPointer(out), tinfo= ); trc =3D gbt_var_node_truncate((GBT_VARKEY *) DatumGetPointer(out), plen= + 1, tinfo); @@ -396,7 +419,7 @@ gbt_var_penalty(float *res, const GISTENTRY *o, const = GISTENTRY *n, *res =3D 0.0; = nk =3D gbt_var_key_readable(newe); - if (nk.lower =3D=3D nk.upper) /* leaf */ + if (nk.lower =3D=3D nk.upper) /* if leaf, convert to internal form */ { GBT_VARKEY *tmp; = diff --git a/contrib/btree_gist/btree_utils_var.h b/contrib/btree_gist/btr= ee_utils_var.h index f28a5d98ae3..c0ac77cb9ec 100644 --- a/contrib/btree_gist/btree_utils_var.h +++ b/contrib/btree_gist/btree_utils_var.h @@ -1,5 +1,7 @@ /* * contrib/btree_gist/btree_utils_var.h + * + * Declarations for btree_gist code working with varlena indexed data typ= es. */ #ifndef __BTREE_UTILS_VAR_H__ #define __BTREE_UTILS_VAR_H__ @@ -7,10 +9,21 @@ #include "access/gist.h" #include "btree_gist.h" = -/* Variable length key */ +/* + * An internal index key (also called a node in some places) includes a + * lower and an upper bound, both of which are varlena datums, packed int= o + * a wrapper varlena datum. We also work with leaf keys, which contain a + * single varlena datum wrapped in another varlena; this case can be + * distinguished by noting that the wrapper isn't long enough to hold a + * second datum. For simplicity we consider the wrapper to be a bytea. + */ typedef bytea GBT_VARKEY; = -/* Better readable key */ +/* + * To actually work on a key, we use this more readable struct containing + * pointers directly to the lower and upper values. gbt_var_key_readable= () + * and gbt_var_key_copy() convert between these two representations. + */ typedef struct { bytea *lower, @@ -19,6 +32,27 @@ typedef struct = /* * type description + * + * For types that set the "trnc" flag, the representation of keys on inte= rnal + * pages may be different from that of keys on leaf pages (which match th= e + * data type's normal representation). The methods f_gt, f_ge, f_eq, f_l= e, + * f_lt expect to work on the normal representation, and therefore can be= used + * only with leaf-page index entries. The f_cmp method expects to work o= n the + * representation used on internal pages, so it must not be used with lea= f + * entries. The f_l2n method converts the leaf-page representation to th= e + * internal-page representation; if that pointer is NULL, they're the sam= e. + * + * Currently, btree_utils_var.c effectively assumes that internal-page ke= ys + * of "trnc" types are equivalent to bytea in representation and semantic= s. + * The truncation process shortens the lower+upper bounds of a downlink n= ode + * to be of length equal to their common prefix's length plus one byte. + * This would not work for types with comparison semantics more complex t= han + * bytewise comparison. Even then, we need a hack to deal with the fact = that + * shortening the upper bound would normally lead to its being considered= less + * than the original maximum leaf-page entry. We handle that by consider= ing + * any search key that matches the bound for the bound's full length to b= e a + * potential match, even if it's longer (see gbt_var_node_pf_match and it= s + * callers). */ typedef struct { -- = 2.52.0 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v3-0002-Fix-btree_gist-s-NotEqual-strategy-on-internal-in.p"; name*1="atch"; charset="UTF-8" Content-ID: <3793655.1783033301.3@sss.pgh.pa.us> Content-Description: v3-0002-Fix-btree_gist-s-NotEqual-strategy-on-internal-in.patch Content-Transfer-Encoding: quoted-printable =46rom 79d8d82023c951086b484a52e85524c48625536c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Jul 2026 17:19:30 -0400 Subject: [PATCH v3 2/5] Fix btree_gist's NotEqual strategy on internal ind= ex pages. MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit gbt_var_consistent() handled the <> (BtreeGistNotEqual) strategy without distinguishing leaf from internal pages, unlike every other strategy. In particular, it tried to apply the datatype-specific f_eq method, which is completely wrong since internal keys might not have the same representation as leaf keys. This led to OOB reads and potentially crashes, and most likely to wrong query results as well. On leaf pages we can apply the inverse of what the Equal strategy does. On internal pages, just descend unconditionally: there's no useful way to prove that a leaf page can't contain any unequal values. (Maybe we could do something in the case where upper and lower bounds are equal, which is what the previous coding seemed to be thinking about. But that case is unlikely, and anyway it won't work for truncated bounds.) Reported-by: =E7=8E=8B=E8=B7=83=E6=9E=97 Author: Ayush Tiwari Reviewed-by: Tom Lane Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.H= mail.3020001251@tju.edu.cn Backpatch-through: 14 --- contrib/btree_gist/btree_utils_var.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btr= ee_utils_var.c index bceadf5ae60..99919ca3974 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -594,6 +594,13 @@ gbt_var_consistent(GBT_VARKEY_R *key, { bool retval =3D false; = + /* + * Remember that f_cmp is for internal pages, f_eq etc for leaf pages, a= nd + * on internal pages we need to check gbt_var_node_pf_match too. + * + * The leaf-page tests use swapped operands (e.g., f_gt(query, lower) + * means "lower < query"), which is why they look reversed. + */ switch (strategy) { case BTLessEqualStrategyNumber: @@ -634,8 +641,13 @@ gbt_var_consistent(GBT_VARKEY_R *key, || gbt_var_node_pf_match(key, query, tinfo); break; case BtreeGistNotEqualStrategyNumber: - retval =3D !(tinfo->f_eq(query, key->lower, collation, flinfo) && - tinfo->f_eq(query, key->upper, collation, flinfo)); + if (is_leaf) + retval =3D !(tinfo->f_eq(query, key->lower, collation, flinfo)); + else + { + /* we can never disprove existence of a not-equal entry below */ + retval =3D true; + } break; default: retval =3D false; -- = 2.52.0 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v3-0003-Use-the-proper-comparator-in-gbt_bit_ssup_cmp.patch"; charset="us-ascii" Content-ID: <3793655.1783033301.4@sss.pgh.pa.us> Content-Description: v3-0003-Use-the-proper-comparator-in-gbt_bit_ssup_cmp.patch Content-Transfer-Encoding: quoted-printable =46rom 4a2d39891e1f3801ee68647c9b0aa49ad7027b2c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Jul 2026 17:27:31 -0400 Subject: [PATCH v3 3/5] Use the proper comparator in gbt_bit_ssup_cmp. If we're dealing with leaf entries, the function to call is bitcmp not byteacmp. Using byteacmp didn't lead to any obvious failure, but it did result in sorting the entries in a way not matching the datatype's actual sort order, so that the constructed index would be much less efficient than one would expect, and in particular worse than what you got before this code was added in v18. We might want to recommend that users reindex btree_gist indexes on bit/varbit. Author: Tom Lane Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.H= mail.3020001251@tju.edu.cn Backpatch-through: 18 --- contrib/btree_gist/btree_bit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit= .c index dcfe6ed17e4..8d8e9454268 100644 --- a/contrib/btree_gist/btree_bit.c +++ b/contrib/btree_gist/btree_bit.c @@ -245,8 +245,7 @@ gbt_bit_ssup_cmp(Datum x, Datum y, SortSupport ssup) Datum result; = /* for leaf items we expect lower =3D=3D upper, so only compare lower */ - /* XXX shouldn't this use bitcmp() ? */ - result =3D DirectFunctionCall2(byteacmp, + result =3D DirectFunctionCall2(bitcmp, PointerGetDatum(arg1.lower), PointerGetDatum(arg2.lower)); = -- = 2.52.0 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v3-0004-Remove-useless-check-against-lower-bound-in-gbt_v.p"; name*1="atch"; charset="us-ascii" Content-ID: <3793655.1783033301.5@sss.pgh.pa.us> Content-Description: v3-0004-Remove-useless-check-against-lower-bound-in-gbt_v.patch Content-Transfer-Encoding: quoted-printable =46rom 4584b8d96e250d6e3cc854a279df80f654719d1f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Jul 2026 17:39:15 -0400 Subject: [PATCH v3 4/5] Remove useless check against lower bound in gbt_var_node_pf_match. Truncating an internal node's upper bound can cause it to compare less than some values that in fact are included in the represented leaf page. So we need a hack to make sure it looks large enough to include all values that could be on the page. But there's no equivalent issue for the lower bound. The fact that this code did a fuzzy comparison for the lower bound seems to be the result of fuzzy thinking. Or maybe there was a desire to not assume too much about what the datatype's comparison rule is; but we've already fully bought into the premise that internal keys compare like bytea. The comparable check in gbt_var_penalty() may also be useless, but I'm not quite sure. In any case that seems negligible from a performance standpoint, so I left it alone. Author: Tom Lane Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.H= mail.3020001251@tju.edu.cn --- contrib/btree_gist/btree_utils_var.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btr= ee_utils_var.c index 99919ca3974..f1b13741630 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -213,16 +213,14 @@ gbt_bytea_pf_match(const bytea *pf, const bytea *que= ry, const gbtree_vinfo *tinf * If the data type is truncatable, then a shortened upper bound must be * considered to include all values that match it up to its own length, * even though longer values would normally be considered larger. - * - * XXX isn't the check against node->lower useless? - * A truncated lower bound would already be less than all included values= . + * We don't need to check the lower bound though: shortening it just + * makes it even smaller. */ static bool gbt_var_node_pf_match(const GBT_VARKEY_R *node, const bytea *query, const= gbtree_vinfo *tinfo) { return (tinfo->trnc && - (gbt_bytea_pf_match(node->lower, query, tinfo) || - gbt_bytea_pf_match(node->upper, query, tinfo))); + gbt_bytea_pf_match(node->upper, query, tinfo)); } = = -- = 2.52.0 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v3-0005-Remove-btree_gist-s-useless-logic-for-encoding-aw.p"; name*1="atch"; charset="us-ascii" Content-ID: <3793655.1783033301.6@sss.pgh.pa.us> Content-Description: v3-0005-Remove-btree_gist-s-useless-logic-for-encoding-aw.patch Content-Transfer-Encoding: quoted-printable =46rom 4225a4b2e51757f12d42459cd993dee674c04739 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Jul 2026 18:11:10 -0400 Subject: [PATCH v3 5/5] Remove btree_gist's useless logic for encoding-awa= re truncation. gbt_var_node_cp_len() contained logic to ensure that its choice of a common prefix length didn't truncate away part of a multibyte character. However, that was really dead code, because we have not allowed truncation of text-string data types since ef770cbb6, and it seems unlikely that that behavior could ever get resurrected. The code is still reachable via gbt_var_penalty, but for that usage it hardly matters if we break in the middle of a multibyte character: we're just calculating a small correction factor that is arguably bunkum anyway in non-C locales. Hence, delete said code. That actually removes all need for gbtree_vinfo.eml, which allows const-ification of the gbtree_vinfo structs in which we were changing it, which removes one headache for future attempts to thread-ify the backend. (Curiously, all this infrastructure was itself added by ef770cbb6. Not sure why Teodor didn't see the contradiction.) Author: Tom Lane Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.H= mail.3020001251@tju.edu.cn --- contrib/btree_gist/btree_bit.c | 1 - contrib/btree_gist/btree_bytea.c | 1 - contrib/btree_gist/btree_numeric.c | 1 - contrib/btree_gist/btree_text.c | 21 ++-------------- contrib/btree_gist/btree_utils_var.c | 36 +++++----------------------- contrib/btree_gist/btree_utils_var.h | 2 -- 6 files changed, 8 insertions(+), 54 deletions(-) diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit= .c index 8d8e9454268..5fb8c8b82f0 100644 --- a/contrib/btree_gist/btree_bit.c +++ b/contrib/btree_gist/btree_bit.c @@ -136,7 +136,6 @@ gbt_bit_l2n(GBT_VARKEY *leaf, FmgrInfo *flinfo) static const gbtree_vinfo tinfo =3D { gbt_t_bit, - 0, true, /* internal keys can be truncated */ gbt_bitgt, gbt_bitge, diff --git a/contrib/btree_gist/btree_bytea.c b/contrib/btree_gist/btree_b= ytea.c index 8411598a99b..bcb469e8c86 100644 --- a/contrib/btree_gist/btree_bytea.c +++ b/contrib/btree_gist/btree_bytea.c @@ -73,7 +73,6 @@ gbt_byteacmp(const void *a, const void *b, Oid collation= , FmgrInfo *flinfo) static const gbtree_vinfo tinfo =3D { gbt_t_bytea, - 0, true, /* internal keys can be truncated */ gbt_byteagt, gbt_byteage, diff --git a/contrib/btree_gist/btree_numeric.c b/contrib/btree_gist/btree= _numeric.c index 8a506865b17..7a75e2ed00d 100644 --- a/contrib/btree_gist/btree_numeric.c +++ b/contrib/btree_gist/btree_numeric.c @@ -83,7 +83,6 @@ gbt_numeric_cmp(const void *a, const void *b, Oid collat= ion, FmgrInfo *flinfo) static const gbtree_vinfo tinfo =3D { gbt_t_numeric, - 0, false, /* no truncation permitted */ gbt_numeric_gt, gbt_numeric_ge, diff --git a/contrib/btree_gist/btree_text.c b/contrib/btree_gist/btree_te= xt.c index e7269ba3e7c..a8657fe461a 100644 --- a/contrib/btree_gist/btree_text.c +++ b/contrib/btree_gist/btree_text.c @@ -86,10 +86,9 @@ gbt_textcmp(const void *a, const void *b, Oid collation= , FmgrInfo *flinfo) * more complicated than C. The prefix-match hack used for simpler types * can't fix it, either. */ -static gbtree_vinfo tinfo =3D +static const gbtree_vinfo tinfo =3D { gbt_t_text, - 0, false, /* no truncation permitted */ gbt_textgt, gbt_textge, @@ -156,10 +155,9 @@ gbt_bpcharcmp(const void *a, const void *b, Oid colla= tion, FmgrInfo *flinfo) PointerGetDatum(b))); } = -static gbtree_vinfo bptinfo =3D +static const gbtree_vinfo bptinfo =3D { gbt_t_bpchar, - 0, false, /* as above, no truncation permitted */ gbt_bpchargt, gbt_bpcharge, @@ -180,11 +178,6 @@ gbt_text_compress(PG_FUNCTION_ARGS) { GISTENTRY *entry =3D (GISTENTRY *) PG_GETARG_POINTER(0); = - if (tinfo.eml =3D=3D 0) - { - tinfo.eml =3D pg_database_encoding_max_length(); - } - PG_RETURN_POINTER(gbt_var_compress(entry, &tinfo)); } = @@ -212,11 +205,6 @@ gbt_text_consistent(PG_FUNCTION_ARGS) /* All cases served by this function are exact */ *recheck =3D false; = - if (tinfo.eml =3D=3D 0) - { - tinfo.eml =3D pg_database_encoding_max_length(); - } - retval =3D gbt_var_consistent(&r, query, strategy, PG_GET_COLLATION(), GIST_LEAF(entry), &tinfo, fcinfo->flinfo); = @@ -240,11 +228,6 @@ gbt_bpchar_consistent(PG_FUNCTION_ARGS) /* All cases served by this function are exact */ *recheck =3D false; = - if (bptinfo.eml =3D=3D 0) - { - bptinfo.eml =3D pg_database_encoding_max_length(); - } - retval =3D gbt_var_consistent(&r, query, strategy, PG_GET_COLLATION(), GIST_LEAF(entry), &bptinfo, fcinfo->flinfo); PG_RETURN_BOOL(retval); diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btr= ee_utils_var.c index f1b13741630..60373aaf07c 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -126,56 +126,32 @@ gbt_var_leaf2node(GBT_VARKEY *leaf, const gbtree_vin= fo *tinfo, FmgrInfo *flinfo) /* * returns the common prefix length of a node key * - * If the underlying type is character data, the prefix length may point = in - * the middle of a multibyte character. + * This is not used for any cases where the underlying type is character = data + * (except in gbt_var_penalty, where it doesn't matter since we're just m= aking + * an estimated correction not constructing a truncated string). If it w= ere, + * we'd need to be careful not to truncate in the middle of a multibyte + * character. */ static int32 gbt_var_node_cp_len(const GBT_VARKEY *node, const gbtree_vinfo *tinfo) { GBT_VARKEY_R r =3D gbt_var_key_readable(node); int32 i =3D 0; - int32 l_left_to_match =3D 0; - int32 l_total =3D 0; int32 t1len =3D VARSIZE(r.lower) - VARHDRSZ; int32 t2len =3D VARSIZE(r.upper) - VARHDRSZ; int32 ml =3D Min(t1len, t2len); char *p1 =3D VARDATA(r.lower); char *p2 =3D VARDATA(r.upper); - const char *end1 =3D p1 + t1len; - const char *end2 =3D p2 + t2len; = if (ml =3D=3D 0) return 0; = while (i < ml) { - if (tinfo->eml > 1 && l_left_to_match =3D=3D 0) - { - l_total =3D pg_mblen_range(p1, end1); - if (l_total !=3D pg_mblen_range(p2, end2)) - { - return i; - } - l_left_to_match =3D l_total; - } if (*p1 !=3D *p2) - { - if (tinfo->eml > 1) - { - int32 l_matched_subset =3D l_total - l_left_to_match; - - /* end common prefix at final byte of last matching char */ - return i - l_matched_subset; - } - else - { - return i; - } - } - + return i; p1++; p2++; - l_left_to_match--; i++; } return ml; /* lower =3D=3D upper */ diff --git a/contrib/btree_gist/btree_utils_var.h b/contrib/btree_gist/btr= ee_utils_var.h index c0ac77cb9ec..4750fc8361e 100644 --- a/contrib/btree_gist/btree_utils_var.h +++ b/contrib/btree_gist/btree_utils_var.h @@ -60,8 +60,6 @@ typedef struct /* Attribs */ = enum gbtree_type t; /* data type */ - int32 eml; /* cached pg_database_encoding_max_length (0: - * undefined) */ bool trnc; /* truncate (=3Dcompress) key */ = /* Methods */ -- = 2.52.0 ------- =_aaaaaaaaaa0--