From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: alvherre@2ndquadrant.com, pryzby@telsasoft.com, sanyo.moura@tatic.net, Jeff Janes <jeff.janes@gmail.com>
Cc: david.rowley@2ndquadrant.com, Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Query with high planning time at version 11.1 compared versions 10.5 and 11.0
Date: Fri, 11 Jan 2019 21:50:19 +0900
Message-ID: <5C38910B.8010901@lab.ntt.co.jp> (raw)
In-Reply-To: <dd1de042-dca5-cfb9-bc66-2e39020fbf6a@lab.ntt.co.jp>
References: <8db4eca8-6e76-70ff-f934-e0228a1cc298@lab.ntt.co.jp>
<20181206075726.pig32e6thltb56hr@alvherre.pgsql>
<CAExHW5umxU5FOoT8FBcqp-GWKMtZhnuEy7U6bNH8QfzvYwUgEQ@mail.gmail.com>
<CAExHW5uFwP_1N7RV6s8piQBvygGH+rppS60nFNHB9DKEM04ShA@mail.gmail.com>
<5C34846E.9090600@lab.ntt.co.jp>
<96720c99-ffa0-01ad-c594-0504c8eda708@lab.ntt.co.jp>
<5C35D912.8040200@lab.ntt.co.jp>
<7354b5bf-5932-4c0c-81ee-d9550de18995@lab.ntt.co.jp>
<5C36E105.2020902@lab.ntt.co.jp>
<dd1de042-dca5-cfb9-bc66-2e39020fbf6a@lab.ntt.co.jp>
(2019/01/11 13:46), Amit Langote wrote:
> On 2019/01/10 15:07, Etsuro Fujita wrote:
>> (2019/01/10 10:41), Amit Langote wrote:
>>> That's a loaded meaning and abusing it to mean something else can be
>>> challenged, but we can live with that if properly documented. Speaking of
>>> which:
>>>
>>> /* used by partitionwise joins: */
>>> bool consider_partitionwise_join; /* consider
>>> partitionwise join
>>> * paths? (if partitioned
>>> rel) */
>>>
>>> Maybe, mention here how it will be abused in back-branches for
>>> non-partitioned relations?
>>
>> Will do.
>
> Thank you.
I know we don't yet reach a consensus on what to do in details to
address this issue, but for the above, how about adding comments like
this to set_append_rel_size(), instead of the header file:
/*
* If we consider partitionwise joins with the parent rel, do
the same
* for partitioned child rels.
*
* Note: here we abuse the consider_partitionwise_join flag for
child
* rels that are not partitioned, to tell try_partitionwise_join()
* that their targetlists and EC entries have been generated.
*/
if (rel->consider_partitionwise_join)
childrel->consider_partitionwise_join = true;
ISTM that that would be more clearer than the header file.
Updated patch attached, which also updated other comments a little bit.
Best regards,
Etsuro Fujita
Attachments:
[text/x-patch] 0001-Add-child-EC-members-for-only-the-non-dummy-children-efujita-v2.patch (8.6K, ../5C38910B.8010901@lab.ntt.co.jp/2-0001-Add-child-EC-members-for-only-the-non-dummy-children-efujita-v2.patch)
download | inline diff:
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
***************
*** 1018,1059 **** set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
Assert(childrel->reloptkind == RELOPT_OTHER_MEMBER_REL);
/*
! * Copy/Modify targetlist. Even if this child is deemed empty, we need
! * its targetlist in case it falls on nullable side in a child-join
! * because of partitionwise join.
! *
! * NB: the resulting childrel->reltarget->exprs may contain arbitrary
! * expressions, which otherwise would not occur in a rel's targetlist.
! * Code that might be looking at an appendrel child must cope with
! * such. (Normally, a rel's targetlist would only include Vars and
! * PlaceHolderVars.) XXX we do not bother to update the cost or width
! * fields of childrel->reltarget; not clear if that would be useful.
! */
! childrel->reltarget->exprs = (List *)
! adjust_appendrel_attrs(root,
! (Node *) rel->reltarget->exprs,
! 1, &appinfo);
!
! /*
! * We have to make child entries in the EquivalenceClass data
! * structures as well. This is needed either if the parent
! * participates in some eclass joins (because we will want to consider
! * inner-indexscan joins on the individual children) or if the parent
! * has useful pathkeys (because we should try to build MergeAppend
! * paths that produce those sort orderings). Even if this child is
! * deemed dummy, it may fall on nullable side in a child-join, which
! * in turn may participate in a MergeAppend, where we will need the
! * EquivalenceClass data structures.
! */
! if (rel->has_eclass_joins || has_useful_pathkeys(root, rel))
! add_child_rel_equivalences(root, appinfo, rel, childrel);
! childrel->has_eclass_joins = rel->has_eclass_joins;
!
! /*
! * We have to copy the parent's quals to the child, with appropriate
! * substitution of variables. However, only the baserestrictinfo
! * quals are needed before we can check for constraint exclusion; so
! * do that first and then check to see if we can disregard this child.
*
* The child rel's targetlist might contain non-Var expressions, which
* means that substitution into the quals could produce opportunities
--- 1018,1028 ----
Assert(childrel->reloptkind == RELOPT_OTHER_MEMBER_REL);
/*
! * We have to copy the parent's targetlist and quals to the child,
! * with appropriate substitution of variables. However, only the
! * baserestrictinfo quals are needed before we can check for
! * constraint exclusion; so do that first and then check to see if we
! * can disregard this child.
*
* The child rel's targetlist might contain non-Var expressions, which
* means that substitution into the quals could produce opportunities
***************
*** 1187,1197 **** set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
continue;
}
! /* CE failed, so finish copying/modifying join quals. */
childrel->joininfo = (List *)
adjust_appendrel_attrs(root,
(Node *) rel->joininfo,
1, &appinfo);
/*
* Note: we could compute appropriate attr_needed data for the child's
--- 1156,1191 ----
continue;
}
! /*
! * CE failed, so finish copying/modifying targetlist and join quals.
! *
! * NB: the resulting childrel->reltarget->exprs may contain arbitrary
! * expressions, which otherwise would not occur in a rel's targetlist.
! * Code that might be looking at an appendrel child must cope with
! * such. (Normally, a rel's targetlist would only include Vars and
! * PlaceHolderVars.) XXX we do not bother to update the cost or width
! * fields of childrel->reltarget; not clear if that would be useful.
! */
childrel->joininfo = (List *)
adjust_appendrel_attrs(root,
(Node *) rel->joininfo,
1, &appinfo);
+ childrel->reltarget->exprs = (List *)+ adjust_appendrel_attrs(root,+ (Node *) rel->reltarget->exprs,+ 1, &appinfo);+ + /*+ * We have to make child entries in the EquivalenceClass data+ * structures as well. This is needed either if the parent+ * participates in some eclass joins (because we will want to consider+ * inner-indexscan joins on the individual children) or if the parent+ * has useful pathkeys (because we should try to build MergeAppend+ * paths that produce those sort orderings).+ */+ if (rel->has_eclass_joins || has_useful_pathkeys(root, rel))+ add_child_rel_equivalences(root, appinfo, rel, childrel);+ childrel->has_eclass_joins = rel->has_eclass_joins;
/*
* Note: we could compute appropriate attr_needed data for the child's
***************
*** 1204,1212 **** set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
/*
* If we consider partitionwise joins with the parent rel, do the same
* for partitioned child rels.
*/
! if (rel->consider_partitionwise_join &&
! childRTE->relkind == RELKIND_PARTITIONED_TABLE)
childrel->consider_partitionwise_join = true;
/*
--- 1198,1209 ----
/*
* If we consider partitionwise joins with the parent rel, do the same
* for partitioned child rels.
+ *+ * Note: here we abuse the consider_partitionwise_join flag for child+ * rels that are not partitioned, to tell try_partitionwise_join()+ * that their targetlists and EC entries have been generated.
*/
! if (rel->consider_partitionwise_join)
childrel->consider_partitionwise_join = true;
/*
*** a/src/backend/optimizer/path/joinrels.c
--- b/src/backend/optimizer/path/joinrels.c
***************
*** 44,49 **** static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
--- 44,51 ----
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
List *parent_restrictlist);
+ static void update_child_rel_info(PlannerInfo *root,+ RelOptInfo *rel, RelOptInfo *childrel);
static int match_expr_to_partition_keys(Expr *expr, RelOptInfo *rel,
bool strict_op);
***************
*** 1312,1317 **** try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
--- 1314,1321 ----
RelOptInfo *joinrel, SpecialJoinInfo *parent_sjinfo,
List *parent_restrictlist)
{
+ bool rel1_is_simple = IS_SIMPLE_REL(rel1);+ bool rel2_is_simple = IS_SIMPLE_REL(rel2);
int nparts;
int cnt_parts;
***************
*** 1376,1381 **** try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
--- 1380,1406 ----
AppendRelInfo **appinfos;
int nappinfos;
+ /*+ * If a child table has consider_partitionwise_join=false, it means+ * that it's a dummy relation for which we skipped setting up tlist+ * expressions and adding EC members in set_append_rel_size(), so do+ * that now for use later.+ */+ if (rel1_is_simple && !child_rel1->consider_partitionwise_join)+ {+ Assert(child_rel1->reloptkind == RELOPT_OTHER_MEMBER_REL);+ Assert(IS_DUMMY_REL(child_rel1));+ update_child_rel_info(root, rel1, child_rel1);+ child_rel1->consider_partitionwise_join = true;+ }+ if (rel2_is_simple && !child_rel2->consider_partitionwise_join)+ {+ Assert(child_rel2->reloptkind == RELOPT_OTHER_MEMBER_REL);+ Assert(IS_DUMMY_REL(child_rel2));+ update_child_rel_info(root, rel2, child_rel2);+ child_rel2->consider_partitionwise_join = true;+ }+
/* We should never try to join two overlapping sets of rels. */
Assert(!bms_overlap(child_rel1->relids, child_rel2->relids));
child_joinrelids = bms_union(child_rel1->relids, child_rel2->relids);
***************
*** 1417,1422 **** try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
--- 1442,1469 ----
}
}
+ /*+ * Set up tlist expressions for the childrel, and add EC members referencing+ * the childrel.+ */+ static void+ update_child_rel_info(PlannerInfo *root,+ RelOptInfo *rel, RelOptInfo *childrel)+ {+ AppendRelInfo *appinfo = root->append_rel_array[childrel->relid];+ + /* Make child tlist expressions */+ childrel->reltarget->exprs = (List *)+ adjust_appendrel_attrs(root,+ (Node *) rel->reltarget->exprs,+ 1, &appinfo);+ + /* Make child entries in the EquivalenceClass as well */+ if (rel->has_eclass_joins || has_useful_pathkeys(root, rel))+ add_child_rel_equivalences(root, appinfo, rel, childrel);+ childrel->has_eclass_joins = rel->has_eclass_joins;+ }+
/*
* Returns true if there exists an equi-join condition for each pair of
* partition keys from given relations being joined.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-performance@postgresql.org
Cc: fujita.etsuro@lab.ntt.co.jp, Langote_Amit_f8@lab.ntt.co.jp, ashutosh.bapat.oss@gmail.com, jeff.janes@gmail.com, robertmhaas@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: Query with high planning time at version 11.1 compared versions 10.5 and 11.0
In-Reply-To: <5C38910B.8010901@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox