Re: Foreign join pushdown vs EvalPlanQual
От | Etsuro Fujita |
---|---|
Тема | Re: Foreign join pushdown vs EvalPlanQual |
Дата | |
Msg-id | 56651859.40904@lab.ntt.co.jp обсуждение исходный текст |
Ответ на | Re: Foreign join pushdown vs EvalPlanQual (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: Foreign join pushdown vs EvalPlanQual
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On 2015/12/05 5:15, Robert Haas wrote: > On Tue, Dec 1, 2015 at 10:20 PM, Etsuro Fujita > <fujita.etsuro@lab.ntt.co.jp> wrote: >> One thing I can think of is that we can keep both the structure of a >> ForeignPath node and the API of create_foreignscan_path as-is. The latter >> is a good thing for FDW authors. And IIUC the patch you posted today, I >> think we could make create_foreignscan_plan a bit simpler too. Ie, in your >> patch, you modified that function as follows: >> >> @@ -2129,7 +2134,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath >> *best_path, >> */ >> scan_plan = rel->fdwroutine->GetForeignPlan(root, rel, rel_oid, >> >> best_path, >> - >> tlist, scan_clauses); >> + >> tlist, >> + >> scan_clauses); >> + outerPlan(scan_plan) = fdw_outerplan; >> >> I think that would be OK, but I think we would have to do a bit more here >> about the fdw_outerplan's targetlist and qual; I think that the targetlist >> needs to be changed to fdw_scan_tlist, as in the patch [1], and that it'd be >> better to change the qual to remote conditions, ie, quals not in the >> scan_plan's scan.plan.qual, to avoid duplicate evaluation of local >> conditions. (In the patch [1], I didn't do anything about the qual because >> the current postgres_fdw join pushdown patch assumes that all the the >> scan_plan's scan.plan.qual are pushed down.) Or, FDW authors might want to >> do something about fdw_recheck_quals for a foreign-join while creating the >> fdw_outerplan. So if we do that during GetForeignPlan, I think we could >> make create_foreignscan_plan a bit simpler, or provide flexibility to FDW >> authors. > It's certainly true that we need the alternative plan's tlist to match > that of the main plan; otherwise, it's going to be difficult for the > FDW to make use of that alternative subplan to fill its slot, which is > kinda the point of all this. OK. > However, I'm quite reluctant to > introduce code into create_foreignscan_plan() that forces the > subplan's tlist to match that of the main plan. For one thing, that > would likely foreclose the possibility of an FDW ever using the outer > plan for any purpose other than EPQ rechecks. It may be hard to > imagine what else you'd do with the outer plan as things are today, > but right now the two haves of the patch - letting FDWs have an outer > subplan, and providing them with a way of overriding the EPQ recheck > behavior - are technically independent. Putting tlist-altering > behavior into create_foreignscan_plan() ties those two things together > irrevocably. Agreed. > Instead, I think we should go the opposite direction and pass the > outerplan to GetForeignPlan after all. I was lulled into a full sense > of security by the realization that every FDW that uses this feature > MUST want to do outerPlan(scan_plan) = fdw_outerplan. That's true, > but irrelevant. The point is that the FDW might want to do something > additional, like frob the outer plan's tlist, and it can't do that if > we don't pass it fdw_outerplan. So we should do that, after all. As I proposed upthread, another idea would be to 1) to store an fdw_outerpath in the fdw_private list of a ForeignPath node, and then 2) to create an fdw_outerplan from *the fdw_outerpath stored into the fdw_private* in GetForeignPlan. One good thing for this is that we keep the API of create_foreignscan_path as-is. What do you think about that? > Updated patch attached. This fixes a couple of whitespace issues that > were pointed out, also. Thanks for updating the patch! Best regards, Etsuro Fujita
В списке pgsql-hackers по дате отправления:
Следующее
От: Michael PaquierДата:
Сообщение: Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc.