From: Tom Lane <tgl@sss.pgh.pa.us>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: eval_const_expresisions and ScalarArrayOpExpr
Date: Sun, 01 Oct 2017 18:52:12 -0400
Message-ID: <15981.1506898332@sss.pgh.pa.us> (raw)
In-Reply-To: <507.1505222976@sss.pgh.pa.us>
References: <3be3b82c-e29c-b674-2163-bf47d98817b1@iki.fi>
<23186.1494516060@sss.pgh.pa.us>
<4b14a9a5-01c2-4042-3ec9-cf27149302a5@iki.fi>
<22029.1494605094@sss.pgh.pa.us>
<507.1505222976@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
I wrote:
> This patch no longer applies cleanly on HEAD, so here's a rebased version
> (no substantive changes). As before, I think the most useful review task
> would be to quantify whether it makes planning noticeably slower.
Rebased again (over the arrays-of-domains patch).
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:better-eval-const-expressions-3.patch (text/x-diff, 15.1K) downloaddiff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.cindex 7961362..c3fab75 100644
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** static List *find_nonnullable_vars_walke
*** 115,120 ****
--- 115,123 ----
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
static Node *eval_const_expressions_mutator(Node *node,
eval_const_expressions_context *context);
+ static bool contain_non_const_walker(Node *node, void *context);+ static bool ece_function_is_safe(Oid funcid,+ eval_const_expressions_context *context);
static List *simplify_or_arguments(List *args,
eval_const_expressions_context *context,
bool *haveNull, bool *forceTrue);
*************** estimate_expression_value(PlannerInfo *r
*** 2472,2477 ****
--- 2475,2511 ----
return eval_const_expressions_mutator(node, &context);
}
+ /*+ * The generic case in eval_const_expressions_mutator is to recurse using+ * expression_tree_mutator, which will copy the given node unchanged but+ * const-simplify its arguments (if any) as far as possible. If the node+ * itself does immutable processing, and each of its arguments were reduced+ * to a Const, we can then reduce it to a Const using evaluate_expr. (Some+ * node types need more complicated logic; for example, a CASE expression+ * might be reducible to a constant even if not all its subtrees are.)+ */+ #define ece_generic_processing(node) \+ expression_tree_mutator((Node *) (node), eval_const_expressions_mutator, \+ (void *) context)+ + /*+ * Check whether all arguments of the given node were reduced to Consts.+ * By going directly to expression_tree_walker, contain_non_const_walker+ * is not applied to the node itself, only to its children.+ */+ #define ece_all_arguments_const(node) \+ (!expression_tree_walker((Node *) (node), contain_non_const_walker, NULL))+ + /* Generic macro for applying evaluate_expr */+ #define ece_evaluate_expr(node) \+ ((Node *) evaluate_expr((Expr *) (node), \+ exprType((Node *) (node)), \+ exprTypmod((Node *) (node)), \+ exprCollation((Node *) (node))))+ + /*+ * Recursive guts of eval_const_expressions/estimate_expression_value+ */
static Node *
eval_const_expressions_mutator(Node *node,
eval_const_expressions_context *context)
*************** eval_const_expressions_mutator(Node *nod
*** 2787,2792 ****
--- 2821,2845 ----
newexpr->location = expr->location;
return (Node *) newexpr;
}
+ case T_ScalarArrayOpExpr:+ {+ ScalarArrayOpExpr *saop;+ + /* Copy the node and const-simplify its arguments */+ saop = (ScalarArrayOpExpr *) ece_generic_processing(node);+ + /* Make sure we know underlying function */+ set_sa_opfuncid(saop);+ + /*+ * If all arguments are Consts, and it's a safe function, we+ * can fold to a constant+ */+ if (ece_all_arguments_const(saop) &&+ ece_function_is_safe(saop->opfuncid, context))+ return ece_evaluate_expr(saop);+ return (Node *) saop;+ }
case T_BoolExpr:
{
BoolExpr *expr = (BoolExpr *) node;
*************** eval_const_expressions_mutator(Node *nod
*** 3011,3057 ****
}
case T_ArrayCoerceExpr:
{
! ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
! Expr *arg;
! Expr *elemexpr;
! ArrayCoerceExpr *newexpr;
!
! /*
! * Reduce constants in the ArrayCoerceExpr's argument and
! * per-element expressions, then build a new ArrayCoerceExpr.
! */
! arg = (Expr *) eval_const_expressions_mutator((Node *) expr->arg,
! context);
! elemexpr = (Expr *) eval_const_expressions_mutator((Node *) expr->elemexpr,
! context);
! newexpr = makeNode(ArrayCoerceExpr);
! newexpr->arg = arg;
! newexpr->elemexpr = elemexpr;
! newexpr->resulttype = expr->resulttype;
! newexpr->resulttypmod = expr->resulttypmod;
! newexpr->resultcollid = expr->resultcollid;
! newexpr->coerceformat = expr->coerceformat;
! newexpr->location = expr->location;
/*
! * If constant argument and per-element expression is
* immutable, we can simplify the whole thing to a constant.
* Exception: although contain_mutable_functions considers
* CoerceToDomain immutable for historical reasons, let's not
* do so here; this ensures coercion to an array-over-domain
* does not apply the domain's constraints until runtime.
*/
! if (arg && IsA(arg, Const) &&
! elemexpr && !IsA(elemexpr, CoerceToDomain) &&
! !contain_mutable_functions((Node *) elemexpr))
! return (Node *) evaluate_expr((Expr *) newexpr,
! newexpr->resulttype,
! newexpr->resulttypmod,
! newexpr->resultcollid);
!
! /* Else we must return the partially-simplified node */
! return (Node *) newexpr;
}
case T_CollateExpr:
{
--- 3064,3087 ----
}
case T_ArrayCoerceExpr:
{
! ArrayCoerceExpr *ac;
! /* Copy the node and const-simplify its arguments */
! ac = (ArrayCoerceExpr *) ece_generic_processing(node);
/*
! * If constant argument and the per-element expression is
* immutable, we can simplify the whole thing to a constant.
* Exception: although contain_mutable_functions considers
* CoerceToDomain immutable for historical reasons, let's not
* do so here; this ensures coercion to an array-over-domain
* does not apply the domain's constraints until runtime.
*/
! if (ac->arg && IsA(ac->arg, Const) &&
! ac->elemexpr && !IsA(ac->elemexpr, CoerceToDomain) &&
! !contain_mutable_functions((Node *) ac->elemexpr))
! return ece_evaluate_expr(ac);
! return (Node *) ac;
}
case T_CollateExpr:
{
*************** eval_const_expressions_mutator(Node *nod
*** 3243,3283 ****
else
return copyObject(node);
}
case T_ArrayExpr:
{
! ArrayExpr *arrayexpr = (ArrayExpr *) node;
! ArrayExpr *newarray;
! bool all_const = true;
! List *newelems;
! ListCell *element;
!
! newelems = NIL;
! foreach(element, arrayexpr->elements)
! {
! Node *e;
!
! e = eval_const_expressions_mutator((Node *) lfirst(element),
! context);
! if (!IsA(e, Const))
! all_const = false;
! newelems = lappend(newelems, e);
! }
!
! newarray = makeNode(ArrayExpr);
! newarray->array_typeid = arrayexpr->array_typeid;
! newarray->array_collid = arrayexpr->array_collid;
! newarray->element_typeid = arrayexpr->element_typeid;
! newarray->elements = newelems;
! newarray->multidims = arrayexpr->multidims;
! newarray->location = arrayexpr->location;
!
! if (all_const)
! return (Node *) evaluate_expr((Expr *) newarray,
! newarray->array_typeid,
! exprTypmod(node),
! newarray->array_collid);
! return (Node *) newarray;
}
case T_CoalesceExpr:
{
--- 3273,3295 ----
else
return copyObject(node);
}
+ case T_ArrayRef:
case T_ArrayExpr:
+ case T_RowExpr:+ case T_BooleanTest:
{
! /*
! * Generic handling for node types whose own processing is
! * known to be immutable, and for which we need no smarts
! * beyond "simplify if all inputs are constants".
! */
! /* Copy the node and const-simplify its arguments */
! node = ece_generic_processing(node);
! /* If all arguments are Consts, we can fold to a constant */
! if (ece_all_arguments_const(node))
! return ece_evaluate_expr(node);
! return node;
}
case T_CoalesceExpr:
{
*************** eval_const_expressions_mutator(Node *nod
*** 3354,3360 ****
* simple Var. (This case won't be generated directly by the
* parser, because ParseComplexProjection short-circuits it.
* But it can arise while simplifying functions.) Also, we
! * can optimize field selection from a RowExpr construct.
*
* However, replacing a whole-row Var in this way has a
* pitfall: if we've already built the rel targetlist for the
--- 3366,3373 ----
* simple Var. (This case won't be generated directly by the
* parser, because ParseComplexProjection short-circuits it.
* But it can arise while simplifying functions.) Also, we
! * can optimize field selection from a RowExpr construct, or
! * of course from a constant.
*
* However, replacing a whole-row Var in this way has a
* pitfall: if we've already built the rel targetlist for the
*************** eval_const_expressions_mutator(Node *nod
*** 3369,3374 ****
--- 3382,3389 ----
* We must also check that the declared type of the field is
* still the same as when the FieldSelect was created --- this
* can change if someone did ALTER COLUMN TYPE on the rowtype.
+ * If it isn't, we skip the optimization; the case will+ * probably fail at runtime, but that's not our problem here.
*/
FieldSelect *fselect = (FieldSelect *) node;
FieldSelect *newfselect;
*************** eval_const_expressions_mutator(Node *nod
*** 3419,3424 ****
--- 3434,3450 ----
newfselect->resulttype = fselect->resulttype;
newfselect->resulttypmod = fselect->resulttypmod;
newfselect->resultcollid = fselect->resultcollid;
+ if (arg && IsA(arg, Const))+ {+ Const *con = (Const *) arg;+ + if (rowtype_field_matches(con->consttype,+ newfselect->fieldnum,+ newfselect->resulttype,+ newfselect->resulttypmod,+ newfselect->resultcollid))+ return ece_evaluate_expr(newfselect);+ }
return (Node *) newfselect;
}
case T_NullTest:
*************** eval_const_expressions_mutator(Node *nod
*** 3512,3570 ****
newntest->location = ntest->location;
return (Node *) newntest;
}
- case T_BooleanTest:- {- BooleanTest *btest = (BooleanTest *) node;- BooleanTest *newbtest;- Node *arg;- - arg = eval_const_expressions_mutator((Node *) btest->arg,- context);- if (arg && IsA(arg, Const))- {- Const *carg = (Const *) arg;- bool result;- - switch (btest->booltesttype)- {- case IS_TRUE:- result = (!carg->constisnull &&- DatumGetBool(carg->constvalue));- break;- case IS_NOT_TRUE:- result = (carg->constisnull ||- !DatumGetBool(carg->constvalue));- break;- case IS_FALSE:- result = (!carg->constisnull &&- !DatumGetBool(carg->constvalue));- break;- case IS_NOT_FALSE:- result = (carg->constisnull ||- DatumGetBool(carg->constvalue));- break;- case IS_UNKNOWN:- result = carg->constisnull;- break;- case IS_NOT_UNKNOWN:- result = !carg->constisnull;- break;- default:- elog(ERROR, "unrecognized booltesttype: %d",- (int) btest->booltesttype);- result = false; /* keep compiler quiet */- break;- }- - return makeBoolConst(result, false);- }- - newbtest = makeNode(BooleanTest);- newbtest->arg = (Expr *) arg;- newbtest->booltesttype = btest->booltesttype;- newbtest->location = btest->location;- return (Node *) newbtest;- }
case T_PlaceHolderVar:
/*
--- 3538,3543 ----
*************** eval_const_expressions_mutator(Node *nod
*** 3587,3600 ****
}
/*
! * For any node type not handled above, we recurse using
! * expression_tree_mutator, which will copy the node unchanged but try to
! * simplify its arguments (if any) using this routine. For example: we
! * cannot eliminate an ArrayRef node, but we might be able to simplify
! * constant expressions in its subscripts.
*/
! return expression_tree_mutator(node, eval_const_expressions_mutator,
! (void *) context);
}
/*
--- 3560,3616 ----
}
/*
! * For any node type not handled above, copy the node unchanged but
! * const-simplify its subexpressions. This is the correct thing for node
! * types whose behavior might change between planning and execution, such
! * as CoerceToDomain. It's also a safe default for new node types not
! * known to this routine.
*/
! return ece_generic_processing(node);
! }
!
! /*
! * Subroutine for eval_const_expressions: check for non-Const nodes.
! *
! * We can abort recursion immediately on finding a non-Const node. This is
! * critical for performance, else eval_const_expressions_mutator would take
! * O(N^2) time on non-simplifiable trees. However, we do need to descend
! * into List nodes since expression_tree_walker sometimes invokes the walker
! * function directly on List subtrees.
! */
! static bool
! contain_non_const_walker(Node *node, void *context)
! {
! if (node == NULL)
! return false;
! if (IsA(node, Const))
! return false;
! if (IsA(node, List))
! return expression_tree_walker(node, contain_non_const_walker, context);
! /* Otherwise, abort the tree traversal and return true */
! return true;
! }
!
! /*
! * Subroutine for eval_const_expressions: check if a function is OK to evaluate
! */
! static bool
! ece_function_is_safe(Oid funcid, eval_const_expressions_context *context)
! {
! char provolatile = func_volatile(funcid);
!
! /*
! * Ordinarily we are only allowed to simplify immutable functions. But for
! * purposes of estimation, we consider it okay to simplify functions that
! * are merely stable; the risk that the result might change from planning
! * time to execution time is worth taking in preference to not being able
! * to estimate the value at all.
! */
! if (provolatile == PROVOLATILE_IMMUTABLE)
! return true;
! if (context->estimate && provolatile == PROVOLATILE_STABLE)
! return true;
! return false;
}
/*
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.outindex 43b36f6..a4bac8e 100644
*** a/src/test/regress/expected/rowtypes.out
--- b/src/test/regress/expected/rowtypes.out
*************** ERROR: cannot compare dissimilar column
*** 307,316 ****
explain (costs off)
select * from int8_tbl i8
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
! QUERY PLAN
! -----------------------------------------------------------------------------------------------------------------
Seq Scan on int8_tbl i8
! Filter: (i8.* = ANY (ARRAY[ROW('123'::bigint, '456'::bigint)::int8_tbl, '(4567890123456789,123)'::int8_tbl]))
(2 rows)
select * from int8_tbl i8
--- 307,316 ----
explain (costs off)
select * from int8_tbl i8
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
! QUERY PLAN
! -------------------------------------------------------------------------------
Seq Scan on int8_tbl i8
! Filter: (i8.* = ANY ('{"(123,456)","(4567890123456789,123)"}'::int8_tbl[]))
(2 rows)
select * from int8_tbl i8
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, hlinnaka@iki.fi
Subject: Re: eval_const_expresisions and ScalarArrayOpExpr
In-Reply-To: <15981.1506898332@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox