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

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index c56ed48270662b22659a97ee7384162b253f5f44..d2346cbe02e408b367a1611d85d9c53054b07ced 100644 (file)
--- 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 e62421dfa881230a72fd43593ecd3a162836f280..a708a8f6740c4a0b6d9dbf0bdee042b965f52fa8 100644 (file)
--- 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);
 
-                       fmgr_info(get_opcode(expr->opno), &opproc);
+                       /* valid only after examine_opclause_expression 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 (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 8fc541993fac9b6785dd4e1d42ca5af4af614b5e..c7f01d4edc7029b8bc7a0d53de758de911da3c66 100644 (file)
--- 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,