Re: Foreign join pushdown vs EvalPlanQual

Hello, sorry in advance for possible brought up of past
discussions or pointless discussion.

> I'm attaching an updated version of the patch. The patch is based on
> the SS_finalize_plan patch that has been recently committed. I'd be
> happy if this helps people discuss more about how to fix this issue.

The two patches make a good contrast to clarify the problem for
me, maybe.

> > code in ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
> > ExecReScanForeignScan. I think that would resolve the name problem
> > also.

I found two points in this discussion.

1. Where (or When) to initialize a foreign/custom scan node for
recheck.

Having a new list to hold substitute plans in planner global
(and PlannedStmt) is added, EvalPlanQualStart() looks to be
the best place to initialize them.

Of couse it could not be a solution unless the new member and
related code are not acceptable or rather unreasonable. The
possible timing left for the case would be ExecInitNode() (as
v2.0) or FDW routines (as v1.0).

2. How the core informs fdw/custom scan handlers wheter it is
during recheck.

In v1.0 patch, nodeForignscan.c routines detect the situation
using es_epqTuple and Scan.scanrelid which the core as is
gives, and v2.0 alternatively replaces scan node implicitly
(and maybe irregularly) itself on initialization. The latter
don't looks to me tidy.

I think refining v1.0 would be more desirable, and resolving
the redundancy would be simply a matter of notation.

If I understand there correctly, Exec*ForeignScan() other than
ExecInitForeignScan() can determine the behavior simply
looking outerPlanState(scanstate). (If we continue to use the
member lefttree for the purpose..). Is it right? and does it
eliminate the redundancy?

ExecEndForeignScan()
{
if ((outerplan = outerPlanState(node)) != NULL)
ExecEndNode(outerPlan);
...

regards,

At Fri, 04 Sep 2015 19:50:46 +0900, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <55E97786(dot)30404(at)lab(dot)ntt(dot)co(dot)jp>
> On 2015/09/03 19:25, Etsuro Fujita wrote:
> > On 2015/09/03 14:22, Etsuro Fujita wrote:
> >> 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.
...
> > I gave it another thought; the following changes to ExecInitNode would
> > make the patch much simpler, ie, we would no longer need to call the
> > new
> > code in ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
> > ExecReScanForeignScan. I think that would resolve the name problem
> > also.
>
> I'm attaching an updated version of the patch. The patch is based on
> the SS_finalize_plan patch that has been recently committed. I'd be
> happy if this helps people discuss more about how to fix this issue.

--
Kyotaro Horiguchi
NTT Open Source Software Center