From 354191c27867d9dc5672f1abad14941e29e3d595 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sat, 13 Jul 2019 00:12:16 +0200 Subject: [PATCH 2/2] Fix processing of OpExpr in extended statistics --- src/backend/statistics/extended_stats.c | 64 ++++++++++++++++++++----- src/backend/statistics/mcv.c | 23 +++------ src/include/statistics/statistics.h | 1 + 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 2ed28c054f..1c7ca07661 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -797,21 +797,13 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, RangeTblEntry *rte = root->simple_rte_array[relid]; OpExpr *expr = (OpExpr *) clause; Var *var; - bool varonleft = true; - bool ok; /* Only expressions with two arguments are considered compatible. */ if (list_length(expr->args) != 2) return false; - /* see if it actually has the right shape (one Var, one Const) */ - ok = (NumRelids((Node *) expr) == 1) && - (is_pseudo_constant_clause(lsecond(expr->args)) || - (varonleft = false, - is_pseudo_constant_clause(linitial(expr->args)))); - - /* unsupported structure (two variables or so) */ - if (!ok) + /* Check if the expression the right shape (one Var, one Const) */ + if (!deconstruct_opexpr(expr, &var, NULL, NULL)) return false; /* @@ -843,8 +835,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, !get_func_leakproof(get_opcode(expr->opno))) return false; - var = (varonleft) ? linitial(expr->args) : lsecond(expr->args); - return statext_is_compatible_clause_internal(root, (Node *) var, relid, attnums); } @@ -1235,3 +1225,53 @@ determine_operator_strategy(Oid opno) return strat; } + +/* + * deconstruct_opexpr + * Split expression into Var and Const parts. + * + * Attempts to match the arguments to either (Var op Const) or (Const op Var), + * possibly with a RelabelType on top of the Var node. When successful, returns + * true, otherwise returns false. + * + * Optionally returns pointers to the extracted elements, when passed non-null + * pointers (varp, cstp and isgtp). + */ +bool +deconstruct_opexpr(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp) +{ + Var *var; + Const *cst; + bool isgt; + + /* enforced by statext_is_compatible_clause_internal */ + Assert(list_length(expr->args) == 2); + + if ((IsA(linitial(expr->args), Var) || IsA(linitial(expr->args), RelabelType)) && + IsA(lsecond(expr->args), Const)) + { + var = linitial(expr->args); + cst = lsecond(expr->args); + isgt = false; + } + else if (IsA(linitial(expr->args), Const) && + (IsA(lsecond(expr->args), Var) || IsA(lsecond(expr->args), RelabelType))) + { + var = lsecond(expr->args); + cst = linitial(expr->args); + isgt = true; + } + else + return false; + + if (varp) + *varp = var; + + if (cstp) + *cstp = cst; + + if (isgtp) + *isgtp = isgt; + + return true; +} diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 1e919c82f9..0d7f6969ee 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -1561,32 +1561,23 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, if (is_opclause(clause)) { OpExpr *expr = (OpExpr *) clause; - bool varonleft = true; - bool ok; FmgrInfo opproc; - fmgr_info(get_opcode(expr->opno), &opproc); + /* valid only after deconstruct_opexpr returns true */ + Var *var; + Const *cst; + bool isgt; - ok = (NumRelids(clause) == 1) && - (is_pseudo_constant_clause(lsecond(expr->args)) || - (varonleft = false, - is_pseudo_constant_clause(linitial(expr->args)))); + fmgr_info(get_opcode(expr->opno), &opproc); - if (ok) + /* extract the var and const from the expression */ + if (deconstruct_opexpr(expr, &var, &cst, &isgt)) { TypeCacheEntry *typecache; FmgrInfo gtproc; - Var *var; - Const *cst; - bool isgt; int idx; int strategy; - /* extract the var and const from the expression */ - var = (varonleft) ? linitial(expr->args) : lsecond(expr->args); - cst = (varonleft) ? lsecond(expr->args) : linitial(expr->args); - isgt = (!varonleft); - /* strip binary-compatible relabeling */ if (IsA(var, RelabelType)) var = (Var *) ((RelabelType *) var)->arg; diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h index 82e2d5c619..f5120d0663 100644 --- a/src/include/statistics/statistics.h +++ b/src/include/statistics/statistics.h @@ -119,5 +119,6 @@ extern bool has_stats_of_kind(List *stats, char requiredkind); extern StatisticExtInfo *choose_best_statistics(List *stats, Bitmapset *attnums, char requiredkind); extern int determine_operator_strategy(Oid opno); +extern bool deconstruct_opexpr(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp); #endif /* STATISTICS_H */ -- 2.20.1