Re: [v9.5] Custom Plan API - Mailing list pgsql-hackers

> Kouhei Kaigai <kaigai@ak.jp.nec.com> writes:
> >> I've done some preliminary cleanup on this patch, but I'm still
> >> pretty desperately unhappy about some aspects of it, in particular
> >> the way that it gets custom scan providers directly involved in
> >> setrefs.c, finalize_primnode, and replace_nestloop_params processing.
> >> I don't want any of that stuff exported outside the core, as freezing
> >> those APIs would be a very nasty restriction on future planner
> development.
>
> > If core backend can know which field contains expression nodes but
> > processed by custom-scan provider, FinalizedCustomScan might be able
> > to rid. However, rid of SetCustomScanRef makes unavailable a
> > significant use case I intend.
> > In case when tlist contains complicated expression node (thus it takes
> > many cpu cycles) and custom-scan provider has a capability to compute
> > this expression node externally, SetCustomScanRef hook allows to
> > replace this complicate expression node by a simple Var node that
> > references a value being externally computed.
>
> That's a fine goal to have, but this is not a solution that works for any
> except trivial cases.  The problem is that that complicated expression
> isn't going to be in the CustomScan's tlist in the first place unless you
> have a one-node plan.  As soon as you have a join, for example, the planner
> is going to delay calculation of anything more complex than a plain Var
> to above the join.  Aggregation, GROUP BY, etc would also defeat such an
> optimization.
>
> This gets back to the remarks I made earlier about it not being possible
> to do anything very interesting in a plugin of this nature.  You really
> need cooperation from other places in the planner if you want to do things
> like pushing down calculations into an external provider.  And it's not
> at all clear how that would even work, let alone how we might make it
> implementable as a plugin rather than core code.
>
> Also, even if we could think of a way to do this from a CustomScan plugin,
> that would fail to cover some very significant use-cases for pushing down
> complex expressions, for example:
> * retrieving values of expensive functions from expression indexes;
> * pushing down expensive functions into FDWs so they can be done remotely.
> And I'm also worried that once we've exported and thereby frozen the APIs
> in this area, we'd be operating with one hand tied behind our backs in solving
> those use-cases.  So I'm not very excited about pursuing the problem in
> this form.
>
I count understand your concern; only available on a one-node plan and
may needs additional interaction between core and extension to push-
down complicated expression.
So, right now, I have to admit to rid of this hook for this purpose.

On the other hand, I thought to use similar functionality, but not
same, to implement join-replacement by custom-scan. I'd like to see
your comment prior to patch submission.

Let assume a custom-scan provider that runs on a materialized-view
(or, something like a query cache in memory) instead of join.
In this case, a reasonable design is to fetch a tuple from the
materialized-view then put it on the ecxt_scantuple of ExprContext
prior to evaluation of qualifier or tlist, unlike usual join takes
two slots - ecxt_innertuple and ecxt_outertuple.
Also, it leads individual varnode has to reference exct_scantuple,
neither ecxt_innertuple nor ecxt_outertuple.
The tuple in exct_scantuple contains attributes come from both
relations, thus, it needs to keep relationship a varattno of the
scanned tuple and its source relation where does it come from.

I intended to use the SetCustomScanRef callback to adjust varno
and varattno of the varnode that references the custom-scan node;
as if set_join_references() doing.
It does not mean a replacement of general expression by varnode,
just re-mapping of varno/varattno.

> So I remain of the opinion that we should get the CustomScan stuff out of
> setrefs processing, and also that having EXPLAIN support for such special
> variables is premature.  It's possible that after the dust settles we'd
> wind up with additions to ruleutils that look exactly like what's in this
> patch ... but I'd bet against that.
>
So, I can agree with rid of SetCustomScanRef and GetSpecialCustomVar.
However, some alternative functionality to implement the varno/varattno
remapping is needed soon.
How about your thought?

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