Re: [v9.5] Custom Plan API

> On Sun, Aug 31, 2014 at 12:54 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
> > 2014-08-29 13:33 GMT-04:00 Robert Haas <robertmhaas@gmail.com>:
> >> Comments:
> >>
> >> 1. There seems to be no reason for custom plan nodes to have
> >> MultiExec support; I think this as an area where extensibility is
> >> extremely unlikely to work out.  The MultiExec mechanism is really
> >> only viable between closely-cooperating nodes, like Hash and
> >> HashJoin, or BitmapIndexScan, BitmapAnd, BitmapOr, and
> >> BitmapHeapScan; and arguably those things could have been written as
> a single, more complex node.
> >> Are we really going to want to support a custom plan that can
> >> substitute for a Hash or BitmapAnd node?  I really doubt that's very
> >> useful.
> >>
> > This intends to allows a particular custom-scan provider to exchange
> > its internal data when multiple custom-scan node is stacked.
> > So, it can be considered a facility to implement closely-cooperating
> > nodes; both of them are managed by same custom-scan provider.
> > An example is gpu-accelerated version of hash-join that takes
> > underlying custom-scan node that will returns a hash table with gpu
> > preferable data structure, but should not be a part of row-by-row
> interface.
> > I believe it is valuable for some use cases, even though I couldn't
> > find a use-case in ctidscan example.
> 
> Color me skeptical.  Please remove that part for now, and we can revisit
> it when, and if, a plausible use case emerges.
> 
Now, I removed the multi-exec portion from the patch set.

Existence of this interface affects to the query execution cost so much,
so I want to revisit it as soon as possible. Also see the EXPLAIN output
on the tail of this message.

> > It came from the discussion I had long time before during patch
> > reviewing of postgres_fdw. I suggested to use static table of
> > FdwRoutine but I got a point that says some compiler raise
> > error/warning to put function pointers on static initialization.
> > I usually use GCC only, so I'm not sure whether this argue is right or
> > not, even though the postgres_fdw_handler() allocates FdwRoutine using
> > palloc() then put function pointers for each.
> 
> That's odd, because aset.c has used static initializers since forever, and
> I'm sure someone would have complained by now if there were a problem with
> that usage.
> 
I reminded the discussion at that time. The GCC specific manner was not
static initialization itself, it was static initialization with field name.
Like:
  static CustomPathMethods   ctidscan_path_methods = {
      .CustomName = "ctidscan",
      .CreateCustomPlan = CreateCtidScanPlan,
      .TextOutCustomPath = TextOutCtidScanPath,
  };


Regarding to the attached three patches:
[1] custom-path and hook
It adds register_custom_path_provider() interface for registration of
custom-path entrypoint. Callbacks are invoked on set_plain_rel_pathlist
to offer alternative scan path on regular relations.
I may need to explain the terms in use. I calls the path-node custom-path
that is the previous step of population of plan-node (like custom-scan
and potentially custom-join and so on). The node object created by
CreateCustomPlan() is called custom-plan because it is abstraction for
all the potential custom-xxx node; custom-scan is the first of all.

[2] custom-scan node
It adds custom-scan node support. The custom-scan node is expected to
generate contents of a particular relation or sub-plan according to its
custom-logic.
Custom-scan provider needs to implement callbacks of CustomScanMethods
and CustomExecMethods. Once a custom-scan node is populated from
custom-path node, the backend calls back these methods in the planning
and execution stage.

[3] contrib/ctidscan
It adds a logic to scan a base relation if WHERE clause contains
inequality expression around ctid system column; that allows to skip
blocks which will be unread obviously.

During the refactoring, I noticed a few interface is omissible.
The backend can know which relation is the target of custom-scan node
being appeared in the plan-tree if its scanrelid > 0. So, I thought
ExplainCustomPlanTargetRel() and ExplainCustomPreScanNode() are
omissible, then removed from the patch.

Please check the attached ones.

--------
Also, regarding to the use-case of multi-exec interface.
Below is an EXPLAIN output of PG-Strom. It shows the custom GpuHashJoin has
two sub-plans; GpuScan and MultiHash.
GpuHashJoin is stacked on the GpuScan. It is a case when these nodes utilize
multi-exec interface for more efficient data exchange between the nodes.
GpuScan already keeps a data structure that is suitable to send to/recv from
GPU devices and constructed on the memory segment being DMA available.
If we have to form a tuple, pass it via row-by-row interface, then deform it,
it will become a major performance degradation in this use case.

postgres=# explain select * from t10 natural join t8 natural join t9 where x < 10;
                                          QUERY PLAN
-----------------------------------------------------------------------------------------------
 Custom (GpuHashJoin)  (cost=10979.56..90064.15 rows=333 width=49)
   pseudo scan tlist: 1:(t10.bid), 3:(t10.aid), 4:<t10.x>, 2:<t8.data>, 5:[t8.aid], 6:[t9.bid]
   hash clause 1: ((t8.aid = t10.aid) AND (t9.bid = t10.bid))
   ->  Custom (GpuScan) on t10  (cost=10000.00..88831.26 rows=3333327 width=16)
         Host References: aid, bid, x
         Device References: x
         Device Filter: (x < 10::double precision)
   ->  Custom (MultiHash)  (cost=464.56..464.56 rows=1000 width=41)
         hash keys: aid, bid
         ->  Hash Join  (cost=60.06..464.56 rows=1000 width=41)
               Hash Cond: (t9.data = t8.data)
               ->  Index Scan using t9_pkey on t9  (cost=0.29..357.29 rows=10000 width=37)
               ->  Hash  (cost=47.27..47.27 rows=1000 width=37)
                     ->  Index Scan using t8_pkey on t8  (cost=0.28..47.27 rows=1000 width=37)
 Planning time: 0.810 ms
(15 rows)

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