On Thu, Nov 30, 2017 at 3:44 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote:
> > > On Thu, Nov 30, 2017 at 1:36 AM, Antonin Houska <a...@cybertec.at> wrote: > >> The following test >> >> -- Input relation to aggregate push down hook is not safe to pushdown and >> thus >> -- the aggregate cannot be pushed down to foreign server. >> explain (verbose, costs off) >> select count(t1.c3) from ft1 t1, ft1 t2 where t1.c1 = >> postgres_fdw_abs(t1.c2); >> >> produces the following plan >> >> QUERY PLAN >> ------------------------------------------------------------ >> ---------------------------------------------- >> Aggregate >> Output: count(t1.c3) >> -> Nested Loop >> Output: t1.c3 >> -> Foreign Scan on public.ft1 t2 >> Remote SQL: SELECT NULL FROM "S 1"."T 1" >> -> Materialize >> Output: t1.c3 >> -> Foreign Scan on public.ft1 t1 >> Output: t1.c3 >> Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE (("C 1" >> = public.postgres_fdw_abs(c2))) >> >> which is not major problem as such, but gdb shows that the comment >> "aggregate >> cannot be pushed" is not correct. In fact, postgresGetForeignUpperPaths() >> *does* create the upper path. >> >> The reason that UPPERREL_GROUP_AGG is eventually not used seems to be that >> postgresGetForeignJoinPaths() -> add_foreign_grouping_paths() -> >> estimate_path_cost_size() estimates the join cost in rather generic way. >> While >> the remote server can push the join clause down to the inner relation of >> NL, >> the postgres_fdw cost computation assumes that the join clause is applied >> to >> each pair of output and input tuple. >> >> I don't think that the postgres_fdw's estimate can be fixed easily, but >> if the >> impact of "shipability" on (not) using the upper relation should be >> tested, we >> need a different test. >> > > Oops. My bad. > Agree with your analysis. > Will send a patch fixing this testcase. > Attached patch to fix the test case. In new test case I am using a JOIN query where JOIN condition is not safe to push down and hence the JOIN itself is unsafe. Due to which AggPushDown does not consider that relation. Also, I have used ft2 in the query which has use_remote_estimate set to true. Thanks > > > Thank you Antonin for catching and reporting it. > > >> >> -- >> Antonin Houska >> Cybertec Schönig & Schönig GmbH >> Gröhrmühlgasse 26 >> A-2700 Wiener Neustadt >> Web: http://www.postgresql-support.de, http://www.cybertec.at >> >> -- Jeevan Chalke Technical Architect, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 1063d92..bce3348 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -3160,21 +3160,23 @@ drop operator public.<^(int, int); -- Input relation to aggregate push down hook is not safe to pushdown and thus -- the aggregate cannot be pushed down to foreign server. explain (verbose, costs off) -select count(t1.c3) from ft1 t1, ft1 t2 where t1.c1 = postgres_fdw_abs(t1.c2); - QUERY PLAN ----------------------------------------------------------------------------------------------------------- +select count(t1.c3) from ft2 t1 left join ft2 t2 on (t1.c1 = random() * t2.c2); + QUERY PLAN +------------------------------------------------------------------------------------------- Aggregate Output: count(t1.c3) - -> Nested Loop + -> Nested Loop Left Join Output: t1.c3 - -> Foreign Scan on public.ft1 t2 - Remote SQL: SELECT NULL FROM "S 1"."T 1" + Join Filter: ((t1.c1)::double precision = (random() * (t2.c2)::double precision)) + -> Foreign Scan on public.ft2 t1 + Output: t1.c3, t1.c1 + Remote SQL: SELECT "C 1", c3 FROM "S 1"."T 1" -> Materialize - Output: t1.c3 - -> Foreign Scan on public.ft1 t1 - Output: t1.c3 - Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE (("C 1" = public.postgres_fdw_abs(c2))) -(11 rows) + Output: t2.c2 + -> Foreign Scan on public.ft2 t2 + Output: t2.c2 + Remote SQL: SELECT c2 FROM "S 1"."T 1" +(13 rows) -- Subquery in FROM clause having aggregate explain (verbose, costs off) diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 0986957..1df1e3a 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -829,7 +829,7 @@ drop operator public.<^(int, int); -- Input relation to aggregate push down hook is not safe to pushdown and thus -- the aggregate cannot be pushed down to foreign server. explain (verbose, costs off) -select count(t1.c3) from ft1 t1, ft1 t2 where t1.c1 = postgres_fdw_abs(t1.c2); +select count(t1.c3) from ft2 t1 left join ft2 t2 on (t1.c1 = random() * t2.c2); -- Subquery in FROM clause having aggregate explain (verbose, costs off)