Re: postgres_fdw bug in 9.6

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: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw bug in 9.6
Date: 2017-01-05 03:10:55
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2016/12/28 17:34, Ashutosh Bapat wrote:
> On Wed, Dec 28, 2016 at 1:29 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2016/12/28 15:54, Ashutosh Bapat wrote:
>>> On Wed, Dec 28, 2016 at 9:20 AM, Etsuro Fujita
>>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>> On 2016/12/27 22:03, Ashutosh Bapat wrote:
>>>>> If mergejoin_allowed is true and mergeclauselist is non-NIL but
>>>>> hashclauselist is NIL (that's rare but there can be types has merge
>>>>> operators but not hash operators), we will end up returning NULL. I
>>>>> think we want to create a merge join in that case. I think the order
>>>>> of conditions should be 1. check hashclause_list then create hash join
>>>>> 2. else check if merge allowed, create merge join. It looks like that
>>>>> would cover all the cases, if there aren't any hash clauses, and also
>>>>> no merge clauses, we won't be able to implement a FULL join, so it
>>>>> will get rejected during path creation itself.

>>>> Right, maybe we can do that by doing similar things as in
>>>> match_unsort_outer
>>>> and/or sort_inner_and_outer. But as you mentioned, the case is rare, so
>>>> the
>>>> problem would be whether it's worth complicating the code (and if it's
>>>> worth, whether we should do that at the first version of the function).

>>> All I am requesting is changing the order of conditions. That doesn't
>>> complicate the code.

>> I might have misunderstood your words, but you are saying we should consider
>> mergejoin paths with some mergeclauses in the case where hashclauses is NIL,
>> right? To do so, we would need to consider the sort orders of outer/inner
>> paths, which would make the code complicated.

> Hmm. If I understand the patch correctly, it does not return any path
> when merge join is allowed and there are merge clauses but no hash
> clauses. In this case we will not create a foreign join path, loosing
> some optimization. If we remove GetExistingLocalJoinPath, which
> returns a path in those cases as well, we have a regression in
> performance.

Ok, will revise, but as I mentioned upthread, I'm not sure it's a good
idea to search the pathlist to get a merge join even in this case. I'd
vote for creating a merge join path from the inner/outer paths in this
case as well. I think that would simplify the code as well.

Best regards,
Etsuro Fujita

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2017-01-05 03:12:09 Re: Supporting huge pages on Windows
Previous Message Thomas Munro 2017-01-05 03:10:32 Re: HASH_CHUNK_SIZE vs malloc rounding