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:36:40
Message-ID: CAM2+6=UGBx+WhUnX9O+vYEgtSeaLP-SKmKMVBNcSOFRqwDCL=g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 30, 2017 at 3:44 PM, Jeevan Chalke <
jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:

>
>
> 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.
>

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

Attachment Content-Type Size
update_agg_push_down_test.patch text/x-patch 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikhil Sontakke 2017-11-30 10:38:59 Re: [HACKERS] logical decoding of two-phase transactions
Previous Message Thomas Munro 2017-11-30 10:19:44 Re: es_query_dsa is broken