|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|
|Views:||Raw Message | Whole Thread | Download mbox|
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
> 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,
> subroot->all_baserels = bms_add_member(subroot->all_baserels,
> /* 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 &&
> 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.
|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|