Re: Add PGDLLIMPORT lines to some variables
[ blast-from-the-past department ] Craig Ringer writes: > On 5 December 2017 at 22:49, Robert Haas wrote: >> Mark assorted variables PGDLLIMPORT. > I was going to pipe up here to add ReplicationSlotCtl to the list. > Otherwise the only way to access slot information is via the SPI and > pg_stat_replication_slots, which isn't super fun. And it's not like > ReplicationSlotCtl is any more internal than MyReplicationSlot. > I missed the boat on your commit, but ... please? Not sure why this request was ignored, but now that we have another request for the same thing [1], I've pushed this. regards, tom lane [1] https://postgr.es/m/345138875.20190611151...@cybertec.at
Re: Add PGDLLIMPORT lines to some variables
On 5 December 2017 at 22:49, Robert Haas wrote: > > Committed with these additions. Please check that I haven't messed > anything up. > > Looks good to me. For the record the commit is commit c572599c65bfe0387563233faabecd2845073538 Author: Robert Haas Date: Tue Dec 5 09:23:57 2017 -0500 Mark assorted variables PGDLLIMPORT. This makes life easier for extension authors who wish to support Windows. Brian Cloutier, slightly amended by me. Discussion: http://postgr.es/m/cajcy68fscdnhmzfps4kyo00cadkvxvea-28h-otenk-pa2o...@mail.gmail.com plus back branches. I was going to pipe up here to add ReplicationSlotCtl to the list. Otherwise the only way to access slot information is via the SPI and pg_stat_replication_slots, which isn't super fun. And it's not like ReplicationSlotCtl is any more internal than MyReplicationSlot. I missed the boat on your commit, but ... please? Patches attached. MyReplicationSlot was only made PGDLLIMPORT in 9.6, so there's one for 9.4/9.5 and one for 9.6, 10, and master. Personally I don't care about 9.4/9.5 in the slightest for this, but that's where c572599c is backpatched to. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services From de25c6aff0e07b6acf761a3b939baf2be0fa6389 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Thu, 7 Dec 2017 13:40:13 +0800 Subject: [PATCH v1] Make MyReplicationSlot and ReplicationSlotCtl PGDLLIMPORT --- src/include/replication/slot.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h index c129a4a..0fee63c 100644 --- a/src/include/replication/slot.h +++ b/src/include/replication/slot.h @@ -136,8 +136,8 @@ typedef struct ReplicationSlotCtlData /* * Pointers to shared memory */ -extern ReplicationSlotCtlData *ReplicationSlotCtl; -extern ReplicationSlot *MyReplicationSlot; +extern PGDLLIMPORT ReplicationSlotCtlData *ReplicationSlotCtl; +extern PGDLLIMPORT ReplicationSlot *MyReplicationSlot; /* GUCs */ extern PGDLLIMPORT int max_replication_slots; -- 2.9.5 From 9e7331ecb3889cbdccc3f8a3a62ac71f215bb3f6 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Thu, 7 Dec 2017 13:32:11 +0800 Subject: [PATCH v1] Mark ReplicationSlotCtl as PGDLLIMPORT --- src/include/replication/slot.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h index 0c44233..c3b7cc8 100644 --- a/src/include/replication/slot.h +++ b/src/include/replication/slot.h @@ -156,7 +156,7 @@ typedef struct ReplicationSlotCtlData /* * Pointers to shared memory */ -extern ReplicationSlotCtlData *ReplicationSlotCtl; +extern PGDLLIMPORT ReplicationSlotCtlData *ReplicationSlotCtl; extern PGDLLIMPORT ReplicationSlot *MyReplicationSlot; /* GUCs */ -- 2.9.5
Re: Add PGDLLIMPORT lines to some variables
On Tue, Dec 5, 2017 at 9:09 AM, Peter Geoghegan wrote: >> Committed with these additions. Please check that I haven't messed anything >> up. > > Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin. > Can you fix it, please? I was looking at the wrong branch. Forget it. Thanks -- Peter Geoghegan
Re: Add PGDLLIMPORT lines to some variables
On Tue, Dec 5, 2017 at 6:49 AM, Robert Haas wrote: >> RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+, >> following commit 56018bf2. I'd like to get that back to 9.4, although >> there is no reason to not include 9.3. >> >> TransactionXmin -- This is needed for the newer heap-matches-index >> verification check. Again, I would like this on 9.4+, but 9.3+ works >> too. >> >> Note that somebody asked about running it on Windows recently, and on >> one other occasion in the past. It does come up. > > Committed with these additions. Please check that I haven't messed anything > up. Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin. Can you fix it, please? -- Peter Geoghegan
Re: Add PGDLLIMPORT lines to some variables
On Mon, Dec 4, 2017 at 8:48 PM, Peter Geoghegan wrote: > On Mon, Dec 4, 2017 at 5:41 PM, Noah Misch wrote: >>> I don't think we quite have an established protocol for this. I >>> personally, but I'm biased in this specific case, is that we should >>> adopt a position that PGDLLIMPORTs should basically backpatched whenever >>> a credible extension even halfway reasonably requires it. There's no >>> easy way to get this done by default, and we're so far unwilling to just >>> slap this onto every variable. So to not further disadvantage people >>> force dto live in the MS environment, that seems the sanest >>> solution. It's not like these are high risk. >> >> +1 > > If that's going to be the policy, then I have some requests of my own. > I would like to add some PGDLLIMPORTs to suit the external version of > amcheck (the version that targets earlier versions of Postgres). These > are: > > RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+, > following commit 56018bf2. I'd like to get that back to 9.4, although > there is no reason to not include 9.3. > > TransactionXmin -- This is needed for the newer heap-matches-index > verification check. Again, I would like this on 9.4+, but 9.3+ works > too. > > Note that somebody asked about running it on Windows recently, and on > one other occasion in the past. It does come up. Committed with these additions. Please check that I haven't messed anything up. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Add PGDLLIMPORT lines to some variables
. On Mon, Dec 4, 2017 at 5:41 PM, Noah Misch wrote: >> I don't think we quite have an established protocol for this. I >> personally, but I'm biased in this specific case, is that we should >> adopt a position that PGDLLIMPORTs should basically backpatched whenever >> a credible extension even halfway reasonably requires it. There's no >> easy way to get this done by default, and we're so far unwilling to just >> slap this onto every variable. So to not further disadvantage people >> force dto live in the MS environment, that seems the sanest >> solution. It's not like these are high risk. > > +1 If that's going to be the policy, then I have some requests of my own. I would like to add some PGDLLIMPORTs to suit the external version of amcheck (the version that targets earlier versions of Postgres). These are: RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+, following commit 56018bf2. I'd like to get that back to 9.4, although there is no reason to not include 9.3. TransactionXmin -- This is needed for the newer heap-matches-index verification check. Again, I would like this on 9.4+, but 9.3+ works too. Note that somebody asked about running it on Windows recently, and on one other occasion in the past. It does come up. -- Peter Geoghegan
Re: Add PGDLLIMPORT lines to some variables
On Mon, Nov 20, 2017 at 12:02:30PM -0800, Andres Freund wrote: > On 2017-11-20 11:58:44 -0800, Brian Cloutier wrote: > > > please, append session_timezone to your list > > > > Here's a v2 patch which also includes session_timezone. > > > > Separately, is this the kind of thing which is eligible for backporting > > into the next releases of 9.6 and 10? > > I don't think we quite have an established protocol for this. I > personally, but I'm biased in this specific case, is that we should > adopt a position that PGDLLIMPORTs should basically backpatched whenever > a credible extension even halfway reasonably requires it. There's no > easy way to get this done by default, and we're so far unwilling to just > slap this onto every variable. So to not further disadvantage people > force dto live in the MS environment, that seems the sanest > solution. It's not like these are high risk. +1
Re: Add PGDLLIMPORT lines to some variables
On Wed, Nov 29, 2017 at 5:00 AM, Brian Cloutier wrote: > Sorry, I'm new to pg-hackers, so I'm not sure what the next step is. > > Do I submit this to commitfest? > > When submitting, do I submit multiple changes, one per branch this should be > packported to? If you want a patch to get reviewed, please register it to the commit fest? Here are more details: https://wiki.postgresql.org/wiki/Submitting_a_Patch Sending as a first step only one patch for HEAD is fine, as discussion will likely occur while you work on a patch. If the patch is worth back-patching, usually the committer who picks up the patch would generate the versions for the back-branches. Help is always appreciated with version-specific patches once it is clearly decided that a certain patch merits a backpatch. Note that PGDLLIMPORT does not have this treatment usually. -- Michael
Re: Add PGDLLIMPORT lines to some variables
Sorry, I'm new to pg-hackers, so I'm not sure what the next step is. Do I submit this to commitfest? When submitting, do I submit multiple changes, one per branch this should be packported to?
Re: Add PGDLLIMPORT lines to some variables
On 21 November 2017 at 04:02, Andres Freund wrote: > Hi, > > Please quote properly on postgres mailing lists... We're old school [tm]. > > On 2017-11-20 11:58:44 -0800, Brian Cloutier wrote: > > > please, append session_timezone to your list > > > > Here's a v2 patch which also includes session_timezone. > > > > Separately, is this the kind of thing which is eligible for backporting > > into the next releases of 9.6 and 10? > > I don't think we quite have an established protocol for this. I > personally, but I'm biased in this specific case, is that we should > adopt a position that PGDLLIMPORTs should basically backpatched whenever > a credible extension even halfway reasonably requires it. There's no > easy way to get this done by default, and we're so far unwilling to just > slap this onto every variable. So to not further disadvantage people > force dto live in the MS environment, that seems the sanest > solution. It's not like these are high risk. > > +1 I'm all in favour of backports to the last supported release or the first one the variable appeared in. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Add PGDLLIMPORT lines to some variables
Hi, Please quote properly on postgres mailing lists... We're old school [tm]. On 2017-11-20 11:58:44 -0800, Brian Cloutier wrote: > > please, append session_timezone to your list > > Here's a v2 patch which also includes session_timezone. > > Separately, is this the kind of thing which is eligible for backporting > into the next releases of 9.6 and 10? I don't think we quite have an established protocol for this. I personally, but I'm biased in this specific case, is that we should adopt a position that PGDLLIMPORTs should basically backpatched whenever a credible extension even halfway reasonably requires it. There's no easy way to get this done by default, and we're so far unwilling to just slap this onto every variable. So to not further disadvantage people force dto live in the MS environment, that seems the sanest solution. It's not like these are high risk. - Andres
Re: Add PGDLLIMPORT lines to some variables
> please, append session_timezone to your list Here's a v2 patch which also includes session_timezone. Separately, is this the kind of thing which is eligible for backporting into the next releases of 9.6 and 10? On Thu, Nov 16, 2017 at 10:27 PM, Pavel Stehule wrote: > Hi > > 2017-11-16 23:59 GMT+01:00 Brian Cloutier : > >> Hello hackers, >> >> I'm porting Citus to Windows and found that we use some variables which >> PG doesn't export; here is a patch which adds PGDLLIMPORT declarations to >> those variables. This is unfortunately required on Windows for extensions >> to be able to use those variables, and appears to already have been done to >> quite a few other variables. >> > > please, append session_timezone to your list > > Regards > > Pavel > > From 872408266fdc9782c6b4c6f3396423b16b86e149 Mon Sep 17 00:00:00 2001 From: Brian Cloutier Date: Tue, 31 Oct 2017 17:53:42 -0700 Subject: [PATCH] Add PGDLLIMPORT declarations to some variables On Windows, PGDLLIMPORT is required for variables to be exported, for those variables to be importable by extensions. Add PGDLLIMPORT to some variables which extensions might want to use. MSDN documentation: https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-declspec-dllexport --- src/include/access/twophase.h | 2 +- src/include/commands/extension.h| 2 +- src/include/miscadmin.h | 10 +- src/include/pgtime.h| 2 +- src/include/postmaster/postmaster.h | 4 ++-- src/include/storage/fd.h| 2 +- src/include/storage/proc.h | 4 ++-- src/include/tcop/dest.h | 2 +- src/include/tcop/tcopprot.h | 2 +- src/include/utils/guc.h | 6 +++--- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h index 54dec4e..f5fbbea 100644 --- a/src/include/access/twophase.h +++ b/src/include/access/twophase.h @@ -25,7 +25,7 @@ typedef struct GlobalTransactionData *GlobalTransaction; /* GUC variable */ -extern int max_prepared_xacts; +extern PGDLLIMPORT int max_prepared_xacts; extern Size TwoPhaseShmemSize(void); extern void TwoPhaseShmemInit(void); diff --git a/src/include/commands/extension.h b/src/include/commands/extension.h index 73bba3c..551cf2b 100644 --- a/src/include/commands/extension.h +++ b/src/include/commands/extension.h @@ -28,7 +28,7 @@ * them from the extension first. */ extern PGDLLIMPORT bool creating_extension; -extern Oid CurrentExtensionObject; +extern PGDLLIMPORT Oid CurrentExtensionObject; extern ObjectAddress CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index dad98de..1877769 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -150,14 +150,14 @@ extern PGDLLIMPORT bool IsUnderPostmaster; extern PGDLLIMPORT bool IsBackgroundWorker; extern PGDLLIMPORT bool IsBinaryUpgrade; -extern bool ExitOnAnyError; +extern PGDLLIMPORT bool ExitOnAnyError; extern PGDLLIMPORT char *DataDir; extern PGDLLIMPORT int NBuffers; -extern int MaxBackends; -extern int MaxConnections; -extern int max_worker_processes; +extern PGDLLIMPORT int MaxBackends; +extern PGDLLIMPORT int MaxConnections; +extern PGDLLIMPORT int max_worker_processes; extern int max_parallel_workers; extern PGDLLIMPORT int MyProcPid; @@ -238,7 +238,7 @@ extern PGDLLIMPORT int IntervalStyle; #define MAXTZLEN 10 /* max TZ name len, not counting tr. null */ extern bool enableFsync; -extern bool allowSystemTableMods; +extern PGDLLIMPORT bool allowSystemTableMods; extern PGDLLIMPORT int work_mem; extern PGDLLIMPORT int maintenance_work_mem; extern PGDLLIMPORT int replacement_sort_tuples; diff --git a/src/include/pgtime.h b/src/include/pgtime.h index 4fd8f75..8a13d71 100644 --- a/src/include/pgtime.h +++ b/src/include/pgtime.h @@ -70,7 +70,7 @@ extern size_t pg_strftime(char *s, size_t max, const char *format, /* these functions and variables are in pgtz.c */ -extern pg_tz *session_timezone; +extern PGDLLIMPORT pg_tz *session_timezone; extern pg_tz *log_timezone; extern void pg_timezone_initialize(void); diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 0f85908..5925160 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -16,7 +16,7 @@ /* GUC options */ extern bool EnableSSL; extern int ReservedBackends; -extern int PostPortNumber; +extern PGDLLIMPORT int PostPortNumber; extern int Unix_socket_permissions; extern char *Unix_socket_group; extern char *Unix_socket_directories; @@ -44,7 +44,7 @@ extern int postmaster_alive_fds[2]; #define POSTMASTER_FD_OWN 1 /* kept open by postmaster only */ #endif -extern const char *progname; +extern PGDLLIMPORT const char *progname; extern void PostmasterMain(int argc, char *argv[]) pg_attribute_noreturn(); extern void ClosePostmasterPorts(bool am
Re: Add PGDLLIMPORT lines to some variables
Hi 2017-11-16 23:59 GMT+01:00 Brian Cloutier : > Hello hackers, > > I'm porting Citus to Windows and found that we use some variables which PG > doesn't export; here is a patch which adds PGDLLIMPORT declarations to > those variables. This is unfortunately required on Windows for extensions > to be able to use those variables, and appears to already have been done to > quite a few other variables. > please, append session_timezone to your list Regards Pavel
