Optimize constant MinMax expressions
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.
I don't think this needs any documentation and I didn't see where we
have any existing tests for eval_const_expressions so I didn't create
any either.
Thoughts?
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
optimize_constant_least_greatest.v01.patchtext/x-patch; name=optimize_constant_least_greatest.v01.patchDownload+1-0
Vik Fearing <vik.fearing@2ndquadrant.com> 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.
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.
regards, tom lane
On 30/12/2018 00:36, Tom Lane wrote:
Vik Fearing <vik.fearing@2ndquadrant.com> 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 smartsCan 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
Attachments:
optimize_constant_least_greatest.v02.patchtext/x-patch; name=optimize_constant_least_greatest.v02.patchDownload+4-0
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
On 30/12/2018 00:36, Tom Lane wrote:
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?
Pushed with a bit of wordsmithing on the comment.
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?
I'd been thinking that we might just add a similar comment there, but
on reflection that doesn't seem like the right thing, so I started a
separate thread about it.
regards, tom lane
On 30/12/2018 19:44, Tom Lane wrote:
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
On 30/12/2018 00:36, Tom Lane wrote:
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?
Pushed with a bit of wordsmithing on the comment.
Thanks! I've updated the commitfest entry to reflect that.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support