From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: postgres_fdw behaves oddly |
Date: | 2014-11-18 09:37:20 |
Message-ID: | CAFjFpRfB35s2xdOPuVckT6ha7Cvc+rdPJqU8gt4Mx1jpybFK_A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Nov 18, 2014 at 1:55 PM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:
> (2014/11/17 19:54), Ashutosh Bapat wrote:
>
>> Here are comments for postgres_fdw-syscol patch.
>>
>
> Thanks for the review!
>
> 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.
> 2. The comment in deparseVar is single line comment, so it should start
>> and end on the same line i.e. /* and */ should be on the same line.
>>
>
> Done.
>
Thanks
>
> 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.
Please find attached an updated version of the patch.
Once we resolve the other patch on this thread, I think this item can be
marked as ready for commiter from my side.
>
>
> Thanks,
>
> Best regards,
> Etsuro Fujita
>
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2014-11-18 09:40:35 | Re: group locking: incomplete patch, just for discussion |
Previous Message | Simon Riggs | 2014-11-18 09:33:48 | Re: Review of Refactoring code for sync node detection |