Fix btree_gist's NotEqual strategy on internal index pages.

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, use a correct implementation of what the previous
code intended: we can descend if the query value equals both bounds,
*so long as the bounds aren't truncated*.  With truncated bounds we
don't quite know the range of what's below, so we must always descend.

Adjust the code in gbt_num_consistent() to look similar, too.  This
fixes a performance buglet in that there's no need to do two comparisons
on a leaf entry, but the main point is just to keep code consistency.

Reported-by: 王跃林 <violin0613@tju.edu.cn>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
Backpatch-through: 14

diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c
index 3affe4c2c4643ab6ac13b087b8ff4c76f4f8b119..b0e41de00e1bb62157b9472de5ee8b6ac740d9ac 100644 (file)
--- a/contrib/btree_gist/btree_utils_num.c
+++ b/contrib/btree_gist/btree_utils_num.c
@@ -297,8 +297,19 @@ gbt_num_consistent(const GBT_NUMKEY_R *key,
                        retval = tinfo->f_le(query, key->upper, flinfo);
                        break;
                case BtreeGistNotEqualStrategyNumber:
-                       retval = (!(tinfo->f_eq(query, key->lower, flinfo) &&
-                                               tinfo->f_eq(query, key->upper, flinfo)));
+                       if (is_leaf)
+                               retval = !(tinfo->f_eq(query, key->lower, flinfo));
+                       else
+                       {
+                               /*
+                                * If the upper/lower bounds are equal, then all entries below
+                                * this node must have exactly that value.  So we can avoid
+                                * descending if the query equals both bounds.  In all other
+                                * cases, we must descend.
+                                */
+                               retval = !(tinfo->f_eq(query, key->lower, flinfo) &&
+                                                  tinfo->f_eq(query, key->upper, flinfo));
+                       }
                        break;
                default:
                        retval = false;

diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c
index bceadf5ae60cbff46b0f6005de239a228dbdecf3..945fa67de3e1eb8a093c669c74327fbb564de4ca 100644 (file)
--- 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 = false;
 
+       /*
+        * Remember that f_cmp is for internal pages, f_eq etc for leaf pages, and
+        * 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,20 @@ gbt_var_consistent(GBT_VARKEY_R *key,
                                        || gbt_var_node_pf_match(key, query, tinfo);
                        break;
                case BtreeGistNotEqualStrategyNumber:
-                       retval = !(tinfo->f_eq(query, key->lower, collation, flinfo) &&
-                                          tinfo->f_eq(query, key->upper, collation, flinfo));
+                       if (is_leaf)
+                               retval = !(tinfo->f_eq(query, key->lower, collation, flinfo));
+                       else
+                       {
+                               /*
+                                * If the upper/lower bounds are equal and not truncated, then
+                                * all entries below this node must have exactly that value.
+                                * So we can avoid descending if the query equals both bounds.
+                                * In all other cases, we must descend.
+                                */
+                               retval = tinfo->trnc ||
+                                       !(tinfo->f_cmp(query, key->lower, collation, flinfo) == 0 &&
+                                         tinfo->f_cmp(query, key->upper, collation, flinfo) == 0);
+                       }
                        break;
                default:
                        retval = false;