diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c index 79c6223f99c..971485dc8eb 100644 --- a/contrib/btree_gist/btree_utils_num.c +++ b/contrib/btree_gist/btree_utils_num.c @@ -269,36 +269,54 @@ gbt_num_consistent(const GBT_NUMKEY_R *key, { bool retval; + /* + * On leaf pages we directly apply the check "key->lower OP query"; we + * need not consider key->upper since it will be equal to key->lower. + * + * On internal pages we mostly need to check "is lower bound below query?" + * and/or "is upper bound above query?", where we must allow equality in + * both cases. + */ +#define lower_is_below_query() \ + tinfo->f_le(key->lower, query, flinfo) +#define upper_is_above_query() \ + tinfo->f_ge(key->upper, query, flinfo) + switch (strategy) { case BTLessEqualStrategyNumber: - retval = tinfo->f_ge(query, key->lower, flinfo); + if (is_leaf) + retval = tinfo->f_le(key->lower, query, flinfo); + else + retval = lower_is_below_query(); break; case BTLessStrategyNumber: if (is_leaf) - retval = tinfo->f_gt(query, key->lower, flinfo); + retval = tinfo->f_lt(key->lower, query, flinfo); else - retval = tinfo->f_ge(query, key->lower, flinfo); + retval = lower_is_below_query(); break; case BTEqualStrategyNumber: if (is_leaf) - retval = tinfo->f_eq(query, key->lower, flinfo); + retval = tinfo->f_eq(key->lower, query, flinfo); else - retval = (tinfo->f_le(key->lower, query, flinfo) && - tinfo->f_le(query, key->upper, flinfo)); + retval = lower_is_below_query() && upper_is_above_query(); break; case BTGreaterStrategyNumber: if (is_leaf) - retval = tinfo->f_lt(query, key->upper, flinfo); + retval = tinfo->f_gt(key->lower, query, flinfo); else - retval = tinfo->f_le(query, key->upper, flinfo); + retval = upper_is_above_query(); break; case BTGreaterEqualStrategyNumber: - retval = tinfo->f_le(query, key->upper, flinfo); + if (is_leaf) + retval = tinfo->f_ge(key->lower, query, flinfo); + else + retval = upper_is_above_query(); break; case BtreeGistNotEqualStrategyNumber: if (is_leaf) - retval = !(tinfo->f_eq(query, key->lower, flinfo)); + retval = !(tinfo->f_eq(key->lower, query, flinfo)); else { /* @@ -307,14 +325,17 @@ gbt_num_consistent(const GBT_NUMKEY_R *key, * 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)); + retval = !(tinfo->f_eq(key->lower, query, flinfo) && + tinfo->f_eq(key->upper, query, flinfo)); } break; default: retval = false; } +#undef lower_is_below_query +#undef upper_is_above_query + return retval; } diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c index 6ab2ca263ef..ae101801355 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 *query, 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)); } @@ -592,57 +590,60 @@ gbt_var_consistent(const GBT_VARKEY_R *key, const gbtree_vinfo *tinfo, FmgrInfo *flinfo) { - bool retval = false; + bool retval; /* - * 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. + * On leaf pages we directly apply the check "key->lower OP query"; we + * need not consider key->upper since it will be equal to key->lower. * - * The leaf-page tests use swapped operands (e.g., f_gt(query, lower) - * means "lower < query"), which is why they look reversed. + * On internal pages we mostly need to check "is lower bound below query?" + * and/or "is upper bound above query?", where we must allow equality in + * both cases. When checking against the upper bound we have to allow a + * gbt_var_node_pf_match too. + * + * Remember that f_cmp is for internal pages, f_eq etc for leaf pages. */ +#define lower_is_below_query() \ + (tinfo->f_cmp(key->lower, query, collation, flinfo) <= 0) +#define upper_is_above_query() \ + (tinfo->f_cmp(key->upper, query, collation, flinfo) >= 0 || \ + gbt_var_node_pf_match(key, query, tinfo)) + switch (strategy) { case BTLessEqualStrategyNumber: if (is_leaf) - retval = tinfo->f_ge(query, key->lower, collation, flinfo); + retval = tinfo->f_le(key->lower, query, collation, flinfo); else - retval = tinfo->f_cmp(query, key->lower, collation, flinfo) >= 0 - || gbt_var_node_pf_match(key, query, tinfo); + retval = lower_is_below_query(); break; case BTLessStrategyNumber: if (is_leaf) - retval = tinfo->f_gt(query, key->lower, collation, flinfo); + retval = tinfo->f_lt(key->lower, query, collation, flinfo); else - retval = tinfo->f_cmp(query, key->lower, collation, flinfo) >= 0 - || gbt_var_node_pf_match(key, query, tinfo); + retval = lower_is_below_query(); break; case BTEqualStrategyNumber: if (is_leaf) - retval = tinfo->f_eq(query, key->lower, collation, flinfo); + retval = tinfo->f_eq(key->lower, query, collation, flinfo); else - retval = - (tinfo->f_cmp(key->lower, query, collation, flinfo) <= 0 && - tinfo->f_cmp(query, key->upper, collation, flinfo) <= 0) || - gbt_var_node_pf_match(key, query, tinfo); + retval = lower_is_below_query() && upper_is_above_query(); break; case BTGreaterStrategyNumber: if (is_leaf) - retval = tinfo->f_lt(query, key->upper, collation, flinfo); + retval = tinfo->f_gt(key->lower, query, collation, flinfo); else - retval = tinfo->f_cmp(query, key->upper, collation, flinfo) <= 0 - || gbt_var_node_pf_match(key, query, tinfo); + retval = upper_is_above_query(); break; case BTGreaterEqualStrategyNumber: if (is_leaf) - retval = tinfo->f_le(query, key->upper, collation, flinfo); + retval = tinfo->f_ge(key->lower, query, collation, flinfo); else - retval = tinfo->f_cmp(query, key->upper, collation, flinfo) <= 0 - || gbt_var_node_pf_match(key, query, tinfo); + retval = upper_is_above_query(); break; case BtreeGistNotEqualStrategyNumber: if (is_leaf) - retval = !(tinfo->f_eq(query, key->lower, collation, flinfo)); + retval = !(tinfo->f_eq(key->lower, query, collation, flinfo)); else { /* @@ -652,13 +653,16 @@ gbt_var_consistent(const GBT_VARKEY_R *key, * 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); + !(tinfo->f_cmp(key->lower, query, collation, flinfo) == 0 && + tinfo->f_cmp(key->upper, query, collation, flinfo) == 0); } break; default: retval = false; } +#undef lower_is_below_query +#undef upper_is_above_query + return retval; }