Обсуждение: Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

Поиск
Список
Период
Сортировка

Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Kevin Grittner
Дата:
On Thu, Apr 21, 2016 at 8:47 AM, Kevin Grittner <kgrittn@postgresql.org> wrote:
> Inline initial comparisons in TestForOldSnapshot()

This seems to have broken Windows builds.  Is there something
special that needs to be done if a function referenced from a
contrib module changes to an inline function?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Kevin Grittner
Дата:
On Thu, Apr 21, 2016 at 9:58 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Thu, Apr 21, 2016 at 8:47 AM, Kevin Grittner <kgrittn@postgresql.org> wrote:
>> Inline initial comparisons in TestForOldSnapshot()
>
> This seems to have broken Windows builds.  Is there something
> special that needs to be done if a function referenced from a
> contrib module changes to an inline function?

I've attempted to fix this by adding an include to blscan.c so that
it can find old_snapshot_threshold.  The fact that this was only
failing in a contrib module on Windows builds has me suspicious
that there is some other way this would be better fixed, but I have
no clue what that would be.  We'll see whether this brings things
green again.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Kevin Grittner
Дата:
On Thu, Apr 21, 2016 at 11:58 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Thu, Apr 21, 2016 at 9:58 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
>> On Thu, Apr 21, 2016 at 8:47 AM, Kevin Grittner <kgrittn@postgresql.org> wrote:
>>> Inline initial comparisons in TestForOldSnapshot()
>>
>> This seems to have broken Windows builds.  Is there something
>> special that needs to be done if a function referenced from a
>> contrib module changes to an inline function?
>
> I've attempted to fix this by adding an include to blscan.c so that
> it can find old_snapshot_threshold.  The fact that this was only
> failing in a contrib module on Windows builds has me suspicious
> that there is some other way this would be better fixed, but I have
> no clue what that would be.  We'll see whether this brings things
> green again.

No joy with that shot in the dark.

Should I add PGDLLEXPORT to the declaration of
old_snapshot_threshold.  That doesn't seem to be documented
anywhere, or have any useful comments; but it looks like it *might*
be the arcane incantation needed here.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Tom Lane
Дата:
Kevin Grittner <kgrittn@gmail.com> writes:
> No joy with that shot in the dark.

> Should I add PGDLLEXPORT to the declaration of
> old_snapshot_threshold.  That doesn't seem to be documented
> anywhere, or have any useful comments; but it looks like it *might*
> be the arcane incantation needed here.

PGDLLIMPORT is what's needed on any backend global variable that's to
be referenced by extensions.  I already pushed a fix before noticing
this thread.
        regards, tom lane



Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Kevin Grittner
Дата:
On Thu, Apr 21, 2016 at 1:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> PGDLLIMPORT is what's needed on any backend global variable that's to
> be referenced by extensions.  I already pushed a fix before noticing
> this thread.

Thanks!

Since I failed to find anything in our docs or C comments, and very
scant clues in the Wiki and list archives, about when to use
PGDLLIMPORT and PGDLLEXPORT I figured it might be helpful to
clarify here, and maybe add something near one of the definitions.

Based on your fix and the meager clues found elsewhere, along with
randomly looking at a few points of existing usage, I infer that
these should be specified from the perspective of loadable modules
which we might want to build for Windows.  That is, an extension
would use PGDLLEXPORT for symbols it wanted to have the backends
find, and core code should use PGDLLIMPORT for symbols that
extensions or other loadable modules might need to find.  These are
used for both data locations and function names.

Close?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()

От
Tom Lane
Дата:
Kevin Grittner <kgrittn@gmail.com> writes:
> Since I failed to find anything in our docs or C comments, and very
> scant clues in the Wiki and list archives, about when to use
> PGDLLIMPORT and PGDLLEXPORT I figured it might be helpful to
> clarify here, and maybe add something near one of the definitions.

> Based on your fix and the meager clues found elsewhere, along with
> randomly looking at a few points of existing usage, I infer that
> these should be specified from the perspective of loadable modules
> which we might want to build for Windows.  That is, an extension
> would use PGDLLEXPORT for symbols it wanted to have the backends
> find, and core code should use PGDLLIMPORT for symbols that
> extensions or other loadable modules might need to find.  These are
> used for both data locations and function names.

I resolutely refuse to become an expert on such things, but from existing
usage it seems we only need PGDLLIMPORT on the "extern"s for core global
variables that need to be accessible to extensions.  If there is a
corresponding requirement for core functions, it must be getting handled
automatically somewhere in the Windows build systems.

There are no manual uses of PGDLLEXPORT anywhere in our tree --- it's
plastered on automatically by PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1,
and that's all.  I rather suspect that this is cargo-cult programming
and those uses are not really necessary, because if they are, how do V0
functions in extensions work?  But as long as it's out of sight I don't
particularly care if they're there or not.
        regards, tom lane