Re: Unclear regression test for postgres_fdw

From: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
To: Antonin Houska <ah(at)cybertec(dot)at>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Unclear regression test for postgres_fdw
Date: 2017-11-30 10:14:05
Message-ID: CAM2+6=Um1iMhWhPHqz8v=4aYNS_+d9-fE4dcaLF4dEoXFoyZkA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 30, 2017 at 1:36 AM, Antonin Houska <ah(at)cybertec(dot)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.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-11-30 10:19:44 Re: es_query_dsa is broken
Previous Message Stas Kelvich 2017-11-30 08:58:25 Re: [HACKERS] Issues with logical replication