Re: Foreign join pushdown vs EvalPlanQual
От | Robert Haas |
---|---|
Тема | Re: Foreign join pushdown vs EvalPlanQual |
Дата | |
Msg-id | CA+TgmobA4MSKgquicgt5CkbpQJ-TmpqEfHt_wy49ndwa91Wkpw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Foreign join pushdown vs EvalPlanQual (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>) |
Ответы |
Re: Foreign join pushdown vs EvalPlanQual
|
Список | pgsql-hackers |
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. 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. 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. Updated patch attached. This fixes a couple of whitespace issues that were pointed out, also. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
В списке pgsql-hackers по дате отправления: