Horton

Thread

Commits

  1. Add PGDLLEXPORTS to some plpgsql function declarations

  2. Default to hidden visibility for extension libraries where possible

  3. Expose a few more PL/pgSQL functions to debugger plugins.

  1. pgsql: Default to hidden visibility for extension libraries where possi

    Andres Freund <andres@anarazel.de> — 2022-07-18T01:05:56Z

    Default to hidden visibility for extension libraries where possible

Until now postgres built extension libraries with global visibility, i.e.
exporting all symbols.  On the one platform where that behavior is not
natively available, namely windows, we emulate it by analyzing the input files
to the shared library and exporting all the symbols therein.

Not exporting all symbols is actually desirable, as it can improve loading
speed, reduces the likelihood of symbol conflicts and can improve intra
extension library function call performance. It also makes the non-windows
builds more similar to windows builds.

Additionally, with meson implementing the export-all-symbols behavior for
windows, turns out to be more verbose than desirable.

This patch adds support for hiding symbols by default and, to counteract that,
explicit symbol visibility annotation for compilers that support
__attribute__((visibility("default"))) and -fvisibility=hidden. That is
expected to be most, if not all, compilers except msvc (for which we already
support explicit symbol export annotations).

Now that extension library symbols are explicitly exported, we don't need to
export all symbols on windows anymore, hence remove that behavior from
src/tools/msvc. The supporting code can't be removed, as we still need to
export all symbols from the main postgres binary.

Author: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/089480c077056fc20fa8d8f5a3032a9dcf5ed812

Modified Files
--------------
configure                  | 152 +++++++++++++++++++++++++++++++++++++++++++++
configure.ac               |  13 ++++
src/Makefile.global.in     |   3 +
src/Makefile.shlib         |  13 ++++
src/include/c.h            |  13 ++--
src/include/pg_config.h.in |   3 +
src/makefiles/pgxs.mk      |   5 +-
src/tools/msvc/Project.pm  |   7 ---
src/tools/msvc/Solution.pm |   1 +
9 files changed, 198 insertions(+), 12 deletions(-)

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

    Andres Freund <andres@anarazel.de> — 2022-07-18T01:39:35Z

    Hi,

On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> Default to hidden visibility for extension libraries where possible

Looking at the odd failures, not sure what went wrong.

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2022-07-18T02:01:55Z

    On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
> On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> > Default to hidden visibility for extension libraries where possible
>
> Looking at the odd failures, not sure what went wrong.

I don't know how the configure exec bit got removed, shell history doesn't
show anything odd in that regard. I do see a mistake in locally merging the
symbol-visibility branch (leading to a crucial commit being missed). I do see
in shell history that I ran check-world without a failure just before pushing,
which should have shown the failure, but didn't.

Definitely a brown paper bag moment.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-18T02:34:42Z

    Andres Freund <andres@anarazel.de> writes:
> I don't know how the configure exec bit got removed, shell history doesn't
> show anything odd in that regard. I do see a mistake in locally merging the
> symbol-visibility branch (leading to a crucial commit being missed). I do see
> in shell history that I ran check-world without a failure just before pushing,
> which should have shown the failure, but didn't.

check-world would not have re-run configure even if it was out of date
(only config.status), so that part of it's not so surprising.

> Definitely a brown paper bag moment.

We've all been there :-)

			regards, tom lane

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

    Justin Pryzby <pryzby@telsasoft.com> — 2022-07-18T23:36:47Z

    On Sun, Jul 17, 2022 at 07:01:55PM -0700, Andres Freund wrote:
> On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
> > On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> > > Default to hidden visibility for extension libraries where possible
> >
> > Looking at the odd failures, not sure what went wrong.
> 
> I don't know how the configure exec bit got removed, shell history doesn't

I think git reflog -p would help to answer that, but I suppose you already
looked.

-- 
Justin

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

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-07-19T03:26:58Z

    Hi

po 18. 7. 2022 v 3:06 odesílatel Andres Freund <andres@anarazel.de> napsal:

