On Thu, Sep 11, 2014 at 11:24 AM, Kouhei Kaigai <kaigai@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