Message-ID: <5C861A2A.6030800@lab.ntt.co.jp>
Date: Mon, 11 Mar 2019 17:19:54 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Re: Problems with plan estimates in postgres_fdw
References: <87pnz1aby9.fsf@news-spur.riddles.org.uk>
 <1175.1533220908@sss.pgh.pa.us> <5BB739CD.6040002@lab.ntt.co.jp>
 <5BBC4140.50403@lab.ntt.co.jp> <5C17A00E.5000005@lab.ntt.co.jp>
 <5C232F39.9060509@lab.ntt.co.jp> <5C25C7B5.2010802@lab.ntt.co.jp>
 <5C518631.5080401@lab.ntt.co.jp> <10994.1549559088@localhost>
 <5C5D778F.3080501@lab.ntt.co.jp> <5C6D474F.5010703@lab.ntt.co.jp>
 <30939.1550848901@localhost> <5C73B7FB.2010309@lab.ntt.co.jp>
 <7520.1551439018@localhost> <5C7E4127.2030802@lab.ntt.co.jp>
 <8642.1552062349@localhost>
In-Reply-To: <8642.1552062349@localhost>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
To: Antonin Houska <ah@cybertec.at>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andrew Gierth <andrew@tao11.riddles.org.uk>,
        pgsql-hackers@postgresql.org
Precedence: bulk

(2019/03/09 1:25), Antonin Houska wrote:
> Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp>  wrote:
>> (2019/03/01 20:16), Antonin Houska wrote:
>>> Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp>   wrote:

>>>> Conversely, it appears that add_foreign_ordered_paths() added by the patchset
>>>> would generate such pre-sorted paths *redundantly* when the input_rel is the
>>>> final scan/join relation.  Will look into that.
>>>
>>> Currently I have no idea how to check the plan created by FDW at the
>>> UPPERREL_ORDERED stage, except for removing the sort from the
>>> UPPERREL_GROUP_AGG stage as I proposed here:
>>>
>>> https://www.postgresql.org/message-id/11807.1549564431%40localhost
>>
>> I don't understand your words "how to check the plan created by FDW". Could
>> you elaborate on that a bit more?
>
> I meant that I don't know how to verify that the plan that sends the ORDER BY
> clause to the remote server was created at the UPPERREL_ORDERED and not at
> UPPERREL_GROUP_AGG. However if the ORDER BY clause really should not be added
> at the UPPERREL_GROUP_AGG stage and if we ensure that it no longer happens,
> then mere presence of ORDER BY in the (remote) plan means that the
> UPPERREL_ORDERED stage works fine.

I don't think we need to consider pushing down the query's ORDER BY to 
the remote in GetForeignUpperPaths() for each of upper relations below 
UPPERREL_ORDERED (ie, UPPERREL_GROUP_AGG, UPPERREL_WINDOW, and 
UPPERREL_DISTINCT); I think that's a job for GetForeignUpperPaths() for 
UPPERREL_ORDERED, though the division of labor would be arbitrary. 
However, I think it's a good thing that there is a room for considering 
remote sorts even in GetForeignUpperPaths() for UPPERREL_GROUP_AGG, 
because some remote sorts might be useful to process its upper relation. 
  Consider this using postgres_fdw:

postgres=# explain verbose select distinct count(a) from ft1 group by b;
                                QUERY PLAN
------------------------------------------------------------------------
  Unique  (cost=121.47..121.52 rows=10 width=12)
    Output: (count(a)), b
    ->  Sort  (cost=121.47..121.49 rows=10 width=12)
          Output: (count(a)), b
          Sort Key: (count(ft1.a))
          ->  Foreign Scan  (cost=105.00..121.30 rows=10 width=12)
                Output: (count(a)), b
                Relations: Aggregate on (public.ft1)
                Remote SQL: SELECT count(a), b FROM public.t1 GROUP BY 2
(9 rows)

For this query it might be useful to push down the sort on top of the 
foreign scan.  To allow that, we should allow GetForeignUpperPaths() for 
UPPERREL_GROUP_AGG to consider that sort pushdown.  (We would soon 
implement the SELECT DISTINCT pushdown for postgres_fdw, and if so, we 
would no longer need to consider that sort pushdown in 
postgresGetForeignUpperPaths() for UPPERREL_GROUP_AGG, but I don't think 
all FDWs can have the ability to push down SELECT DISTINCT to the remote...)

Best regards,
Etsuro Fujita