On Wed, Sep 6, 2017 at 10:04 AM, Amit Kapila <amit.kapil...@gmail.com>
wrote:

> During my recent work on costing of parallel paths [1], I noticed that
> we are missing to push target list below GatherMerge in some simple
> cases like below.
>
> Test prepration
> ---------------------
> create or replace function simple_func(var1 integer) returns integer
> as $$
> begin
>         return var1 + 10;
> end;
> $$ language plpgsql PARALLEL SAFE;
>
> create table t1(c1 int, c2 char(5));
> insert into t1 values(generate_series(1,500000), 'aaa');
> set parallel_setup_cost=0;
> set parallel_tuple_cost=0;
> set min_parallel_table_scan_size=0;
> set max_parallel_workers_per_gather=4;
> set cpu_operator_cost=0; set min_parallel_index_scan_size=0;
>
> Case-1
> -------------
> postgres=# explain (costs off, verbose) select c1,simple_func(c1) from
> t1 where c1 < 10000 order by c1;
>                      QUERY PLAN
> -----------------------------------------------------
>  Gather Merge
>    Output: c1, simple_func(c1)
>    Workers Planned: 4
>    ->  Parallel Index Scan using idx_t1 on public.t1
>          Output: c1
>          Index Cond: (t1.c1 < 10000)
> (6 rows)
>
> In the above case, I don't see any reason why we can't push the target
> list expression (simple_func(c1)) down to workers.
>
> Case-2
> ----------
> set enable_indexonlyscan=off;
> set enable_indexscan=off;
> postgres=# explain (costs off, verbose) select c1,simple_func(c1) from
> t1 where c1 < 10000 order by c1;
>                      QUERY PLAN
> ----------------------------------------------------
>  Gather Merge
>    Output: c1, simple_func(c1)
>    Workers Planned: 4
>    ->  Sort
>          Output: c1
>          Sort Key: t1.c1
>          ->  Parallel Bitmap Heap Scan on public.t1
>                Output: c1
>                Recheck Cond: (t1.c1 < 10000)
>                ->  Bitmap Index Scan on idx_t1
>                      Index Cond: (t1.c1 < 10000)
> (11 rows)
>
> It is different from above case (Case-1) because sort node can't
> project, but I think adding a Result node on top of it can help to
> project and serve our purpose.
>
> The attached patch allows pushing the target list expression in both
> the cases. I think we should handle GatherMerge case in
> apply_projection_to_path similar to Gather so that target list can be
> pushed.  Another problem was during the creation of ordered paths
> where we don't allow target expressions to be pushed below gather
> merge.
>
> Plans after patch:
>
> Case-1
> ----------
> postgres=# explain (costs off, verbose) select c1,simple_func(c1) from
> t1 where c1 < 10000 order by c1;
>                         QUERY PLAN
> ----------------------------------------------------------
>  Gather Merge
>    Output: c1, (simple_func(c1))
>    Workers Planned: 4
>    ->  Parallel Index Only Scan using idx_t1 on public.t1
>          Output: c1, simple_func(c1)
>          Index Cond: (t1.c1 < 10000)
> (6 rows)
>
> Case-2
> -----------
> postgres=# explain (costs off, verbose) select c1,simple_func(c1) from
> t1 where c1 < 10000 order by c1;
>                         QUERY PLAN
> ----------------------------------------------------------
>  Gather Merge
>    Output: c1, (simple_func(c1))
>    Workers Planned: 4
>    ->  Result
>          Output: c1, simple_func(c1)
>          ->  Sort
>                Output: c1
>                Sort Key: t1.c1
>                ->  Parallel Bitmap Heap Scan on public.t1
>                      Output: c1
>                      Recheck Cond: (t1.c1 < 10000)
>                      ->  Bitmap Index Scan on idx_t1
>                            Index Cond: (t1.c1 < 10000)
> (13 rows)
>
> Note, that simple_func() is pushed down to workers in both the cases.
>
> Thoughts?
>

This seems like a good optimization. I tried to simulate the test given
in the mail, initially wasn't able to generate the exact test - as index
creation is missing in the test shared.

I also won't consider this as bug, but its definitely good optimization
for GatherMerge.


>
> Note - If we agree on the problems and fix, then I can add regression
> tests to cover above cases in the patch.
>
> [1] - https://www.postgresql.org/message-id/CAA4eK1Ji_
> 0pgrjFotAyvvfxGikxJQEKcxD863VQ-xYtfQBy0uQ%40mail.gmail.com


Sure, once you do that - I will review the patch.

Thanks,


>
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>


-- 
Rushabh Lathia

Reply via email to