Commit e8b6ae21 authored by Tomas Vondra's avatar Tomas Vondra

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
parent a4303a07
...@@ -796,21 +796,13 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, ...@@ -796,21 +796,13 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
RangeTblEntry *rte = root->simple_rte_array[relid]; RangeTblEntry *rte = root->simple_rte_array[relid];
OpExpr *expr = (OpExpr *) clause; OpExpr *expr = (OpExpr *) clause;
Var *var; Var *var;
bool varonleft = true;
bool ok;
/* Only expressions with two arguments are considered compatible. */ /* Only expressions with two arguments are considered compatible. */
if (list_length(expr->args) != 2) if (list_length(expr->args) != 2)
return false; return false;
/* see if it actually has the right shape (one Var, one Const) */ /* Check if the expression the right shape (one Var, one Const) */
ok = (NumRelids((Node *) expr) == 1) && if (!examine_opclause_expression(expr, &var, NULL, NULL))
(is_pseudo_constant_clause(lsecond(expr->args)) ||
(varonleft = false,
is_pseudo_constant_clause(linitial(expr->args))));
/* unsupported structure (two variables or so) */
if (!ok)
return false; return false;
/* /*
...@@ -850,8 +842,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, ...@@ -850,8 +842,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
!get_func_leakproof(get_opcode(expr->opno))) !get_func_leakproof(get_opcode(expr->opno)))
return false; return false;
var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
return statext_is_compatible_clause_internal(root, (Node *) var, return statext_is_compatible_clause_internal(root, (Node *) var,
relid, attnums); relid, attnums);
} }
...@@ -1196,3 +1186,65 @@ statext_clauselist_selectivity(PlannerInfo *root, List *clauses, int varRelid, ...@@ -1196,3 +1186,65 @@ statext_clauselist_selectivity(PlannerInfo *root, List *clauses, int varRelid,
return sel; 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;
}
...@@ -1561,35 +1561,22 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, ...@@ -1561,35 +1561,22 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
if (is_opclause(clause)) if (is_opclause(clause))
{ {
OpExpr *expr = (OpExpr *) clause; OpExpr *expr = (OpExpr *) clause;
bool varonleft = true;
bool ok;
FmgrInfo opproc; FmgrInfo opproc;
/* get procedure computing operator selectivity */ /* get procedure computing operator selectivity */
RegProcedure oprrest = get_oprrest(expr->opno); RegProcedure oprrest = get_oprrest(expr->opno);
fmgr_info(get_opcode(expr->opno), &opproc); /* valid only after examine_opclause_expression returns true */
ok = (NumRelids(clause) == 1) &&
(is_pseudo_constant_clause(lsecond(expr->args)) ||
(varonleft = false,
is_pseudo_constant_clause(linitial(expr->args))));
if (ok)
{
Var *var; Var *var;
Const *cst; Const *cst;
bool isgt; bool isgt;
int idx;
/* extract the var and const from the expression */ fmgr_info(get_opcode(expr->opno), &opproc);
var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
cst = (varonleft) ? lsecond(expr->args) : linitial(expr->args);
isgt = (!varonleft);
/* strip binary-compatible relabeling */ /* extract the var and const from the expression */
if (IsA(var, RelabelType)) if (examine_opclause_expression(expr, &var, &cst, &isgt))
var = (Var *) ((RelabelType *) var)->arg; {
int idx;
/* match the attribute to a dimension of the statistic */ /* match the attribute to a dimension of the statistic */
idx = bms_member_index(keys, var->varattno); idx = bms_member_index(keys, var->varattno);
......
...@@ -97,6 +97,8 @@ extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ...@@ -97,6 +97,8 @@ extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows,
TupleDesc tdesc, MultiSortSupport mss, TupleDesc tdesc, MultiSortSupport mss,
int numattrs, AttrNumber *attnums); int numattrs, AttrNumber *attnums);
extern bool examine_opclause_expression(OpExpr *expr, Var **varp,
Const **cstp, bool *isgtp);
extern Selectivity mcv_clauselist_selectivity(PlannerInfo *root, extern Selectivity mcv_clauselist_selectivity(PlannerInfo *root,
StatisticExtInfo *stat, StatisticExtInfo *stat,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment