Thread: Re: pgsql: Default to hidden visibility for extension libraries where possi

[ Redirecting thread to -hackers from -committers ]

On 2022-Jul-19, Tom Lane wrote:

> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

> > Do you just need to send a patch to add an exports.txt file to
> > src/pl/plpgsql/src/ for these functions?
> 
> The precedent of plpython says that PGDLLEXPORT markers are sufficient.
> But yeah, we need a list of exactly which functions need to be
> re-exposed.  I imagine pldebugger has its own needs.

A reasonable guess.  I went as far as downloading pldebugger and
compiling it, but it doesn't have a test suite of its own, so I couldn't
verify anything about it.  I did notice that plpgsql_check is calling
function load_external_function(), and that doesn't appear in
pldebugger.  I wonder if the find_rendezvous_variable business is at
play.

Anyway, the minimal patch that makes plpgsql_check tests pass is
attached.  This seems a bit random.  Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.


... oh, and:

$ postmaster -c shared_preload_libraries=plugin_debugger
2022-07-19 16:27:24.006 CEST [742142] FATAL:  cannot request additional shared memory outside shmem_request_hook

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

On Tue, Jul 19, 2022 at 04:28:07PM +0200, Alvaro Herrera wrote:
> ... oh, and:
>
> $ postmaster -c shared_preload_libraries=plugin_debugger
> 2022-07-19 16:27:24.006 CEST [742142] FATAL:  cannot request additional shared memory outside shmem_request_hook

This has been reported multiple times (including on one of my own projects),
see
https://www.postgresql.org/message-id/flat/81f82c00-8818-91f3-96fa-47976f94662b%40pm.me
for the last report.

Hi,

On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
> Anyway, the minimal patch that makes plpgsql_check tests pass is
> attached.  This seems a bit random.  Maybe it'd be better to have a
> plpgsql_internal.h with functions that are exported only for plpgsql
> itself, and keep plpgsql.h with a set of functions, all marked
> PGDLLEXPORT, that are for external use.

It does seem a bit random. But I think we probably should err on the side of
adding more declarations, rather than the oposite.

I like the plpgsql_internal.h idea, but probably done separately?

Greetings,

Andres Freund

Hi,

On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
> Anyway, the minimal patch that makes plpgsql_check tests pass is
> attached.

Do you want to commit that or should I?

Greetings,

Andres Freund

Hi,

On 2022-07-19 08:31:49 -0700, Andres Freund wrote:
> But I think we probably should err on the side of adding more
> declarations, rather than the oposite.

To see if there's other declarations that should be added, I used
https://codesearch.debian.net/search?q=load_external_function&literal=1&perpkg=1

which shows plpgsql_check and hstore_pllua. All the hstore symbols for
the latter are exported already.

Greetings,

Andres Freund

Hello

On 2022-Jul-19, Andres Freund wrote:

> On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
> > Anyway, the minimal patch that makes plpgsql_check tests pass is
> > attached.
> 
> Do you want to commit that or should I?

Done.

No immediate plans for splitting plpgsql.h, so if anyone wants to take a
stab at that, be my guest.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)