Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)

> On Tue, Jan 6, 2015 at 9:17 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> > The attached patch is newer revision of custom-/foreign-join
> > interface.
> 
> It seems that the basic purpose of this patch is to allow a foreign scan
> or custom scan to have scanrelid == 0, reflecting the case where we are
> scanning a joinrel rather than a baserel.  The major problem that seems
> to create is that we can't set the target list from the relation descriptor,
> because there isn't one.  To work around that, you've added fdw_ps_list
> and custom_ps_tlist, which the FDW or custom-plan provider must set.  I
> don't know off-hand whether that's a good interface or not.  How does the
> FDW know what to stick in there?
>
In the most usual scenario, FDP/CSP will make a ps_tlist according to the
target-list of the joinrel (that contains mixture of var-nodes to left-side
and right-side), and qualifier's expression tree if any.
As long as FDW can construct a remote query, it knows which attributes shall
be returned and which relation does it come from. It is equivalent to what
ps_tlist tries to inform the core optimizer.


> There's a comment that seems to be trying to explain this:
> 
> + * An optional fdw_ps_tlist is used to map a reference to an attribute
> + of
> + * underlying relation(s) on a pair of INDEX_VAR and alternative varattno.
> + * It looks like a scan on pseudo relation that is usually result of
> + * relations join on remote data source, and FDW driver is responsible
> + to
> + * set expected target list for this. If FDW returns records as
> + foreign-
> + * table definition, just put NIL here.
> 
> ...but I can't understand what that's telling me.
> 
Sorry, let me explain in another expression.

A joinrel has a target-list that can/may contain references to both of
the left and right relations. These are eventually mapped to either
INNER_VAR or OUTER_VAR, then executor switch the TupleTableSlot
(whether ecxt_innertuple or ecxt_outertuple) according to the special
varno.
On the other hands, because ForeignScan/CustomScan is a scan plan, it
shall have just one TupleTableSlot on execution time. Thus, we need a
mechanism that maps attributes from both of the relations on a certain
location of the slot; that shall be eventually translated to var-node
with INDEX_VAR to reference ecxt_scantuple.
Of course, ps_tlist is not necessary if ForeignScan/CustomScan scans
on a base relation as literal. In this case, the interface contract
expects NIL is set on the ps_tlist field.

> You've added an "Oid fdw_handler" field to the ForeignScan and RelOptInfo
> structures.  I think this is the OID of the pg_proc entry for the handler
> function; and I think we need it because, if scanrelid == 0 then we don't
> have a relation that we can trace to a foreign table, to a server, to an
> FDW, and then to a handler.  So we need to get that information some other
> way.  When building joinrels, the fdw_handler OID, and the associated
> routine, are propagated from any two relations that share the same
> fdw_handler OID to the resulting joinrel.  I guess that's reasonable,
> although it feels a little weird that we're copying around both the OID
> and the structure-pointer.
>
Unlike CustomScan node, ForeignScan node does not have function pointers.
In addition, it is dynamically allocated by palloc(), so we have no
guarantee the pointer constructed on plan-stage is valid on beginning
of the executor.
It is the reason why I put OID of the FDW handler routine.
Any other better idea?

> For non-obvious reasons, you've made create_plan_recurse() non-static.
> 
When custom-scan node replaced a join-plan, it shall have at least two
child plan-nodes. The callback handler of PlanCustomPath needs to be
able to call create_plan_recurse() to transform the underlying path-nodes
to plan-nodes, because this custom-scan node may take other built-in
scan or sub-join nodes as its inner/outer input.
In case of FDW, it shall kick any underlying scan relations to remote
side, thus we may not expect ForeignScan has underlying plans...

Thanks,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>