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

> > >> Why does it need to know that?  I don't see that it's doing
> > >> anything that requires knowing the size of that node, and if it is,
> > >> I think it shouldn't be.  That should get delegated to the callback
> > >> provided by the custom plan provider.
> > >>
> > > Sorry, my explanation might be confusable. The create_custom_scan()
> > > does not need to know the exact size of the CustomScan (or its
> > > inheritance) because of the two separated hooks; one is node
> > > allocation time, the other is the tail of the series of initialization.
> > > If we have only one hook here, we need to have a mechanism to
> > > informs
> > > create_custom_scan() an exact size of the CustomScan node; including
> > > private fields managed by the provider, instead of the first hook on
> > > node allocation time. In this case, node allocation shall be
> > > processed by create_custom_scan() and it has to know exact size of
> > > the node to be
> > allocated.
> > >
> > > How do I implement the feature here? Is the combination of static
> > > node size and callback on the tail more simple than the existing
> > > design that takes two individual hooks on create_custom_scan()?
> >
> > I still don't get it.  Right now, the logic in create_custom_scan(),
> > which I think should really be create_custom_plan() or
> > create_plan_from_custom_path(), basically looks like this:
> >
> > 1. call hook function CreateCustomPlan 2. compute values for tlist and
> > clauses 3. pass those values to hook function InitCustomScan() 4. call
> > copy_path_costsize
> >
> > What I think we should do is:
> >
> > 1. compute values for tlist and clauses 2. pass those values to hook
> > function PlanCustomPath(), which will return a Plan 3. call
> > copy_path_costsize
> >
> > create_custom_scan() does not need to allocate the node.  You don't
> > need the node to be allocated before computing tlist and clauses.
> >
> Thanks, I could get the point.
> I'll revise the patch according to the suggestion above.
> 
At this moment, I revised the above portion of the patches.
create_custom_plan() was modified to call "PlanCustomPath" callback
next to the initialization of tlist and clauses.

It's probably same as what you suggested.

> It seems to me, we can also apply similar manner on ExecInitCustomScan().
> The current implementation doing is:
> 1. call CreateCustomScanState() to allocate a CustomScanState node 2.
> common initialization of the fields on CustomScanState, but not private
>    fields.
> 3. call BeginCustomScan() to initialize remaining stuffs and begin
> execution.
> 
> If BeginCustomScan() is re-defined to accept values for common
> initialization portions and to return a CustomScanState node, we may be
> able to eliminate the CreateCustomScanState() hook.
> 
> Unlike create_custom_scan() case, it takes more number of values for common
> initialization portions; expression tree of tlist and quals, scan and result
> tuple-slot, projection info and relation handler. It may mess up the
> interface specification.
> In addition, BeginCustomScan() has to belong to CustomScanMethods, not
> CustomexecMethods. I'm uncertain whether it is straightforward location.
> (a whisper: It may not need to be separate tables. CustomScan always
> populates CustomScanState, unlike relationship between CustomPath and
> CustomScan.)
> 
> How about your opinion to apply the above manner on ExecInitCustomScan()
> also?
> 
I kept existing implementation around ExecInitCustomScan() right now.

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