(2018/06/22 22:54), Ashutosh Bapat wrote:
I have started reviewing the patch.
Thanks for the review!
+ if (enable_partitionwise_join&& rel->top_parent_is_partitioned)
+ {
+ build_childrel_tlist(root, rel, childrel, 1,&appinfo);
+ }
Why do we need rel->top_parent_is_partitioned? If a relation is
partitioned (if (rel->part_scheme), it's either the top parent or is
partition of some other partitioned table. In either case this
condition will be true.
This would be needed to avoid unnecessarily applying
build_childrel_tlist to child rels of a partitioned table for which we
don't consider partitionwise join. Consider:
postgres=# create table lpt (c1 int, c2 text) partition by list (c1);
CREATE TABLE
postgres=# create table lpt_p1 partition of lpt for values in (1);
CREATE TABLE
postgres=# create table lpt_p2 (c1 int check (c1 in (2)), c2 text);
CREATE TABLE
postgres=# create table test (c1 int, c2 text);
CREATE TABLE
postgres=# explain verbose select * from (select * from lpt union all
select * from lpt_p2) ss(c1, c2) inner join test on (ss.c1 = test.c1);
QUERY PLAN
--------------------------------------------------------------------------------
----
Merge Join (cost=289.92..538.20 rows=16129 width=72)
Output: lpt_p1.c1, lpt_p1.c2, test.c1, test.c2
Merge Cond: (test.c1 = lpt_p1.c1)
-> Sort (cost=88.17..91.35 rows=1270 width=36)
Output: test.c1, test.c2
Sort Key: test.c1
-> Seq Scan on public.test (cost=0.00..22.70 rows=1270 width=36)
Output: test.c1, test.c2
-> Sort (cost=201.74..208.09 rows=2540 width=36)
Output: lpt_p1.c1, lpt_p1.c2
Sort Key: lpt_p1.c1
-> Append (cost=0.00..58.10 rows=2540 width=36)
-> Seq Scan on public.lpt_p1 (cost=0.00..22.70
rows=1270 width=
36)
Output: lpt_p1.c1, lpt_p1.c2
-> Seq Scan on public.lpt_p2 (cost=0.00..22.70
rows=1270 width=
36)
Output: lpt_p2.c1, lpt_p2.c2
(16 rows)
In this example, the top parent is not a partitioned table and we don't
need to consider partitionwise join for that, so we don't need to apply
that to the child rel lpt_p1 of the partitioned table lpt (and the table
lpt_p2).
+ /* No work if the child plan is an Append or MergeAppend */
+ if (IsA(subplan, Append) || IsA(subplan, MergeAppend))
+ return;
Why? Probably it's related to the answer to the first question, But I
don't see the connection. Given that partition-wise join will be
applicable level by level, we need to recurse in
adjust_subplan_tlist().
I don't think so; I think if the child plan is an Append or MergeAppend,
the tlist for each subplan of the Append or MergeAppend would have
already been adjusted while create_plan_recurse before we are called here.
+ /* The child plan should be able to do projection */
+ Assert(is_projection_capable_plan(subplan));
+
Again why? A MergeAppend's subplan could be a Sort plan, which will
not be projection capable.
IIUC, since we are called here right after create_plan_recurse, the
child plan would be a scan or join unless it's neither Append nor
MergeAppend. So it should be projection-capable. Maybe I'm missing
something, though.
This is not a full review. I will continue reviewing it further.
Thanks again.
Best regards,
Etsuro Fujita
