Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Apr 14, 2014 at 4:43 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah. After a fast review of the custom-scan and cache-scan patches, it
>> seems to me that my original fears are largely confirmed: the custom scan
>> patch is not going to be sufficient to allow development of any truly new
>> plan type. Yeah, you can plug in some new execution node types, but
>> actually doing anything interesting is going to require patching other
>> parts of the system.
> Without prejudice to the rest of what you said, this argument doesn't
> hold much water with me. I mean, anything that our extensibility
> mechanism doesn't support today will require new hooks, but does that
> mean we're never going to add any more hooks? I sure hope not.
No, that's not what I said. ISTM that the argument for the custom-scan
API is that it allows interesting new things to be done *without further
modifying the core code*. But the example application (cache_scan) fails
to demonstrate that, and indeed seems to be a counterexample. Whether
we'd accept cache_scan on its own merits is a separate question. The
problem for me is that custom-scan isn't showing that it can support what
was claimed without doing serious damage to modularity and maintainability
of the core code.
What this may mean is that we need more attention to refactoring of the
core code. But just removing "static" from any function that looks like
it might be handy isn't my idea of well-considered refactoring. More the
opposite in fact: if those things turn into APIs that we have to support,
it's going to kill any ability to do such refactoring.
A concrete example here is setrefs.c, whose responsibilities tend to
change from release to release. I think if we committed custom-scan
as is, we'd have great difficulty changing setrefs.c's transformations
ever again, at least if we hoped to not break users of the custom-scan
API. I'm not sure what the solution is --- but turning setrefs into
a white box instead of a black box isn't it.
regards, tom lane