Re: PassDownLimitBound for ForeignScan/CustomScan [take-2]

From: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
To: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: Tels <nospam-pg-abuse(at)bloodgate(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Andres Freund <andres(at)anarazel(dot)de>, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Subject: Re: PassDownLimitBound for ForeignScan/CustomScan [take-2]
Date: 2017-03-13 07:25:55
Message-ID: CAM2+6=XSwHGq5RgGwTc97dZ9TohWj9+kWfO2dVUCbcu0A9ebfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I have reviewed this patch further and here are my comments:

1.
Will it be better to use compare_pathkeys() instead of
equal(root->query_pathkeys, pathkeys)?

2.
I think it will be good if we add a simple test-case in postgres_fdw.sql
which shows that LIMIT is passed to remote server. We might convert one of
the affected EXPLAIN to EXPLAIN ANALYZE.
With this patch, we do push LIMIT on foreign server but not at planning
time.
Thus we still show remote query in EXPLAIN output without LIMIT clause but
we
actually push that at execution time. Such explain plan confuses the reader.
So I think we better convert them to EXPLAIN ANALYZE or put some comments
before the query explaining this. I prefer to put comments as EXPLAIN
ANALYZE
is costly.

3.
"-- CROSS JOIN, not pushed down"
Above comment need to be changed now.
As you explained already, due to limit clause, CROSS-JOIN is pushed down to
remote server, thus need to update this comment.

Rest of the changes look good to me.

Thanks

--
Jeevan Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-03-13 07:33:33 Typo in snapbuild.c
Previous Message Andres Freund 2017-03-13 07:22:22 Re: Relation cache invalidation on replica