RE: Small performance tweak to run-time partition pruning

Hi, David.
Thanks for the patch!

On Mon, Oct 8, 2018 at 1:00 AM, David Rowley wrote:
> I was looking at this again and I realised that we can completely skip
> the re-sequence of the subplan map when we're not going to perform any
> further pruning during execution.

I checked codes and I think so too.

Confirmation of my understanding and For more information to others:

The subplan map is used when we call ExecFindInitialMatchingSubPlans or 
ExecFindMatchingSubPlans.
ExecFindInitialMatchingSubPlans is called by ExecInit(Merge)Append.
ExecFindmatchingSubPlans is called by below fours which is executed after
ExecInit(Merge)Append and it is called when the 
as_valid_subplans(or ms_valid_subplans) is not NULL.

1. choose_next_subplan_locally(AppendState *node)
2. choose_next_subplan_for_leader(AppendState *node)
3. choose_next_subplan_for_worker(AppendState *node)
4. ExecMergeAppend(PlanState *pstate)

The as_valid_subplans(or ms_valid_subplans) is set in ExecInit(Merge)Append
if pruning during execution is not required.

That's why we can completely skip the re-sequence of the subplan map 
when we're not going to perform any further pruning during execution.


> I've attached an updated patch which skips the re-sequence work when
> doing that is not required for anything.

I saw the patch and it seems good to me about the codes.
I still couldn't check additional test cases in the patch.


BTW, when I looking the codes, I thought there is further improvements in
ExecInitAppend which is described below. 

        j = i = 0;
        firstvalid = nplans;
        foreach(lc, node->appendplans)
        {
                if (bms_is_member(i, validsubplans))
                {
                        Plan       *initNode = (Plan *) lfirst(lc);

                        /*
                         * Record the lowest appendplans index which is a valid partial
                         * plan.
                         */
                        if (i >= node->first_partial_plan && j < firstvalid)
                                firstvalid = j;

                        appendplanstates[j++] = ExecInitNode(initNode, estate, eflags);
                }
                i++;
        }

If number of valid subplans is few compared to node->appendplans, it is a waste to check
bms_is_member(i, validsubplans) for all node->appendplans.
Of course, node->appendplans is List struct and we have to loop it to find valid plan,
that we can't simply use while(bms_next_member(i, validsubplans)) loop.
I don't have any good idea for it now, but can we improve it?


--
Yoshikazu Imai