Re: Foreign join pushdown vs EvalPlanQual - Mailing list pgsql-hackers

From Etsuro Fujita
Subject Re: Foreign join pushdown vs EvalPlanQual
Date
Msg-id 5596181E.6040500@lab.ntt.co.jp
Whole thread Raw
In response to Re: Foreign join pushdown vs EvalPlanQual  (Kouhei Kaigai <kaigai@ak.jp.nec.com>)
Responses Re: Foreign join pushdown vs EvalPlanQual
List pgsql-hackers
On 2015/07/02 23:13, Kouhei Kaigai wrote:
>> To be honest, ISTM that it's difficult to do that simply and efficiently
>> for the foreign/custom-join-pushdown API that we have for 9.5.  It's a
>> little late, but what I started thinking is to redesign that API so that
>> that API is called at standard_join_search, as discussed in [2]; (1) to
>> place that API call *after* the set_cheapest call and (2) to place
>> another set_cheapest call after that API call for each joinrel.  By the
>> first set_cheapest call, I think we could probably save an alternative
>> path that we need in "cheapest_builtin_path".  By the second
>> set_cheapest call following that API call, we could consider
>> foreign/custom-join-pushdown paths also.  What do you think about this idea?

> Disadvantage is larger than advantage, sorry.
> The reason why we put foreign/custom-join hook on add_paths_to_joinrel()
> is that the source relations (inner/outer) were not obvious, thus,
> we cannot reproduce which relations are the source of this join.
> So, I had to throw a spoon when I tried this approach before.

Maybe I'm missing something, but my image about this approach is that if 
base relations for a given joinrel are all foreign tables and belong to 
the same foreign server, then by calling that API there, we consider the 
remote join over all the foreign tables, and that if not, we give up to 
consider the remote join.

> My idea is that we save the cheapest_total_path of RelOptInfo onto the
> new cheapest_builtin_path just before the GetForeignJoinPaths() hook.
> Why? It should be a built-in join logic, never be a foreign/custom-join,
> because of the hook location; only built-in logic shall be added here.

My concern about your idea is that since that (a) add_paths_to_joinrel 
is called multiple times per joinrel and that (b) repetitive add_path 
calls through GetForeignJoinPaths in add_paths_to_joinrel might remove 
old paths that are builtin, it's possible to save a path that is not 
builtin onto the cheapest_total_path and thus to save that path wrongly 
onto the cheapest_builtin_path.  There might be a good way to cope with 
that, though.

> Regarding to the development timeline, I prefer to put something
> workaround not to kick Assert() on ExecScanFetch().
> We may add a warning in the documentation not to replace built-in
> join if either/both of sub-trees are target of UPDATE/DELETE or
> FOR SHARE/UPDATE.

I'm not sure that that is a good idea, but anyway, I think we need to 
hurry fixing this issue.

Best regards,
Etsuro Fujita



pgsql-hackers by date:

Previous
From: Beena Emerson
Date:
Subject: Re: Support for N synchronous standby servers - take 2
Next
From: Martijn van Oosterhout
Date:
Subject: Re: WAL logging problem in 9.4.3?