> Default to hidden visibility for extension libraries where possible
>
> Until now postgres built extension libraries with global visibility, i.e.
> exporting all symbols.  On the one platform where that behavior is not
> natively available, namely windows, we emulate it by analyzing the input
> files
> to the shared library and exporting all the symbols therein.
>
> Not exporting all symbols is actually desirable, as it can improve loading
> speed, reduces the likelihood of symbol conflicts and can improve intra
> extension library function call performance. It also makes the non-windows
> builds more similar to windows builds.
>
> Additionally, with meson implementing the export-all-symbols behavior for
> windows, turns out to be more verbose than desirable.
>
> This patch adds support for hiding symbols by default and, to counteract
> that,
> explicit symbol visibility annotation for compilers that support
> __attribute__((visibility("default"))) and -fvisibility=hidden. That is
> expected to be most, if not all, compilers except msvc (for which we
> already
> support explicit symbol export annotations).
>
> Now that extension library symbols are explicitly exported, we don't need
> to
> export all symbols on windows anymore, hence remove that behavior from
> src/tools/msvc. The supporting code can't be removed, as we still need to
> export all symbols from the main postgres binary.
>
> Author: Andres Freund <andres@anarazel.de>
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Discussion:
> https://postgr.es/m/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de
>
>
Unfortunately, this commit definitely breaks plpgsql_check. Can the
following routines be visible?

    AssertVariableIsOfType(&plpgsql_build_datatype,
plpgsql_check__build_datatype_t);
    plpgsql_check__build_datatype_p = (plpgsql_check__build_datatype_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_build_datatype");

    AssertVariableIsOfType(&plpgsql_compile, plpgsql_check__compile_t);
    plpgsql_check__compile_p = (plpgsql_check__compile_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_compile");

    AssertVariableIsOfType(&plpgsql_parser_setup,
plpgsql_check__parser_setup_t);
    plpgsql_check__parser_setup_p = (plpgsql_check__parser_setup_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_parser_setup");

    AssertVariableIsOfType(&plpgsql_stmt_typename,
plpgsql_check__stmt_typename_t);
    plpgsql_check__stmt_typename_p = (plpgsql_check__stmt_typename_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_stmt_typename");

    AssertVariableIsOfType(&plpgsql_exec_get_datum_type,
plpgsql_check__exec_get_datum_type_t);
    plpgsql_check__exec_get_datum_type_p =
(plpgsql_check__exec_get_datum_type_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql",
"plpgsql_exec_get_datum_type");

    AssertVariableIsOfType(&plpgsql_recognize_err_condition,
plpgsql_check__recognize_err_condition_t);
    plpgsql_check__recognize_err_condition_p =
(plpgsql_check__recognize_err_condition_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql",
"plpgsql_recognize_err_condition");

    AssertVariableIsOfType(&plpgsql_ns_lookup, plpgsql_check__ns_lookup_t);
    plpgsql_check__ns_lookup_p = (plpgsql_check__ns_lookup_t)
        LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_ns_lookup");

Regards

Pavel




> Branch
> ------
> master
>
> Details
> -------
>
> https://git.postgresql.org/pg/commitdiff/089480c077056fc20fa8d8f5a3032a9dcf5ed812
>
> Modified Files
> --------------
> configure                  | 152
> +++++++++++++++++++++++++++++++++++++++++++++
> configure.ac               |  13 ++++
> src/Makefile.global.in     |   3 +
> src/Makefile.shlib         |  13 ++++
> src/include/c.h            |  13 ++--
> src/include/pg_config.h.in |   3 +
> src/makefiles/pgxs.mk      |   5 +-
> src/tools/msvc/Project.pm  |   7 ---
> src/tools/msvc/Solution.pm |   1 +
> 9 files changed, 198 insertions(+), 12 deletions(-)
>
>

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-07-19T13:18:01Z

    On 2022-Jul-19, Pavel Stehule wrote:

> po 18. 7. 2022 v 3:06 odesílatel Andres Freund <andres@anarazel.de> napsal:
> 
> > Default to hidden visibility for extension libraries where possible

> Unfortunately, this commit definitely breaks plpgsql_check. Can the
> following routines be visible?

Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?

plpgsql_build_datatype
plpgsql_compile
plpgsql_parser_setup
plpgsql_stmt_typename
plpgsql_exec_get_datum_type
plpgsql_recognize_err_condition
plpgsql_ns_lookup

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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-19T13:45:52Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Jul-19, Pavel Stehule wrote:
>> Unfortunately, this commit definitely breaks plpgsql_check. Can the
>> following routines be visible?

> 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.

			regards, tom lane

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-07-19T14:28:07Z

    [ 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/

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

    Julien Rouhaud <rjuju123@gmail.com> — 2022-07-19T14:45:52Z

    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.

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

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-07-19T14:47:43Z

    Hi

út 19. 7. 2022 v 16:28 odesílatel Alvaro Herrera <alvherre@alvh.no-ip.org>
napsal:

> [ 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.
>
>
I can confirm that the attached patch fixes plpgsql_check.

Thank you

Pavel




>
> ... 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/
>

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

    Andres Freund <andres@anarazel.de> — 2022-07-19T15:31:49Z

    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

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

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-07-19T15:37:11Z

    Hi

út 19. 7. 2022 v 17:31 odesílatel Andres Freund <andres@anarazel.de> napsal:

> 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.
>

This list can be extended. I think plpgsql_check is maybe one extension
that uses code from another extension directly. This is really not common
usage.


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

can be

I have not any problem with it or with exports.txt file.



> Greetings,
>
> Andres Freund
>

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

    Andres Freund <andres@anarazel.de> — 2022-07-19T15:40:04Z

    Hi,

On 2022-07-19 17:37:11 +0200, Pavel Stehule wrote:
> út 19. 7. 2022 v 17:31 odesílatel Andres Freund <andres@anarazel.de> napsal:
> 
> > 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.
> >
> 
> This list can be extended. I think plpgsql_check is maybe one extension
> that uses code from another extension directly. This is really not common
> usage.

There's a few more use cases, e.g. transform modules. Hence exposing e.g. many
plpython helpers.


> I have not any problem with it or with exports.txt file.

Just to be clear, there shouldn't be any use exports.txt here, just a few
PGDLLEXPORTs.

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2022-07-19T22:39:30Z

    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

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

    Andres Freund <andres@anarazel.de> — 2022-07-19T22:44:35Z

    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

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-07-20T09:02:47Z

    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)

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-20T13:38:50Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> No immediate plans for splitting plpgsql.h, so if anyone wants to take a
> stab at that, be my guest.

ISTM that a comment pointing out that the functions marked PGDLLEXPORT
are meant to be externally accessible should be sufficient.

I'll try to do some research later today to identify anything else
we need to mark in plpgsql.  I recall doing some work specifically
creating functions for pldebugger's use, but I'll need to dig.

			regards, tom lane

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

    Robert Haas <robertmhaas@gmail.com> — 2022-07-20T13:54:03Z

    On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ISTM that a comment pointing out that the functions marked PGDLLEXPORT
> are meant to be externally accessible should be sufficient.

The name PGDLLEXPORT is actually slightly misleading, now, because
there's no longer anything about it that is specific to DLLs.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-07-20T14:04:30Z

    On 2022-Jul-20, Tom Lane wrote:

> I'll try to do some research later today to identify anything else
> we need to mark in plpgsql.  I recall doing some work specifically
> creating functions for pldebugger's use, but I'll need to dig.

I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr.  Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.

That was also reported by Pavel.  He was concerned about plpgsql_check,
though, not pldebugger.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree.              (Don Knuth)

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-20T14:09:01Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ISTM that a comment pointing out that the functions marked PGDLLEXPORT
>> are meant to be externally accessible should be sufficient.

> The name PGDLLEXPORT is actually slightly misleading, now, because
> there's no longer anything about it that is specific to DLLs.

True, but the mess of changing it seems to outweigh any likely clarity
gain.  As long as there's adequate commentary about what it means,
I'm okay with the existing naming.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2022-07-20T14:12:43Z

    Hi,

On July 20, 2022 3:54:03 PM GMT+02:00, Robert Haas <robertmhaas@gmail.com> wrote:
>On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ISTM that a comment pointing out that the functions marked PGDLLEXPORT
>> are meant to be externally accessible should be sufficient.
>
>The name PGDLLEXPORT is actually slightly misleading, now, because
>there's no longer anything about it that is specific to DLLs.

How so? Right now it's solely used for making symbols in DLLs as exported?

Andres 
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-20T14:15:55Z

    Andres Freund <andres@anarazel.de> writes:
> On July 20, 2022 3:54:03 PM GMT+02:00, Robert Haas <robertmhaas@gmail.com> wrote:
>> The name PGDLLEXPORT is actually slightly misleading, now, because
>> there's no longer anything about it that is specific to DLLs.

> How so? Right now it's solely used for making symbols in DLLs as exported?

I suspect Robert is reading "DLL" as meaning only a Windows thing.
You're right, if you read it as a generic term for loadable libraries,
it's more or less applicable everywhere.

			regards, tom lane

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-20T14:20:04Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
> expose functions directly, but through plpgsql_plugin_ptr.  Maybe that
> one does need to be made PGDLLEXPORT, since currently it isn't.

Hm.  Not sure if the rules are the same for global variables as
they are for functions, but if so, yeah ...

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2022-07-20T15:05:00Z

    Hi, 

On July 20, 2022 4:20:04 PM GMT+02:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
>> expose functions directly, but through plpgsql_plugin_ptr.  Maybe that
>> one does need to be made PGDLLEXPORT, since currently it isn't.
>
>Hm.  Not sure if the rules are the same for global variables as
>they are for functions, but if so, yeah ...

They're the same on the export side. On windows the rules for linking to variables are stricter (they need declspec dllimport), but that doesn't matter for dlsym style stuff.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-20T15:11:51Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Jul-20, Tom Lane wrote:
>> I'll try to do some research later today to identify anything else
>> we need to mark in plpgsql.  I recall doing some work specifically
>> creating functions for pldebugger's use, but I'll need to dig.

> I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
> expose functions directly, but through plpgsql_plugin_ptr.  Maybe that
> one does need to be made PGDLLEXPORT, since currently it isn't.

After some experimentation, it does not need to be marked: pldebugger
gets at that via find_rendezvous_variable(), so there is no need for
any explicit linkage at all between plpgsql.so and plugin_debugger.so.

Along the way, I made a quick hack to get pldebugger to load into
v15/HEAD.  It lacks #ifdef's which'd be needed so that it'd still
compile against older branches, but perhaps this'll save someone
some time.

			regards, tom lane

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

    Dave Page <dpage@postgresql.org> — 2022-07-20T15:58:24Z

    On Wed, 20 Jul 2022 at 16:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > On 2022-Jul-20, Tom Lane wrote:
> >> I'll try to do some research later today to identify anything else
> >> we need to mark in plpgsql.  I recall doing some work specifically
> >> creating functions for pldebugger's use, but I'll need to dig.
>
> > I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
> > expose functions directly, but through plpgsql_plugin_ptr.  Maybe that
> > one does need to be made PGDLLEXPORT, since currently it isn't.
>
> After some experimentation, it does not need to be marked: pldebugger
> gets at that via find_rendezvous_variable(), so there is no need for
> any explicit linkage at all between plpgsql.so and plugin_debugger.so.
>
> Along the way, I made a quick hack to get pldebugger to load into
> v15/HEAD.  It lacks #ifdef's which'd be needed so that it'd still
> compile against older branches, but perhaps this'll save someone
> some time.
>

Thanks Tom - I've pushed that patch with the relevant #ifdefs added.

-- 
Dave Page
PostgreSQL Core Team
http://www.postgresql.org/