Re: [HACKERS] postgres_fdw bug in 9.6

On 2016/12/17 1:13, Tom Lane wrote:
> Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> writes:
>> On 2016/12/16 11:25, Etsuro Fujita wrote:
>>> As I said upthread, an alternative I am thinking is (1) to create an
>>> equivalent nestloop join path using inner/outer paths of a foreign join
>>> path, except when that join path implements a full join, in which case a
>>> merge/hash join path is used, (2) store it in fdw_outerpath as-is, and
>>> (3) during an EPQ recheck, apply postgresRecheckForeignScan to the outer
>>> subplan created from the fdw_outerpath as-is.  What do you think about
>>> that?

>> Let me explain about #1 and #2 a bit more.  What I have in mind is:

>> * modify postgresGetForeignPaths so that a simple foreign table scan
>> path is stored into the base relation's PgFdwRelationInfo.
>> * modify postgresGetForeignJoinPaths so that
>>      (a) a local join path for a 2-way foreign join is created using
>> simple foreign table scan paths stored in the base relations'
>> PgFdwRelationInfos, and stored into the join relation's PgFdwRelationInfo.
>>      (b) a local join path for a 3-way foreign join, whose left input is
>> a 2-way foreign join, is created using a local join path stored in the
>> left input join relation's PgFdwRelationInfo and a simple foreign table
>> scan path stored into the right input base relation's PgFdwRelationInfo.
>>      (c) Likewise for higher level foreign joins.
>>      (d) local join paths created are passed to create_foreignscan_path
>> and stored into the fdw_outerpaths of the resulting ForeignPahts.

> Hm, isn't this overcomplicated?  As you said earlier, we don't really
> care all that much whether the fdw_outerpath is free of lower foreign
> joins, because EPQ setup will select those lower join's substitute EPQ
> plans anyway.  All that we need is that the EPQ tree be a legal
> implementation of the join order with join quals applied at the right
> places.

Exactly.  I thought the EPQ trees without lower foreign joins would be 
better because that would be easier to understand.

> So I think the rule could be

> "When first asked to produce a path for a given foreign joinrel, collect
> the cheapest paths for its left and right inputs, and make a nestloop path
> (or hashjoin path, if full join) from those, using the join quals needed
> for the current input relation pair.

Seems reasonable.

> Use this as the fdw_outerpath for
> all foreign paths made for the joinrel."

I'm not sure that would work well for foreign joins with sort orders. 
Consider a merge join, whose left input is a 2-way foreign join with a 
sort order that implements a full join and whose right input is a sorted 
local table scan.  If the EPQ subplan for the foreign join wouldn't 
produce the right sort order, the merge join might break during EPQ 
rechecks (note that in this case the EPQ subplan for the foreign join 
might produce more than a single row during an EPQ recheck).  So, I 
think we would need to add an explicit sort to the fdw_outerpath for the 
foreign join.

> The important point here is that we avoid using a merge join because that
> has assumptions about input ordering that likely won't be satisfied by
> the child paths chosen through this method.  (I guess you could fall back
> to it for the case of no quals in a fulljoin, because then the ordering
> assumptions are vacuous anyway.)

I agree on that point.  I'll create a patch.

Best regards,
Etsuro Fujita