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