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: Wed, 09 Jan 2019 20:20:50 +0900
Message-ID: <5C35D912.8040200@lab.ntt.co.jp> (raw)
In-Reply-To: <96720c99-ffa0-01ad-c594-0504c8eda708@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>
Amit-san,
(2019/01/09 9:30), Amit Langote wrote:
> (sorry about the repeated email, but my previous attempt failed due to
> trying to send to the -hackers and -performance lists at the same time, so
> trying again after removing -performance)
Thanks! (Actually, I also failed to send my post to those lists...)
> On 2019/01/08 20:07, Etsuro Fujita wrote:
>> (2018/12/07 20:14), Ashutosh Bapat wrote:
>>> On Fri, Dec 7, 2018 at 11:13 AM Ashutosh Bapat
>>> <ashutosh.bapat.oss@gmail.com<mailto:ashutosh.bapat.oss@gmail.com>> wrote:
>>
>>> Robert, Ashutosh, any comments on this? I'm unfamiliar with the
>>> partitionwise join code.
>>
>>> As the comment says it has to do with the equivalence classes being
>>> used during merge append. EC's are used to create pathkeys used for
>>> sorting. Creating a sort node which has column on the nullable side
>>> of an OUTER join will fail if it doesn't find corresponding
>>> equivalence class. You may not notice this if both the partitions
>>> being joined are pruned for some reason. Amit's idea to make
>>> partition-wise join code do this may work, but will add a similar
>>> overhead esp. in N-way partition-wise join once those equivalence
>>> classes are added.
>>
>>> I looked at the patch. The problem there is that for a given relation,
>>> we will add child ec member multiple times, as many times as the number
>>> of joins it participates in. We need to avoid that to keep ec_member
>>> list length in check.
>>
>> Amit-san, are you still working on this, perhaps as part of the
>> speeding-up-planning-with-partitions patch [1]?
>
> I had tried to continue working on it after PGConf.ASIA last month, but
> got distracted by something else.
>
> So, while the patch at [1] can take care of this issue as I also mentioned
> upthread, I was trying to come up with a solution that can be back-patched
> to PG 11. The patch I posted above is one such solution and as Ashutosh
> points out it's perhaps not the best, because it can result in potentially
> creating many copies of the same child EC member if we do it in joinrel.c,
> as the patch proposes. I will try to respond to the concerns he raised in
> the next week if possible.
Thanks for working on this!
I like your patch in general. I think one way to address Ashutosh's
concerns would be to use the consider_partitionwise_join flag:
originally, that was introduced for partitioned relations to show that
they can be partitionwise-joined, but I think that flag could also be
used for non-partitioned relations to show that they have been set up
properly for partitionwise-joins, and I think by checking that flag we
could avoid creating those copies for child dummy rels in
try_partitionwise_join. Please find attached an updated version of the
patch. I modified your version so that building tlists for child dummy
rels are also postponed until after they actually participate in
partitionwise-joins, to avoid that possibly-useless work as well. I
haven't done any performance tests yet though.
Best regards,
Etsuro Fujita
Attachments:
[text/x-patch] 0001-Add-child-EC-members-for-only-the-non-dummy-children-efujita.patch (7.1K, ../5C35D912.8040200@lab.ntt.co.jp/2-0001-Add-child-EC-members-for-only-the-non-dummy-children-efujita.patch)
download | inline diff:diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.cindex 256fe16..feccc60 100644--- a/src/backend/optimizer/path/allpaths.c+++ b/src/backend/optimizer/path/allpaths.c@@ -1017,38 +1017,6 @@ 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
@@ -1186,11 +1154,36 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
continue;
}
- /* CE failed, so finish copying/modifying join quals. */+ /*+ * 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,8 +1197,7 @@ 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)+ if (rel->consider_partitionwise_join)
childrel->consider_partitionwise_join = true;
/*
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.cindex 67b9ca8..6645c5a 100644--- a/src/backend/optimizer/path/joinrels.c+++ b/src/backend/optimizer/path/joinrels.c@@ -44,6 +44,8 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
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,6 +1314,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
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,6 +1380,27 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
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);
@@ -1418,6 +1443,28 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
}
/*
+ * 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: <5C35D912.8040200@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