Re: speeding up planning with partitions

Thanks David for the patch that Alvaro committed yesterday.  Agree
that it's a good idea.

On Fri, Feb 1, 2019 at 3:11 PM David Rowley
<david.rowley@2ndquadrant.com> wrote:
> On Fri, 1 Feb 2019 at 23:01, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Please rebase.
>
> I had a short list of other things I noticed when making a partial
> pass over the patch again.
>
> I may as well send these now if there's a new version on the way:

Thanks.

> 1. I think it's okay to convert the following:
>
> /*
> * Adjust all_baserels to replace the original target relation with the
> * child target relation.  Copy it before modifying though.
> */
> subroot->all_baserels = bms_copy(root->all_baserels);
> subroot->all_baserels = bms_del_member(subroot->all_baserels,
>    root->parse->resultRelation);
> subroot->all_baserels = bms_add_member(subroot->all_baserels,
>    subroot->parse->resultRelation);
>
> into:
> /* Adjust all_baserels */
> subroot->all_baserels = adjust_child_relids(root->all_baserels, 1, &appinfo);

Makes sense, done.

> 2. Any reason to do:
>
> /*
> * Generate access paths for the entire join tree.
> *
> * For UPDATE/DELETE on an inheritance parent, join paths should be
> * generated for each child result rel separately.
> */
> if (root->parse->resultRelation &&
> root->simple_rte_array[root->parse->resultRelation]->inh)
>
> instead of just checking: if (root->inherited_update)

Good reminder, done.

> 3. This seems like useless code in set_inherit_target_rel_sizes().
>
> /*
> * If parallelism is allowable for this query in general, see whether
> * it's allowable for this childrel in particular.  For consistency,
> * do this before calling set_rel_size() for the child.
> */
> if (root->glob->parallelModeOK)
> set_rel_consider_parallel(subroot, childrel, childRTE);
>
>
> parallelModeOK is only ever set for SELECT.  Likely it's fine just to
> replace these with:
>
> +               /* We don't consider parallel paths for UPDATE/DELETE
> statements */
> +               childrel->consider_parallel = false;
>
> or perhaps it's fine to leave it out since build_simple_rel() sets it to false.

OK, removed that code.

Attached updated patches.  It took me a bit longer than expected to
rebase the patches as I hit a mysterious bug that I couldn't pinpoint
until this afternoon.

One big change is related to how ECs are transferred to child
PlannerInfos.  As David suggested upthread, I created a block in
adjust_appendrel_attrs_mutator that creates a translated copy of a
given EC containing wherein the parent expression in the original
ec_members list is replaced by the corresponding child expression.
With that in place, we no longer need the changes to
add_child_rel_equivalences().  Instead there's just:
subroot->eq_classes = adjust_appendrel_attrs(root, root->eq_classes,
...), just as David described upthread.

Thanks,
Amit