Re: [PATCH] Magic block for modules

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> On Sun, May 07, 2006 at 08:21:43PM -0400, Tom Lane wrote:
>> That seems way overkill to me. FUNC_MAX_ARGS is good to check, but
>> most of those other things are noncritical for typical add-on modules.

> I was trying to find variables that when changed would make some things
> corrupt. For example, a changed NAMEDATALEN will make any use of the
> syscache a source of errors. A change in INDEX_MAX_KEYS will break the
> GiST interface, etc.

By that rationale you'd have to record just about every #define in the
system headers. And it still wouldn't be bulletproof --- what of
custom-modified code with, say, extra fields inserted into some widely
used struct?

But you're missing the larger point, which is that in many cases this
would be breaking stuff without any need at all. The majority of
catversion bumps, for instance, are for things that don't affect the
typical add-on module. So checking for identical catversion won't
accomplish much except to force additional recompile churn on people
doing development against CVS HEAD. The original proposal was just
to check for major PG version match. I can see checking FUNC_MAX_ARGS
too, because that has a very direct impact on the ABI that every
external function sees, but I think the cost/benefit ratio rises pretty
darn steeply after that.

Another problem with an expansive list of stuff-to-check is where does
the add-on module find it out from? AFAICS your proposal would make for
a large laundry list of random headers that every add-on would now have
to #include. If it's not defined by postgres.h or fmgr.h (which are two
things that every backend addon is surely including already) then I'm
dubious about using it in the magic block.

> Sure, I just didn't want to break every module in one weekend. I was
> thinking of adding it with LOG level now, send a message on -announce
> saying that at the beginning of the 8.2 freeze it will be an ERROR.
> Give people time to react.

I think that will just mean that we'll break every module at the start
of 8.2 freeze ;-). Unless we forget to change it to error, which IMHO
is way too likely.

regards, tom lane