Re: [HACKERS] postgres_fdw bug in 9.6

On Wed, Dec 28, 2016 at 1:29 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> On 2016/12/28 15:54, Ashutosh Bapat wrote:
>>
>> On Wed, Dec 28, 2016 at 9:20 AM, Etsuro Fujita
>> <fujita.etsuro@lab.ntt.co.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.

>
>>>> The reason we chose to pick up an existing path was that the
>>>> discussion in thread [1] concluded the efficiency of the local plan
>>>> wasn't a concern for EPQ. Are we now saying something otherwise?
>
>
>>> No, I won't.  Usually, the overhead would be negligible, but in some
>>> cases
>>> where there are many concurrent updates, the overhead might not be
>>> negligible due to many EPQ rechecks.  So it would be better to have an
>>> efficient local plan.
>
>
>> All that the EPQ rechecks do is apply the join and other quals again
>> on the base relation rows. Will choice of plan affect the efficiency?
>
>
> Merge or hash joins would need extra steps to start that work (for example,
> building a hash table from the inner relation for a hash join.)

Hmm, I agree.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company