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
tobe 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?
anddoes 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.etsuro@lab.ntt.co.jp> wrote in
<55E97786.30404@lab.ntt.co.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
