> Kouhei Kaigai <kai...@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 <kai...@ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
