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 <t...@sss.pgh.pa.us> wrote: >> Dmitry Dolgov <9erthali...@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