Re: speeding up planning with partitions

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, "Imai, Yoshikazu" <imai(dot)yoshikazu(at)jp(dot)fujitsu(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: speeding up planning with partitions
Date: 2019-02-02 13:52:37
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

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(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On Fri, 1 Feb 2019 at 23:01, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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:


> 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.


Attachment Content-Type Size
v19-0004-Do-not-lock-all-partitions-at-the-beginning.patch application/octet-stream 1.9 KB
v19-0002-Lazy-creation-of-RTEs-for-inheritance-children.patch application/octet-stream 93.9 KB
v19-0003-Teach-planner-to-only-process-unpruned-partition.patch application/octet-stream 6.9 KB
v19-0001-Overhaul-inheritance-update-delete-planning.patch application/octet-stream 74.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2019-02-02 13:53:42 Re: Able to do ALTER DEFAULT PRIVILEGES from a user who is not the owner
Previous Message Masahiko Sawada 2019-02-02 13:48:07 Re: [HACKERS] Block level parallel vacuum