On Thu, Nov 3, 2016 at 1:58 PM, Jeevan Chalke <jeevan.cha...@enterprisedb.com> wrote: > > On Wed, Nov 2, 2016 at 10:09 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> >> In 9.6, "make installcheck" in contrib/postgres_fdw takes a shade >> under 3 seconds on my machine. In HEAD, it's taking 10 seconds. >> I am not happy, especially not since there's no parallelization >> of the contrib regression tests. That's a direct consumption of >> my time and all other developers' time too. This evidently came >> in with commit 7012b132d (Push down aggregates to remote servers), >> and while that's a laudable feature, I cannot help thinking that >> it does not deserve this much of an imposition on every make check >> that's ever done for the rest of eternity. > > > Thanks Tom for reporting this substantial bloat in postgres_fdw regression > test runtime. On my machine "make installcheck" in contrib/postgres_fdw > takes 6.2 seconds on master (HEAD: > 770671062f130a830aa89100c9aa2d26f8d4bf32). > However if I remove all tests added for aggregate push down, it drops down > to 2.2 seconds. Oops 4 seconds more. > > I have timed each of my tests added as part of aggregate push down patch and > observed that one of the test (LATERAL one) is taking around 3.5 seconds. > This is causing because of the parameterization. I have added a filter so > that we will have less number of rows for parameterization. Doing this, > lateral query in question now runs in 100ms. Also updated few more queries > which were taking more than 100ms to have runtime around 30ms or so. So > effectively, with changes "make installcheck" now takes around 2.5 seconds. > > Attached patch with test-case modification. >

## Advertising

I verified that this patch indeed bring the time down to 2 to 3 seconds from 10 seconds. The additional condition t2.c2 = 6 seems to echo the filter t2.c2 = 6 of aggregate. We wouldn't know which of those actually worked. I modified the testcase to use t2.c2 % 6 = 0 instead and keep the filter condition intact. This increases the execution time by .2s, which may be ok. Let me know what you thing of the attached patch. Also, please add this to the commitfest, so that it isn't forgotten. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 0045f3f..3a09280 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -2685,9 +2685,9 @@ select sum(c1%3), sum(distinct c1%3 order by c1%3) filter (where c1%3 < 2), c2 f -- Outer query is aggregation query explain (verbose, costs off) -select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; - QUERY PLAN -------------------------------------------------------------------------------------------------------- +select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------ Unique Output: ((SubPlan 1)) -> Sort @@ -2696,14 +2696,14 @@ select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft -> Foreign Scan Output: (SubPlan 1) Relations: Aggregate on (public.ft2 t2) - Remote SQL: SELECT count(*) FILTER (WHERE ((c2 = 6) AND ("C 1" < 10))) FROM "S 1"."T 1" + Remote SQL: SELECT count(*) FILTER (WHERE ((c2 = 6) AND ("C 1" < 10))) FROM "S 1"."T 1" WHERE (((c2 % 6) = 0)) SubPlan 1 -> Foreign Scan on public.ft1 t1 Output: (count(*) FILTER (WHERE ((t2.c2 = 6) AND (t2.c1 < 10)))) Remote SQL: SELECT NULL FROM "S 1"."T 1" WHERE (("C 1" = 6)) (13 rows) -select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; +select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; count ------- 1 @@ -2711,7 +2711,7 @@ select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft -- Inner query is aggregation query explain (verbose, costs off) -select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; +select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------------------ Unique @@ -2721,7 +2721,7 @@ select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) fro Sort Key: ((SubPlan 1)) -> Foreign Scan on public.ft2 t2 Output: (SubPlan 1) - Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" + Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (((c2 % 6) = 0)) SubPlan 1 -> Foreign Scan Output: (count(t1.c1) FILTER (WHERE ((t2.c2 = 6) AND (t2.c1 < 10)))) @@ -2729,7 +2729,7 @@ select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) fro Remote SQL: SELECT count("C 1") FILTER (WHERE (($1::integer = 6) AND ($2::integer < 10))) FROM "S 1"."T 1" WHERE (("C 1" = 6)) (13 rows) -select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; +select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; count ------- 0 @@ -3086,7 +3086,7 @@ select sum(c2) * (random() <= 1)::int as sum from ft1 order by 1; -- LATERAL join, with parameterization set enable_hashagg to false; explain (verbose, costs off) -select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 10 order by 1; +select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1; QUERY PLAN ---------------------------------------------------------------------------------------------------------- Sort @@ -3094,9 +3094,10 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr Sort Key: t1.c2 -> Nested Loop Output: t1.c2, qry.sum - -> Seq Scan on "S 1"."T 1" t1 + -> Index Scan using t1_pkey on "S 1"."T 1" t1 Output: t1."C 1", t1.c2, t1.c3, t1.c4, t1.c5, t1.c6, t1.c7, t1.c8 - Filter: (t1.c2 < 10) + Index Cond: (t1."C 1" < 100) + Filter: (t1.c2 < 3) -> Subquery Scan on qry Output: qry.sum, t2.c1 Filter: ((t1.c2 * 2) = qry.sum) @@ -3104,21 +3105,14 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr Output: (sum((t2.c1 + t1."C 1"))), t2.c1 Relations: Aggregate on (public.ft2 t2) Remote SQL: SELECT sum(("C 1" + $1::integer)), "C 1" FROM "S 1"."T 1" GROUP BY "C 1" -(15 rows) +(16 rows) -select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 10 order by 1; +select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1; c2 | sum ----+----- 1 | 2 2 | 4 - 3 | 6 - 4 | 8 - 5 | 10 - 6 | 12 - 7 | 14 - 8 | 16 - 9 | 18 -(9 rows) +(2 rows) reset enable_hashagg; -- Check with placeHolderVars diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 9191776..e19a3ef 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -641,12 +641,12 @@ select sum(c1%3), sum(distinct c1%3 order by c1%3) filter (where c1%3 < 2), c2 f -- Outer query is aggregation query explain (verbose, costs off) -select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; -select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; +select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; +select distinct (select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; -- Inner query is aggregation query explain (verbose, costs off) -select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; -select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 order by 1; +select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; +select distinct (select count(t1.c1) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where t1.c1 = 6) from ft2 t2 where t2.c2 % 6 = 0 order by 1; -- Aggregate not pushed down as FILTER condition is not pushable explain (verbose, costs off) @@ -802,8 +802,8 @@ select sum(c2) * (random() <= 1)::int as sum from ft1 order by 1; -- LATERAL join, with parameterization set enable_hashagg to false; explain (verbose, costs off) -select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 10 order by 1; -select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 10 order by 1; +select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1; +select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1; reset enable_hashagg; -- Check with placeHolderVars

-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers