Re: Mark all GUC variable as PGDLLIMPORT

2022-05-12 Thread Michael Paquier
On Thu, May 12, 2022 at 11:59:49AM -0400, Tom Lane wrote: >> It seems pretty nonsensical to add PGDLLIMPORT to frontend only headers / >> variables. What is that supposed to mean? > > Yeah, that's why I removed it in 9a374b77. Perhaps we should try to remove it from the header itself in the long

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-12 Thread Tom Lane
Andres Freund writes: > On 2022-05-12 15:15:10 +0900, Michael Paquier wrote: >> After an extra look, PGDLLIMPORT missing from __pg_log_level looks >> like an imbroglio between 8ec5694, that has added the marking, and >> 9a374b77 that has removed it the same day. All that has been fixed in >>

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-12 Thread Andres Freund
Hi, On 2022-05-12 15:15:10 +0900, Michael Paquier wrote: > On Tue, May 10, 2022 at 04:09:47PM +0900, Michael Paquier wrote: > > Well, what about the attached then? While looking at all the headers > > in the tree, I have noticed that __pg_log_level is not marked, but > > we'd better paint a

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-12 Thread Michael Paquier
On Tue, May 10, 2022 at 04:09:47PM +0900, Michael Paquier wrote: > Well, what about the attached then? While looking at all the headers > in the tree, I have noticed that __pg_log_level is not marked, but > we'd better paint a PGDLLIMPORT also for it? This is getting used by > pgbench for some

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-10 Thread Michael Paquier
On Mon, May 09, 2022 at 09:23:47AM -0400, Robert Haas wrote: > Either of you please feel free to change these things, at least as far > as I'm concerned. Well, what about the attached then? While looking at all the headers in the tree, I have noticed that __pg_log_level is not marked, but we'd

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-09 Thread Robert Haas
On Mon, May 9, 2022 at 3:15 AM Michael Paquier wrote: > On Fri, May 06, 2022 at 04:49:24PM -0700, Andres Freund wrote: > > Just noticed that > > extern sigset_t UnBlockSig, > > BlockSig, > > StartupBlockSig; > > are unadorned. > > mark_pgdllimport.pl is

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-09 Thread Michael Paquier
On Fri, May 06, 2022 at 04:49:24PM -0700, Andres Freund wrote: > Just noticed that > extern sigset_t UnBlockSig, > BlockSig, > StartupBlockSig; > are unadorned. mark_pgdllimport.pl is not able to capture this part of the change because of this logic,

Re: Mark all GUC variable as PGDLLIMPORT

2022-05-06 Thread Andres Freund
Hi, On 2022-04-08 08:42:38 -0400, Robert Haas wrote: > On Wed, Apr 6, 2022 at 7:56 PM Michael Paquier wrote: > > On Wed, Apr 06, 2022 at 12:57:29AM +0700, John Naylor wrote: > > > For these two patches, I'd say a day or two after feature freeze is a > > > reasonable goal. > > > > Yeah. For

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-08 Thread Julien Rouhaud
On Fri, Apr 08, 2022 at 03:04:18PM +0200, Magnus Hagander wrote: > On Fri, Apr 8, 2022 at 2:42 PM Robert Haas wrote: > > > On Wed, Apr 6, 2022 at 7:56 PM Michael Paquier > > wrote: > > > On Wed, Apr 06, 2022 at 12:57:29AM +0700, John Naylor wrote: > > > > For these two patches, I'd say a day or

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-08 Thread Magnus Hagander
On Fri, Apr 8, 2022 at 2:42 PM Robert Haas wrote: > On Wed, Apr 6, 2022 at 7:56 PM Michael Paquier > wrote: > > On Wed, Apr 06, 2022 at 12:57:29AM +0700, John Naylor wrote: > > > For these two patches, I'd say a day or two after feature freeze is a > > > reasonable goal. > > > > Yeah. For

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-08 Thread Robert Haas
On Wed, Apr 6, 2022 at 7:56 PM Michael Paquier wrote: > On Wed, Apr 06, 2022 at 12:57:29AM +0700, John Naylor wrote: > > For these two patches, I'd say a day or two after feature freeze is a > > reasonable goal. > > Yeah. For patches as invasive as the PGDLLIMPORT business and the > frontend

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-06 Thread Michael Paquier
On Wed, Apr 06, 2022 at 12:57:29AM +0700, John Naylor wrote: > For these two patches, I'd say a day or two after feature freeze is a > reasonable goal. Yeah. For patches as invasive as the PGDLLIMPORT business and the frontend error refactoring, I am also fine to have two exceptions with the

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-05 Thread Tom Lane
Robert Haas writes: > Do you care whether your commit > or mine goes in first? I do not. If they're not independent, at least one of us has messed up. I have family commitments on Saturday, so if I don't get mine in on Friday it'll likely not happen before Sunday.

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-05 Thread John Naylor
On Tue, Apr 5, 2022 at 10:06 PM Robert Haas wrote: > > On Tue, Apr 5, 2022 at 10:07 AM Tom Lane wrote: > > Yeah, the frontend error message rework in [1]. That has exactly > > the same constraint that it's likely to break other open patches, > > so it'd be better to do it after the CF cutoff.

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-05 Thread Robert Haas
On Tue, Apr 5, 2022 at 10:07 AM Tom Lane wrote: > Yeah, the frontend error message rework in [1]. That has exactly > the same constraint that it's likely to break other open patches, > so it'd be better to do it after the CF cutoff. I think that doing > that concurrently with Robert's thing

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-05 Thread Tom Lane
Andrew Dunstan writes: > On 3/30/22 14:37, Robert Haas wrote: >> @RMT: Andres proposed upthread that we should plan to do this just >> after feature freeze. Accordingly I propose to commit at least 0002 >> and perhaps 0001 if people want it just after feature freeze. I >> therefore ask that the

Re: Mark all GUC variable as PGDLLIMPORT

2022-04-05 Thread Andrew Dunstan
On 3/30/22 14:37, Robert Haas wrote: > On Fri, Feb 18, 2022 at 7:02 PM Andres Freund wrote: >> On 2022-02-15 08:06:58 -0800, Andres Freund wrote: >>> The more I think about it the more I'm convinced that if we want to do this, >>> we should do it for variables and functions. >> Btw, if we were

Re: Mark all GUC variable as PGDLLIMPORT

2022-03-30 Thread Robert Haas
On Fri, Feb 18, 2022 at 7:02 PM Andres Freund wrote: > On 2022-02-15 08:06:58 -0800, Andres Freund wrote: > > The more I think about it the more I'm convinced that if we want to do this, > > we should do it for variables and functions. > > Btw, if we were to do this, we should just use

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-18 Thread Andres Freund
Hi, On 2022-02-15 08:06:58 -0800, Andres Freund wrote: > The more I think about it the more I'm convinced that if we want to do this, > we should do it for variables and functions. Btw, if we were to do this, we should just use -fvisibility=hidden everywhere and would see the same set of

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread John Naylor
On Tue, Feb 15, 2022 at 11:07 PM Andres Freund wrote: > > diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h > > index a4b5dc853b..13849a3790 100644 > > --- a/src/include/common/relpath.h > > +++ b/src/include/common/relpath.h > > @@ -56,7 +56,7 @@ typedef enum ForkNumber >

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Julien Rouhaud
Hi, Sorry for taking so much time to answer here. I definitely wanted to work on that but I was under the impression that although we now had an agreement to apply PGDLLIMPORT globally a patch itself wasn't really rushed, so I spent the last two days trying to setup a new Windows environment as

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Bruce Momjian
On Wed, Feb 16, 2022 at 01:10:50AM +0800, Julien Rouhaud wrote: > Hi, > > On Mon, Feb 14, 2022 at 12:45:08PM -0500, Robert Haas wrote: > > On Mon, Feb 14, 2022 at 12:34 PM Tom Lane wrote: > > > > > * There's a moderately sizable subset of GUCs where the underlying > > > variable is not visible

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Julien Rouhaud
Hi, On Mon, Feb 14, 2022 at 12:45:08PM -0500, Robert Haas wrote: > On Mon, Feb 14, 2022 at 12:34 PM Tom Lane wrote: > > > * There's a moderately sizable subset of GUCs where the underlying > > variable is not visible at all because it's static in guc.c. > > Typically this is because that

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 08:58:05 -0500, Robert Haas wrote: > On Mon, Feb 14, 2022 at 8:53 PM Andres Freund wrote: > > > An alternative rule which would dodge that particular issue would be > > > to just slap PGDLLIMPORT on every global variable in every header > > > file. That would arguably be a

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Robert Haas
On Mon, Feb 14, 2022 at 8:53 PM Andres Freund wrote: > > An alternative rule which would dodge that particular issue would be > > to just slap PGDLLIMPORT on every global variable in every header > > file. That would arguably be a simpler rule, though it means even more > > PGDLLIMPORT

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Andres Freund
Hi, On 2022-02-14 12:45:08 -0500, Robert Haas wrote: > On Mon, Feb 14, 2022 at 12:34 PM Tom Lane wrote: > > * If we institute a policy that all GUCs should be PGDLLIMPORT'd, > > how will we enforce that new patches follow that? I don't want to be > > endlessly going back and adding forgotten

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Robert Haas
On Mon, Feb 14, 2022 at 12:55 PM Tom Lane wrote: > Robert Haas writes: > > An alternative rule which would dodge that particular issue would be > > to just slap PGDLLIMPORT on every global variable in every header > > file. That would arguably be a simpler rule, though it means even more > >

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Tom Lane
Robert Haas writes: > An alternative rule which would dodge that particular issue would be > to just slap PGDLLIMPORT on every global variable in every header > file. That would arguably be a simpler rule, though it means even more > PGDLLIMPORT declarations floating around. Yeah, if the

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Robert Haas
On Mon, Feb 14, 2022 at 12:34 PM Tom Lane wrote: > I think you are attributing straw-man positions to me. What I'd actually > *like* is some solution that has the effect of (1) without having to mark > up our code with a bunch of Microsoft-isms. However I don't know how to > do that, and I do

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Robert Haas
On Mon, Feb 14, 2022 at 12:25 PM Chapman Flack wrote: > On 02/14/22 11:43, Robert Haas wrote: > > On Mon, Feb 14, 2022 at 10:38 AM Tom Lane wrote: > >> Robert Haas writes: > >>> I don't particularly like Chapman's solution, > > ... and (3) is an attempt at compromise that is nobody's first

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Tom Lane
Robert Haas writes: > Hmm, I guess I'd need to know who those people are in order to be able > to review their comments. I don't *like* the extra notational cruft, > but applying it inconsistently isn't better than being consistent. As > I see it, we have four choices: (1) apply PGDLLIMPORT

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Chapman Flack
On 02/14/22 11:43, Robert Haas wrote: > On Mon, Feb 14, 2022 at 10:38 AM Tom Lane wrote: >> Robert Haas writes: >>> I don't particularly like Chapman's solution, > ... and (3) is an attempt at compromise that is nobody's first choice. Ok, I guess that's )sniffle( a pretty fair way of putting

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Robert Haas
On Mon, Feb 14, 2022 at 10:38 AM Tom Lane wrote: > Robert Haas writes: > > I don't particularly like Chapman's solution, but given that you've > > repeatedly blocked every effort to just apply PGDLLIMPORT markings > > across the board, I'm not sure what the realistic alternative is. > > You do

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Tom Lane
Robert Haas writes: > I don't particularly like Chapman's solution, but given that you've > repeatedly blocked every effort to just apply PGDLLIMPORT markings > across the board, I'm not sure what the realistic alternative is. You do realize that I just have one vote in these matters? If I'm

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-14 Thread Robert Haas
On Sun, Feb 13, 2022 at 3:17 PM Tom Lane wrote: > > ... > > ObserveTypedConfigValue("log_statement_sample_rate", > > , _changed, SAMPRATE_CHANGED); > > ... > > > and will be subscribed to have the native-format value stored into samprate, > > and SAMPRATE_CHANGED ORed into gucs_changed,

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Chapman Flack
On 02/13/22 15:48, Tom Lane wrote: > much fix any worries there. Or we could provide APIs that let an > extension look up a "struct config_generic *" once and then fetch > directly using that pointer. (But I'd prefer not to, since it'd > constrain the internals more than I think is wise.) There

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Tom Lane
Andres Freund writes: > On 2022-02-13 15:36:16 -0500, Chapman Flack wrote: >> Also, I think there are some options that are only represented by >> an int, float8, etc., when shown, but whose native internal form >> is something else, like a struct. I was definitely contemplating >> that you could

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Tom Lane
Chapman Flack writes: > On 02/13/22 15:16, Tom Lane wrote: >> Why not just provide equivalents to GetConfigOption() that can >> deliver int, float8, etc values instead of strings? > And repeat the bsearch to find the option every time the interested > extension wants to check the value, even

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Andres Freund
Hi, On 2022-02-13 15:36:16 -0500, Chapman Flack wrote: > Clearly I'm not thinking here of the GUCs that are read-only views of > values that are determined some other way. How many of those are there > that are mutable, and could have their values changed without going > through the GUC

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Chapman Flack
On 02/13/22 15:16, Tom Lane wrote: > That seems like about 10X more complexity than is warranted, > not only in terms of the infrastructure required, but also in > the intellectual complexity around "just when could that value > change?" > > Why not just provide equivalents to GetConfigOption()

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Tom Lane
Chapman Flack writes: > On 02/13/22 02:29, Julien Rouhaud wrote: >> Maybe we could have an actually usable GUC API to retrieve values in their >> native format rather than C string for instance, that we could make sure also >> works for cases like max_backend? > I proposed a sketch of such an

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-13 Thread Chapman Flack
Hi, On 02/13/22 02:29, Julien Rouhaud wrote: > Maybe we could have an actually usable GUC API to retrieve values in their > native format rather than C string for instance, that we could make sure also > works for cases like max_backend? I proposed a sketch of such an API for discussion back in

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-12 Thread Julien Rouhaud
Hi, On Sat, Feb 12, 2022 at 07:59:56PM -0800, Andres Freund wrote: > > On 2021-08-23 14:53:34 +0800, Julien Rouhaud wrote: > > So since the non currently explicitly exported GUC global variables > > shouldn't > > be accessible by third-party code, I'm attaching a POC patch that does the > >

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-12 Thread Andres Freund
Hi, On 2021-08-23 14:53:34 +0800, Julien Rouhaud wrote: > So since the non currently explicitly exported GUC global variables shouldn't > be accessible by third-party code, I'm attaching a POC patch that does the > opposite of v1: enforce that restriction using a new pg_attribute_hidden() >

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-27 Thread Craig Ringer
On Fri, 27 Aug 2021 at 08:59, Julien Rouhaud wrote: > On Fri, Aug 27, 2021 at 3:42 AM Magnus Hagander > wrote: > > > > Yeah, but that does move the problem to the other side doesn't it? So > > if you (as a pure test of course) were to remove the variable > > completely from the included header

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-27 Thread Craig Ringer
On Thu, 26 Aug 2021 at 01:51, Alvaro Herrera wrote: > On 2021-Aug-25, Magnus Hagander wrote: > > > The thing we need the PGDLLIMPORT definition for is to *import* them > > on the other end? > > Oh ... so modules that are willing to cheat can include their own > declarations of the variables they

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-27 Thread Craig Ringer
On Wed, 25 Aug 2021 at 22:36, Magnus Hagander wrote: > On Wed, Aug 25, 2021 at 4:06 PM Robert Haas wrote: > > > > On Tue, Aug 24, 2021 at 5:06 PM Chapman Flack > wrote: > > > The thing is, I think I have somewhere a list of all the threads on > this > > > topic that I've read through since the

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-27 Thread Craig Ringer
On Wed, 25 Aug 2021 at 03:13, Robert Haas wrote: > On Tue, Aug 24, 2021 at 2:52 PM Chapman Flack > wrote: > > I don't think that's true of the second proposal in [0]. I don't foresee > > a noticeable runtime cost unless there is a plausible workload that > > involves very frequent updates to

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-26 Thread Julien Rouhaud
On Fri, Aug 27, 2021 at 3:42 AM Magnus Hagander wrote: > > Yeah, but that does move the problem to the other side doesn't it? So > if you (as a pure test of course) were to remove the variable > completely from the included header and just declare it manually with > PGDLLSPEC in your file, it

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-26 Thread Michael Paquier
On Thu, Aug 26, 2021 at 05:10:39PM -0400, Andrew Dunstan wrote: > On 8/26/21 3:57 PM, Robert Haas wrote: >> On Thu, Aug 26, 2021 at 3:42 PM Magnus Hagander wrote: >>> Ugly as it is, I wonder if there's a chance we could just process all >>> the headers at install times and inject the PGDLLIMPORT.

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-26 Thread Andrew Dunstan
On 8/26/21 3:57 PM, Robert Haas wrote: > On Thu, Aug 26, 2021 at 3:42 PM Magnus Hagander wrote: >> Ugly as it is, I wonder if there's a chance we could just process all >> the headers at install times and inject the PGDLLIMPORT. We know which >> symvols it is on account of what we're getting in

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-26 Thread Robert Haas
On Thu, Aug 26, 2021 at 3:42 PM Magnus Hagander wrote: > Ugly as it is, I wonder if there's a chance we could just process all > the headers at install times and inject the PGDLLIMPORT. We know which > symvols it is on account of what we're getting in the DEF file. > > Not saying that's not a

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-26 Thread Magnus Hagander
On Thu, Aug 26, 2021 at 3:38 AM Julien Rouhaud wrote: > > On Thu, Aug 26, 2021 at 1:51 AM Alvaro Herrera > wrote: > > > > On 2021-Aug-25, Magnus Hagander wrote: > > > > > The thing we need the PGDLLIMPORT definition for is to *import* them > > > on the other end? > > > > Oh ... so modules that

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Bruce Momjian
On Wed, Aug 25, 2021 at 10:41:14AM -0400, Tom Lane wrote: > Magnus Hagander writes: > > On Wed, Aug 25, 2021 at 4:06 PM Robert Haas wrote: > >> It does tend to be controversial, but I think that's basically only > >> because Tom Lane has reservations about it. I think if Tom dropped his > >>

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Julien Rouhaud
On Thu, Aug 26, 2021 at 1:51 AM Alvaro Herrera wrote: > > On 2021-Aug-25, Magnus Hagander wrote: > > > The thing we need the PGDLLIMPORT definition for is to *import* them > > on the other end? > > Oh ... so modules that are willing to cheat can include their own > declarations of the variables

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Alvaro Herrera
On 2021-Aug-25, Magnus Hagander wrote: > The thing we need the PGDLLIMPORT definition for is to *import* them > on the other end? Oh ... so modules that are willing to cheat can include their own declarations of the variables they need, and mark them __declspec (dllimport)? -- Álvaro Herrera

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Magnus Hagander
On Wed, Aug 25, 2021 at 4:41 PM Tom Lane wrote: > > Magnus Hagander writes: > > On Wed, Aug 25, 2021 at 4:06 PM Robert Haas wrote: > >> It does tend to be controversial, but I think that's basically only > >> because Tom Lane has reservations about it. I think if Tom dropped his > >> opposition

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Tom Lane
Magnus Hagander writes: > On Wed, Aug 25, 2021 at 4:06 PM Robert Haas wrote: >> It does tend to be controversial, but I think that's basically only >> because Tom Lane has reservations about it. I think if Tom dropped his >> opposition to this, nobody else would really care. And I think that >>

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Magnus Hagander
On Wed, Aug 25, 2021 at 4:06 PM Robert Haas wrote: > > On Tue, Aug 24, 2021 at 5:06 PM Chapman Flack wrote: > > The thing is, I think I have somewhere a list of all the threads on this > > topic that I've read through since the first time I had to come with my own > > hat in hand asking for a

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-25 Thread Robert Haas
On Tue, Aug 24, 2021 at 5:06 PM Chapman Flack wrote: > The thing is, I think I have somewhere a list of all the threads on this > topic that I've read through since the first time I had to come with my own > hat in hand asking for a PGDLLIMPORT on something, years ago now, and > I don't think I

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Noah Misch
On Tue, Aug 24, 2021 at 05:06:54PM -0400, Chapman Flack wrote: > On 08/24/21 16:31, Robert Haas wrote: > > about adding PGDLLIMPORT, which ought to be totally uncontroversial, > > The thing is, I think I have somewhere a list of all the threads on this > topic that I've read through since the

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Bruce Momjian
On Tue, Aug 24, 2021 at 04:31:32PM -0400, Robert Haas wrote: > On Tue, Aug 24, 2021 at 3:36 PM Chapman Flack wrote: > > Peter may have advocated for that kind of across-the-board adoption; > > my leaning is more to add an API that /can/ be adopted, initially with > > separately-linked extensions

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Chapman Flack
On 08/24/21 16:31, Robert Haas wrote: > about adding PGDLLIMPORT, which ought to be totally uncontroversial, The thing is, I think I have somewhere a list of all the threads on this topic that I've read through since the first time I had to come with my own hat in hand asking for a PGDLLIMPORT

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Robert Haas
On Tue, Aug 24, 2021 at 3:36 PM Chapman Flack wrote: > Peter may have advocated for that kind of across-the-board adoption; > my leaning is more to add an API that /can/ be adopted, initially with > separately-linked extensions as the audience. Nothing would stop it being > used in core as well,

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Chapman Flack
On 08/24/21 15:12, Robert Haas wrote: > I find it hard to > believe that we would seriously consider replacing every direct GUC > reference in the backend with something that goes through an API. Peter may have advocated for that kind of across-the-board adoption; my leaning is more to add an API

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Robert Haas
On Tue, Aug 24, 2021 at 2:52 PM Chapman Flack wrote: > I don't think that's true of the second proposal in [0]. I don't foresee > a noticeable runtime cost unless there is a plausible workload that > involves very frequent updates to GUC settings that are also of interest > to a bunch of

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Chapman Flack
On 08/24/21 14:28, Robert Haas wrote: > cost would, I think, be quite terrible. If you really had to force > everything through an API, I think what you'd want to do is define an > API where code can look up a handle object for a GUC using the name of > the GUC, and then hold onto a pointer to

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Robert Haas
On Tue, Aug 24, 2021 at 4:28 AM Peter Eisentraut wrote: > If there were an API, then in-core code should use it as well. ...which is presumably never going to happen, because the performance cost would, I think, be quite terrible. If you really had to force everything through an API, I think

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-24 Thread Peter Eisentraut
On 23.08.21 16:47, Julien Rouhaud wrote: On Mon, Aug 23, 2021 at 10:36 PM Bruce Momjian wrote: So the problem is that extensions only _need_ to use that API on Windows, so many initially don't, or that the API is too limited? The inconvenience with that API is that it's only returning c

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Mon, 23 Aug 2021 at 22:45, Julien Rouhaud wrote: > On Mon, Aug 23, 2021 at 10:22 PM Tom Lane wrote: > > > > Bruce Momjian writes: > > > On Mon, Aug 23, 2021 at 10:15:04AM -0400, Tom Lane wrote: > > >> By that argument, *every* globally-visible variable should be marked > > >> PGDLLIMPORT.

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Tue, 24 Aug 2021 at 05:08, Chapman Flack wrote: > On 08/23/21 14:30, Robert Haas wrote: > > it seems likely that this proposed > > API would have the exact same problem, because it would let people do > > exactly the same thing. And, going through this proposed API would > > still be

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Tue, 24 Aug 2021 at 13:21, Craig Ringer wrote: > > There is not even the thinnest pretense of Pg having a dedicated extension > API or any sort of internal vs public API separation. > Oh, and if we do want such a separation, we'll need to introduce a MUCH lower-pain-and-overhead way to get

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Tue, 24 Aug 2021 at 02:31, Robert Haas wrote: > On Mon, Aug 23, 2021 at 11:40 AM Alvaro Herrera > wrote: > > In that case, why not improve the API with functions that return the > > values in some native datatype? For scalars with native C types (int, > > floats, Boolean etc) this is easy

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Mon, 23 Aug 2021 at 22:15, Tom Lane wrote: > Bruce Momjian writes: > > On Sun, Aug 22, 2021 at 09:29:01PM +0800, Julien Rouhaud wrote: > >> Then shouldn't we try to prevent direct access on all platforms rather > than > >> only one? > > > Agreed. If Julian says 99% of the non-export

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Craig Ringer
On Sun, 22 Aug 2021 at 21:29, Julien Rouhaud wrote: > On Sun, Aug 22, 2021 at 09:19:42AM -0400, Tom Lane wrote: > > > > Uh, no, it's exactly *not* clear. There are a lot of GUCs that are only > > of interest to particular subsystems. I do not see why being a GUC makes > > something

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Julien Rouhaud
On Tue, Aug 24, 2021 at 12:36 PM Pavel Stehule wrote: > > There are few GUC variables, where we need extra fast access - work_mem, > encoding settings, and maybe an application name. For others I hadn't needed > to access it for over 20 years. But I understand that more complex extensions >

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Pavel Stehule
po 23. 8. 2021 v 23:08 odesílatel Chapman Flack napsal: > On 08/23/21 14:30, Robert Haas wrote: > > it seems likely that this proposed > > API would have the exact same problem, because it would let people do > > exactly the same thing. And, going through this proposed API would > > still be

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Chapman Flack
On 08/23/21 14:30, Robert Haas wrote: > it seems likely that this proposed > API would have the exact same problem, because it would let people do > exactly the same thing. And, going through this proposed API would > still be significantly more expensive than just accessing the bare > variables,

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread David Fetter
On Mon, Aug 23, 2021 at 10:56:52AM -0400, Robert Haas wrote: > On Mon, Aug 23, 2021 at 10:15 AM Tom Lane wrote: > > And yes, I absolutely would prohibit extensions from accessing many > > of them, if there were a reasonable way to do it. It would be a good > > start towards establishing a

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Peter Geoghegan
On Mon, Aug 23, 2021 at 7:57 AM Robert Haas wrote: > In short, +1 from me for the original proposal of marking all GUCs as > PGDLLIMPORT. +1 > And, heck, +1 for marking all the other global variables > that way, too. We're not solving any problem here. We're just annoying > people, mostly

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Robert Haas
On Mon, Aug 23, 2021 at 11:40 AM Alvaro Herrera wrote: > In that case, why not improve the API with functions that return the > values in some native datatype? For scalars with native C types (int, > floats, Boolean etc) this is easy enough; I bet it'll solve 99% of the > problems or more.

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Chapman Flack
On 08/23/21 10:57, Chapman Flack wrote: > Maybe those cases aren't very numerous ... and maybe they're distinctive > enough to recognize when creating one ("hmm, I am creating a check or > assign hook that does significant work here, will it be worth exposing > a getter API for the product of the

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Alvaro Herrera
On 2021-Aug-23, Robert Haas wrote: > It's also a bit unfair to say, well we have APIs for accessing GUC > values. It's true that we do. But if the GUC variable is, say, a > Boolean, you do not want your extension to call some function that > does a bunch of shenanigans and returns a string so

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Chapman Flack
On 08/23/21 10:36, Bruce Momjian wrote: > So the problem is that extensions only _need_ to use that API on > Windows, so many initially don't, or that the API is too limited? I think there can be cases where it's too limited, such as when significant computation or validation is needed between

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Robert Haas
On Mon, Aug 23, 2021 at 10:15 AM Tom Lane wrote: > And yes, I absolutely would prohibit extensions from accessing many > of them, if there were a reasonable way to do it. It would be a good > start towards establishing a defined API for extensions. Mostly, it would make extension development

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Julien Rouhaud
On Mon, Aug 23, 2021 at 10:15 PM Tom Lane wrote: > > And yes, I absolutely would prohibit extensions from accessing many > of them, if there were a reasonable way to do it. It would be a good > start towards establishing a defined API for extensions. The v2 patch I sent does that, at least when

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Julien Rouhaud
On Mon, Aug 23, 2021 at 10:36 PM Bruce Momjian wrote: > > So the problem is that extensions only _need_ to use that API on > Windows, so many initially don't, or that the API is too limited? The inconvenience with that API is that it's only returning c strings, so you gave to convert it back to

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Julien Rouhaud
On Mon, Aug 23, 2021 at 10:22 PM Tom Lane wrote: > > Bruce Momjian writes: > > On Mon, Aug 23, 2021 at 10:15:04AM -0400, Tom Lane wrote: > >> By that argument, *every* globally-visible variable should be marked > >> PGDLLIMPORT. But the mere fact that two backend .c files need to access > > >

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Bruce Momjian
On Mon, Aug 23, 2021 at 10:22:51AM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Mon, Aug 23, 2021 at 10:15:04AM -0400, Tom Lane wrote: > >> By that argument, *every* globally-visible variable should be marked > >> PGDLLIMPORT. But the mere fact that two backend .c files need to access >

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Tom Lane
Bruce Momjian writes: > On Mon, Aug 23, 2021 at 10:15:04AM -0400, Tom Lane wrote: >> By that argument, *every* globally-visible variable should be marked >> PGDLLIMPORT. But the mere fact that two backend .c files need to access > No, Julien says 99% need only the GUCs, so that is not the

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Bruce Momjian
On Mon, Aug 23, 2021 at 10:15:04AM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Sun, Aug 22, 2021 at 09:29:01PM +0800, Julien Rouhaud wrote: > >> Then shouldn't we try to prevent direct access on all platforms rather than > >> only one? > > > Agreed. If Julian says 99% of the

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Tom Lane
Bruce Momjian writes: > On Sun, Aug 22, 2021 at 09:29:01PM +0800, Julien Rouhaud wrote: >> Then shouldn't we try to prevent direct access on all platforms rather than >> only one? > Agreed. If Julian says 99% of the non-export problems are GUCs, and we > can just export them all, why not do it?

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Bruce Momjian
On Sun, Aug 22, 2021 at 09:29:01PM +0800, Julien Rouhaud wrote: > On Sun, Aug 22, 2021 at 09:19:42AM -0400, Tom Lane wrote: > > > > Uh, no, it's exactly *not* clear. There are a lot of GUCs that are only > > of interest to particular subsystems. I do not see why being a GUC makes > > something

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-23 Thread Julien Rouhaud
On Sun, Aug 22, 2021 at 09:29:01PM +0800, Julien Rouhaud wrote: > On Sun, Aug 22, 2021 at 09:19:42AM -0400, Tom Lane wrote: > > > > Uh, no, it's exactly *not* clear. There are a lot of GUCs that are only > > of interest to particular subsystems. I do not see why being a GUC makes > > something

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Julien Rouhaud
On Sun, Aug 22, 2021 at 09:19:42AM -0400, Tom Lane wrote: > > Uh, no, it's exactly *not* clear. There are a lot of GUCs that are only > of interest to particular subsystems. I do not see why being a GUC makes > something automatically more interesting than any other global variable. > Usually,

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Tom Lane
Julien Rouhaud writes: > On Sun, Aug 22, 2021 at 08:51:26PM +0900, Michael Paquier wrote: >> ... and on a parameter-basis based >> on requests from extension authors. If you wish to make your >> extensions able to work on Windows, that's a good idea, but I would >> recommend to limit this

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Julien Rouhaud
On Sun, Aug 22, 2021 at 02:17:16PM +0200, Pavel Stehule wrote: > > Is true, so it increases differences between Windows and Unix, and fixing > these issues is not fun work. On the other hand, maybe direct access to > these variables from extensions is an antipattern, and we should use a > direct

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Pavel Stehule
ne 22. 8. 2021 v 14:08 odesílatel Julien Rouhaud napsal: > On Sun, Aug 22, 2021 at 08:51:26PM +0900, Michael Paquier wrote: > > On Sun, Aug 22, 2021 at 04:10:33PM +0800, Julien Rouhaud wrote: > > > This topic has been raised multiple time over the years, and I don't > see any > > > objection to

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Julien Rouhaud
On Sun, Aug 22, 2021 at 08:51:26PM +0900, Michael Paquier wrote: > On Sun, Aug 22, 2021 at 04:10:33PM +0800, Julien Rouhaud wrote: > > This topic has been raised multiple time over the years, and I don't see any > > objection to add such an annotation at least for all GUC variables (either > >

Re: Mark all GUC variable as PGDLLIMPORT

2021-08-22 Thread Michael Paquier
On Sun, Aug 22, 2021 at 04:10:33PM +0800, Julien Rouhaud wrote: > This topic has been raised multiple time over the years, and I don't see any > objection to add such an annotation at least for all GUC variables (either the > direct variables or the indirect variables set during the hook

  1   2   >