Re: [COMMITTERS] pgsql: Add transforms feature - Mailing list pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:
> On 5/3/15 2:27 PM, Tom Lane wrote:
>> 2. Preventing undefined-symbol errors at link time will hide actual coding
>> errors, not only the intended cross-plugin reference cases.  We have
>> repeatedly seen the buildfarm members that complain about this find actual
>> bugs that were missed on laxer platforms.  Therefore I think that this
>> approach is going to lead to masking bugs that we'll find only by much
>> more expensive/painful means than noticing a buildfarm failure.

> I appreciate this issue, and I have actually done quite a bit of
> research in the hope of being able to provide similar functionality on
> other platforms.

I agree that it's probably impractical to persuade toolchains to do this
if they don't want to.  But that doesn't mean that it's a good idea to
remove the whole class of error checks on platforms where it *is* possible
to make the check.  The entire reason that we have a buildfarm is, in
essence, to get the union of all checks that are possible on any supported
platform.

> Moreover, I'm not sure this error checking actually buys us much in
> practice.  A typoed symbol will be flagged by a compiler warning, and
> any undefined symbol will be caught be the test suite as soon as the
> module is loaded. So I can't imagine what those buildfarm complaints
> are, although I'd be interested look into them the analyze the
> circumstances if someone could point me to some concrete cases.

On many platforms, undefined symbols will only be complained of if you
actually try to call them, so I think this position places undue faith
in the code coverage of our buildfarm tests.  (As a counterexample,
consider SSL, which I don't think we're exercising at all in the
buildfarm because of the difficulty of setup.  And we have had bugs of
this sort versus OpenSSL, cf commit c9e1ad7fa.)

>> Surely there are other ways to deal with this requirement that don't
>> require forcing undefined symbols to be treated as valid.  For instance,
>> our existing find_rendezvous_variable() infrastructure might offer a
>> usable solution for passing a function pointer from plugin A to plugin B.

> The rendezvous variables were invented for a different problem, namely
> that you can load modules in arbitrary order, and only the first guy
> initializes a variable.  That's not the functionality that we need.

Sure it is.  Look at what plpgsql uses it for: to export a
"PLpgSQL_plugin" struct containing function pointers that might be used
by other extensions.  That seems to me to be exactly the same case as
what we're talking about here.

> Also, any such approach would likely entail casting all functions and
> variables through common pointer types, which would lose all kinds of
> compiler checking.  (All rendezvous variables are void pointers.)

Only with very poor coding style.  Properly declared function pointers
are pretty type-safe.

> It would put quite a bit of extra burden on the authors of modules such
> as hstore or postgis to not only maintain a list of exported functions
> but also test those interfaces.

Well, that actually gets to the core of my point: if a module author
exports a struct full of function pointers then he has put a stake in the
ground as to exactly what his exported API *is*.  With what you've done
for transforms, what is the exported API of hstore or plpython or plperl?
Conceivably, any non-static function in those libraries is now fair game
for somebody else to call.  We have this problem already with respect to
the core code, and it's nasty.  We should not be encouraging the same
situation to develop for extensions.

I do not understand the second part of your point.  Surely, if an
extension author expects other extensions to call function X, he's got the
same testing problem regardless of how the other extensions would reach X
exactly.
        regards, tom lane