Re: Custom Plan node

On Thu, Oct 3, 2013 at 3:05 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> Sorry for my late response. I've tried to investigate the planner code
> to find out the way to integrate this custom api, and it is still in
> progress.
> One special handling I found was that create_join_plan() adjust
> root->curOuterRels prior to recursion of inner tree if NestLoop.
> Probably, we need some flags to control these special handling
> in the core.
> It is a hard job to list up all the stuff, so it seems to me we need
> to check-up them during code construction...

I'm pretty sure that is only a very small tip of a very large iceberg.

> This above framework was exactly what I considered.
> Probably, we have to put a hook on functions invoked by
> set_base_rel_pathlist() to add another possible way to scan
> the provided baserel, then set_cheapest() will choose the
> most reasonable one.
> The attached patch, it's just a works-in-progress, shows
> which hook I try to put around the code. Please grep it
> with "add_custom_scan_paths".

That seems fairly reasonable to me, but I think we'd want a working
demonstration

> Regarding to the "guest module" of this framework, another
> idea that I have is, built-in query cache module that returns
> previous scan result being cached if table contents was not
> updated from the previous run. Probably, it makes sense in
> case when most of rows are filtered out in this scan.
> Anyway, I'd like to consider something useful to demonstrate
> this API.

I doubt that has any chance of working well. Supposing that query
caching is a feature we want, I don't think that plan trees are the
right place to try to install it. That sounds like something that
ought to be done before we plan and execute the query in the first
place.

>> I am a little less sanguine about the chances of a CustomJoin node
>> working out well. I agree that we need something to handle join
>> pushdown, but it seems to me that might be done by providing a Foreign
>> Scan path into the joinrel rather than by adding a concept of foreign
>> joins per se.
>>
> Indeed, if we have a hook on add_paths_to_joinrel(), it also makes
> sense for foreign tables; probably, planner will choose foreign-path
> instead of existing join node including foreign-scans.

Yes, I think it's reasonable to think about injecting a scan path into
a join node.

>> And I think that lumping everything else together under "not a scan or
>> join" has the least promise of all. Is replacing Append really the
>> same as replacing Sort? I think we'll need to think harder here about
>> what we're trying to accomplish and how to get there.
>>
> As long as extension modifies PlannedStmt on the planner_hook,
> I don't think it is not difficult so much, as I demonstrate on the
> previous patch.
> Unlike scan or join, existing code is not designed to compare
> multiple possible paths, so it seems to me a feature to adjust
> a plan-tree already construct is sufficient for most usage
> because extension can decide which one can offer more cheap
> path than built-in ones.

Well, there were a lot of problems with your demonstration, which have
already been pointed out upthread. I'm skeptical about the idea of
simply replacing planner nodes wholesale, and Tom is outright opposed.
I think you'll do better to focus on a narrower case - I'd suggest
custom scan nodes - and leave the rest as a project for another time.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company