Hi Dmitry,
Thanks for creating the patch. I looked at it and have some comments.
On 2018/06/04 22:30, Dmitry Dolgov wrote:
>> On 3 June 2018 at 19:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Dmitry Dolgov <9erthalion6@gmail.com> writes:
>>> Just to clarify for myself, for evaluating any stable function here would it be
>>> enough to handle all function-like expressions (FuncExpr / OpExpr /
>>> DistinctExpr / NullIfExpr) and check a corresponding function for provolatile,
>>> like in the attached patch?
>>
>> I think the entire approach is wrong here. Rather than concerning
>> yourself with Params, or any other specific expression type, you
>> should be using !contain_volatile_functions() to decide whether
>> an expression is run-time-constant. If it is, use the regular
>> expression evaluation machinery to extract the value.
>
> Yes, it makes sense. Then, to my understanding, the attached code is close to
> what was described above (although I'm not sure about the Const part).
This:
@@ -2727,6 +2730,13 @@ pull_partkey_params(PartitionPruneInfo *pinfo, List
*steps)
}
gotone = true;
}
+ else if (!IsA(expr, Const))
+ {
+ Param *param = (Param *) expr;
+ pinfo->execparams = bms_add_member(pinfo->execparams,
+ param->paramid);
+ gotone = true;
+ }
doesn't look quite right. What says expr is really a Param? The patch
appears to work because, by setting pinfo->execparams to *something*, it
triggers execution-time pruning to run; its contents aren't necessarily
used during execution pruning. In fact, it would've crashed if the
execution-time pruning code had required execparams to contain *valid*
param id, but currently it doesn't.
What I think we'd need to do to make this work is to make execution-time
pruning be invoked even if there aren't any Params involved. IOW, let's
try to teach make_partition_pruneinfo that it can go ahead also in the
cases where there are expressions being compared with the partition key
that contain (only) stable functions. Then, go and fix the
execution-pruning code to not *always* expect there to be Params to prune
with.
Maybe, David (added to cc) has something to say about all this, especially
whether he considers this a feature and not a bug fix.
Thanks,
Amit