Re: [v9.5] Custom Plan API

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I'm pretty skeptical about this whole line of inquiry. We've only got
> three kinds of joins, and each one of them has quite a bit of bespoke
> logic, and all of this code is pretty performance-sensitive on large
> join nests. If there's a way to make this work for KaiGai's use case
> at all, I suspect something really lightweight like a hook, which
> should have negligible impact on other workloads, is a better fit than
> something involving system catalog access. But I might be wrong.

We do a great deal of catalog consultation already during planning,
so I think a few more wouldn't be a problem, especially if the planner
is smart enough to touch the catalogs just once (per query?) and cache
the results. However, your point about lots of bespoke logic is dead
on, and I'm afraid it's damn near a fatal objection. As just one example,
if we did not have merge joins then an awful lot of what the planner does
with path keys simply wouldn't exist, or at least would look a lot
different than it does. Without that infrastructure, I can't imagine
that a plugin approach would be able to plan mergejoins anywhere near as
effectively. Maybe there's a way around this issue, but it sure won't
just be a pg_am-like API.

> I also think that there are really two separate problems here: getting
> the executor to call a custom scan node when it shows up in the plan
> tree; and figuring out how to get it into the plan tree in the first
> place. I'm not sure we've properly separated those problems, and I'm
> not sure into which category the issues that sunk KaiGai's 9.4 patch
> fell.

I thought that the executor side of his patch wasn't in bad shape. The
real problems were in the planner, and indeed largely in the "backend"
part of the planner where there's a lot of hard-wired logic for fixing up
low-level details of the constructed plan tree. It seems like in
principle it might be possible to make that logic cleanly extensible,
but it'll likely take a major rewrite. The patch tried to skate by with
just exposing a bunch of internal functions, which I don't think is a
maintainable approach, either for the core or for the extensions using it.

regards, tom lane