Re: GatherMerge misses to push target list

From: Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: GatherMerge misses to push target list
Date: 2017-09-13 12:00:02
Message-ID: CAGPqQf2r-XYxnCOKjnrdWvRbkUQXM9nj5QETJUbnHWnZeePLVg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 6, 2017 at 10:04 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--
Rushabh Lathia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2017-09-13 12:20:57 Re: Patches that don't apply or don't compile: 2017-09-12
Previous Message Simon Riggs 2017-09-13 11:00:08 Re: Surjective functional indexes