Re: Misplacement of function declaration in contrib/postgres_fdw/postgres_fdw.h

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

In response to

Browse pgsql-hackers by date

  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