From: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Misplacement of function declaration in contrib/postgres_fdw/postgres_fdw.h |
Date: | 2017-01-13 06:44:03 |
Message-ID: | 1108705a-c70d-c9af-2ea5-2bee6df63551@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2017/01/12 13:52, Ashutosh Bapat wrote:
> On Thu, Jan 12, 2017 at 8:49 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> While working on pushing down more joins/updates to the remote, I noticed
>> that in contrib/postgres_fdw/postgres_fdw.h the declaration of
>> get_jointype_name is misplaced in the section of shippable.c. Since that
>> function is defined in contrib/postgres_fdw/deparse.c, we should put that
>> declaration in the section of deparse.c in the header file. Attached is a
>> patch for that.
> I think, initially (probably in a never committed patch) the function
> was used to check whether a join type is shippable and if so return
> the name to be used in the query. That may be the reason why it ended
> up in shippability.c. But later the shippability test was separated
> from the code which required the string representation.
Thanks for the explanation!
> Thanks for
> pointing out the descripancy. The patch looks good. As a side change,
> should we include "JOIN" in the string returned by this fuction? The
> two places where this function is called, append "JOIN" to the string
> returned by this function.
I was thinking that, so +1.
> Although, even without that change, the
> patch looks good.
Thanks again.
Best regards,
Etsuro Fujita
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2017-01-13 06:48:10 | Re: Transactions involving multiple postgres foreign servers |
Previous Message | Pavel Stehule | 2017-01-13 06:30:44 | Re: plpgsql - additional extra checks |