On 30/12/2018 00:36, Tom Lane wrote:
> Vik Fearing <[email protected]> writes:
>> I was working on a little thing where I needed to simulate BETWEEN
>> SYMMETRIC so naturally I used least() and greatest(). I was a little
>> surprised to see that my expressions were not folded into straight
>> constants and the estimates were way off as a consequence.
>
>> I came up with the attached patch to fix it, but it's so ridiculously
>> small that I fear I'm missing something.
>
> Well, the question this is begging is in the adjacent comment:
>
> * Generic handling for node types whose own processing is
> * known to be immutable, and for which we need no smarts
>
> Can we assume that the underlying datatype comparison function is
> immutable? I guess so, since we assume that in nearby code such as
> contain_mutable_functions_walker, but I don't think it should be done
> without at least a comment.
Adding a comment is easy enough. How is the attached?
> BTW, poking around for other code involving MinMaxExpr, I notice that
> contain_leaked_vars_walker is effectively assuming that all datatype
> comparison functions are leakproof, an assumption I find a bit debatable.
> Maybe it's all right, but again, it should certainly not have gone without
> a comment.
Surely this is out of scope for my patch?
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index f4446169f5..ee8ed870fa 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -3378,11 +3378,15 @@ eval_const_expressions_mutator(Node *node,
case T_ArrayRef:
case T_ArrayExpr:
case T_RowExpr:
+ case T_MinMaxExpr:
{
/*
* 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".
+ *
+ * For MinMaxExpr, we assume btree comparison functions are
+ * immutable. See comment in contain_mutable_functions_walker.
*/
/* Copy the node and const-simplify its arguments */
