On Thu, Sep 11, 2014 at 11:24 AM, Kouhei Kaigai <kai...@ak.jp.nec.com> wrote: >> Don't the changes to src/backend/optimizer/plan/createplan.c belong in >> patch #2? >> > The borderline between #1 and #2 is little bit bogus. So, I moved most of > portion into #1, however, invocation of InitCustomScan (that is a callback > in CustomPlanMethod) in create_custom_plan() is still in #2.
Eh, create_custom_scan() certainly looks like it is in #1 from here, or at least part of it is. It calculates tlist and clauses and then does nothing with them. That clearly can't be the right division. I think it would make sense to have create_custom_scan() compute tlist and clauses first, and then pass those to CreateCustomPlan(). Then you don't need a separate InitCustomScan() - which is misnamed anyway, since it has nothing to do with ExecInitCustomScan(). > OK, I revised. Now custom-scan assumes it has a particular valid relation > to be scanned, so no code path with scanrelid == 0 at this moment. > > Let us revisit this scenario when custom-scan replaces relation-joins. > In this case, custom-scan will not be associated with a particular base- > relation, thus it needs to admit a custom-scan node with scanrelid == 0. Yeah, I guess the question there is whether we'll want let CustomScan have scanrelid == 0 or require that CustomJoin be used there instead. >> Why can't the Custom(GpuHashJoin) node build the hash table internally >> instead of using a separate node? >> > It's possible, however, it prevents to check sub-plans using EXPLAIN if we > manage inner-plans internally. So, I'd like to have a separate node being > connected to the inner-plan. Isn't that just a matter of letting the EXPLAIN code print more stuff? Why can't it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers