On 2015/09/03 9:41, Robert Haas wrote:
> That having been said, I don't entirely like Fujita-san's patch
> either. Much of the new code is called immediately adjacent to an FDW
> callback which could pretty trivially do the same thing itself, if
> needed.
Another idea about that code is to call that code in eg, ExecProcNode,
instead of calling ExecForeignScan there. I think that that might be
much cleaner and resolve the naming problem below.
> And much of it is contingent on whether estate->es_epqTuple
> != NULL and scanrelid == 0, but perhaps out would be better to check
> whether the subplan is actually present instead of checking whether we
> think it should be present.
Agreed with this.
> Also, the naming is a bit weird:
> node->fs_subplan gets shoved into outerPlanState(), which seems like a
> kludge.
And with this. Proposals welcome.
> I'm wondering if there's another approach. If I understand correctly,
> there are two reasons why the current situation is untenable. The
> first is that ForeignRecheck always returns true, but we could instead
> call an FDW-supplied callback routine there. The callback could be
> optional, so that we just return true if there is none, which is nice
> for already-existing FDWs that then don't need to do anything.
My question about this is, is the callback really needed? If there are
any FDWs that want to do the work *in their own way*, instead of just
doing ExecProcNode for executing a local join execution plan in case of
foreign join (or just doing ExecQual for checking remote quals in case
of foreign table), I'd agree with introducing the callback, but if not,
I don't think that that makes much sense.
Best regards,
Etsuro Fujita
