(2014/11/18 18:37), Ashutosh Bapat wrote:
On Tue, Nov 18, 2014 at 1:55 PM, Etsuro Fujita
<[email protected] <mailto:[email protected]>> wrote:
(2014/11/17 19:54), Ashutosh Bapat wrote:
Here are comments for postgres_fdw-syscol patch.
Code
-------
1. Instead of a single liner comment "System columns can't be
sent to
remote.", it might be better to explain why system columns can't
be sent
to the remote.
Done.
I would add " and foreign values do not make sense locally (except may
be ctid clubbed with foreign server_id)" to make it more clear. But I
will leave that for the commiter to decide.
OK.
3. Since there is already a testcase which triggered this particular
change, it will good, if we add that to regression in postgres_fdw.
Done.
I think, a better case would be SELECT * FROM ft1 t1, pg_class t2 WHERE
t1.tableoid = t2.oid. The condition makes sure that the tableoid in the
row is same as the OID of the foreign table recorded in pg_class
locally. And the EXPLAIN of the query which clearly shows that the
tableoid column in not fetched from the foreign server.
I thought that test, but I didn't use it because I think we can't see
(at least from the EXPLAIN) why the qual is not pushed down: the qual
isn't pushed down possibly becasue the qual is considered as a *join*
qual, not because the qual just cotains tableoid. (Having said that, I
think we can see if the qual isn't pushed down as a join qual for a
parameterized plan, but ISTM it's worth complicating regression tests.)
Once we resolve the other patch on this thread, I think this item can
be marked as ready for commiter from my side.
OK. Thank you for the review.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
