Re: Custom Scan APIs (Re: Custom Plan node)

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Feb 26, 2014 at 3:02 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > The line between a foreign table and a local one is becoming blurred
> > already, but still, if this is the goal then I really think the
> > background worker is where you should be focused, not on this Custom
> > Scan API.  Consider that, once we've got proper background workers,
> > we're going to need new nodes which operate in parallel (or some other
> > rejiggering of the nodes- I don't pretend to know exactly what Robert is
> > thinking here, and I've apparently forgotten it if he's posted it
> > somewhere) and those interfaces may drive changes which would impact the
> > Custom Scan API- or worse, make us deprecate or regret having added it
> > because now we'll need to break backwards compatibility to add in the
> > parallel node capability to satisfy the more general non-GPU case.
>
> This critique seems pretty odd to me.  I haven't had the time to look
> at this patch set, but I don't see why anyone would want to use the
> background worker facility for GPU acceleration, which is what
> KaiGai's trying to accomplish here.

Eh, that didn't come out quite right.  I had intended it to be more
along the lines of "look at what Robert's doing".

I was trying to point out that parallel query execution is coming soon
thanks to the work on background worker and that parallel query
execution might drive changes to the way nodes interact in the executor
driving a need to change the API.  In other words, CustomScan could
easily end up being broken by that change and I'd rather we not have to
worry about such breakage.

> I seriously doubt there's any real conflict with parallelism here.
> Parallelism may indeed add more ways to scan a relation
> (ParallelSeqScan, ParallelIndexScan?) but that doesn't mean that we
> shouldn't have a Custom Scan node too.

What about parallel execution through the tree itself, rather than just
at specific end nodes like SeqScan and IndexScan?  Or parallel
aggregates?  I agree that simple parallel SeqScan/IndexScan isn't going
to change any of the interfaces, but surely we're going for more than
that.  Indeed, I'm wishing that I had found more time to spend on just
a simple select-based Append node which could parallelize I/O across
tablespaces and FDWs underneath the Append.

> Indeed, my principle concern
> about this patch set isn't that it's too specialized, as you seem to
> be worrying about, but that it's aiming to satisfy *too many* use
> cases.  I think FDW join pushdown is a fairly different problem from
> adding a custom scan type, and I doubt one patch should try to solve
> both problems.

Yeah, I've voiced those same concerns later in the thread also,
specifically that this punts on nearly everything and expects the
implementor to figure it all out.  We should be able to do better wrt
FDW join-pushdown, etc.
Thanks,
    Stephen