Thread: Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()
Re: [COMMITTERS] pgsql: Inline initial comparisons in TestForOldSnapshot()
From
Kevin Grittner
Date:
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()
From
Kevin Grittner
Date:
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()
From
Kevin Grittner
Date:
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
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()
From
Kevin Grittner
Date:
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
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