From e8b6ae2130e3a95bb776708a9a7c9cb21fe8ac87 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Sat, 13 Jul 2019 00:12:16 +0200
Subject: [PATCH] Fix handling of opclauses in extended statistics
We expect opclauses to have exactly one Var and one Const, but the code
was checking the Const by calling is_pseudo_constant_clause() which is
incorrect - we need a proper constant.
Fixed by using plain IsA(x,Const) to check type of the node. We need to
do these checks in two places, so move it into a separate function that
can be called in both places.
Reported by Andreas Seltenreich, based on crash reported by sqlsmith.
Backpatch to v12, where this code was introduced.
Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu
Backpatch-to: 12
---
src/backend/statistics/extended_stats.c | 76 ++++++++++++++++---
src/backend/statistics/mcv.c | 27 ++-----
.../statistics/extended_stats_internal.h | 2 +
3 files changed, 73 insertions(+), 32 deletions(-)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index c56ed48270..d2346cbe02 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -796,21 +796,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 (!examine_opclause_expression(expr, &var, NULL, NULL))
return false;
/*
@@ -850,8 +842,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);
}
@@ -1196,3 +1186,65 @@ statext_clauselist_selectivity(PlannerInfo *root, List *clauses, int varRelid,
return sel;
}
+
+/*
+ * examine_operator_expression
+ * 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. When the expression matches this form,
+ * returns true, otherwise returns false.
+ *
+ * Optionally returns pointers to the extracted Var/Const nodes, when passed
+ * non-null pointers (varp, cstp and isgtp). The isgt flag specifies whether
+ * the Var node is on the left (false) or right (true) side of the operator.
+ */
+bool
+examine_opclause_expression(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp)
+{
+ Var *var;
+ Const *cst;
+ bool isgt;
+ Node *leftop,
+ *rightop;
+
+ /* enforced by statext_is_compatible_clause_internal */
+ Assert(list_length(expr->args) == 2);
+
+ leftop = linitial(expr->args);
+ rightop = lsecond(expr->args);
+
+ /* strip RelabelType from either side of the expression */
+ if (IsA(leftop, RelabelType))
+ leftop = (Node *) ((RelabelType *) leftop)->arg;
+
+ if (IsA(rightop, RelabelType))
+ rightop = (Node *) ((RelabelType *) rightop)->arg;
+
+ if (IsA(leftop, Var) && IsA(rightop, Const))
+ {
+ var = (Var *) leftop;
+ cst = (Const *) rightop;
+ isgt = false;
+ }
+ else if (IsA(leftop, Const) && IsA(rightop, Var))
+ {
+ var = (Var *) rightop;
+ cst = (Const *) leftop;
+ isgt = true;
+ }
+ else
+ return false;
+
+ /* return pointers to the extracted parts if requested */
+ 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 e62421dfa8..a708a8f674 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1561,36 +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;
/* get procedure computing operator selectivity */
RegProcedure oprrest = get_oprrest(expr->opno);
+ /* valid only after examine_opclause_expression returns true */
+ Var *var;
+ Const *cst;
+ bool isgt;
+
fmgr_info(get_opcode(expr->opno), &opproc);
- ok = (NumRelids(clause) == 1) &&
- (is_pseudo_constant_clause(lsecond(expr->args)) ||
- (varonleft = false,
- is_pseudo_constant_clause(linitial(expr->args))));
-
- if (ok)
+ /* extract the var and const from the expression */
+ if (examine_opclause_expression(expr, &var, &cst, &isgt))
{
- Var *var;
- Const *cst;
- bool isgt;
int idx;
- /* 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;
-
/* match the attribute to a dimension of the statistic */
idx = bms_member_index(keys, var->varattno);
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index 8fc541993f..c7f01d4edc 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -97,6 +97,8 @@ extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows,
TupleDesc tdesc, MultiSortSupport mss,
int numattrs, AttrNumber *attnums);
+extern bool examine_opclause_expression(OpExpr *expr, Var **varp,
+ Const **cstp, bool *isgtp);
extern Selectivity mcv_clauselist_selectivity(PlannerInfo *root,
StatisticExtInfo *stat,