Add PGDLLIMPORT to enable_hashagg

Started by Metin Doslualmost 8 years ago12 messages
#1Metin Doslu
metin@citusdata.com
1 attachment(s)

Hey all,

There was already a discussion and commit for adding PGDLLIMPORT to some
variables which enables extensions to use them on Windows builds. For
reference, the previous thread:"Add PGDLLIMPORT lines to some variables".

I would like to add PGDLLIMPORT to enable_hashagg for the same reason. I'm
adding a very simple patch. Please let me know if I'm missing anything
given that this is my first patch submission.

Best,
Metin

Attachments:

add_pgdllimport-v1.patchapplication/octet-stream; name=add_pgdllimport-v1.patchDownload
From 8be09b464a2db68fad92cd940b4888173b697a2c Mon Sep 17 00:00:00 2001
From: Metin Doslu <metin@citusdata.com>
Date: Mon, 5 Feb 2018 13:03:41 +0200
Subject: [PATCH] Add PGDLLIMPORT declaration to enable_hashagg

On Windows, PGDLLIMPORT is required for variables to be
exported, for those variables to be importable by extensions.
Add PGDLLIMPORT to enable_hashagg in optimizer/cost.h.

MSDN documentation:
https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-declspec-dllexport
---
 src/include/optimizer/cost.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index d2fff76..278b746 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -61,7 +61,7 @@ extern bool enable_indexonlyscan;
 extern bool enable_bitmapscan;
 extern bool enable_tidscan;
 extern bool enable_sort;
-extern bool enable_hashagg;
+extern PGDLLIMPORT bool enable_hashagg;
 extern bool enable_nestloop;
 extern bool enable_material;
 extern bool enable_mergejoin;
-- 
2.8.4 (Apple Git-73)

#2Robert Haas
robertmhaas@gmail.com
In reply to: Metin Doslu (#1)
Re: Add PGDLLIMPORT to enable_hashagg

On Mon, Feb 5, 2018 at 6:17 AM, Metin Doslu <metin@citusdata.com> wrote:

There was already a discussion and commit for adding PGDLLIMPORT to some
variables which enables extensions to use them on Windows builds. For
reference, the previous thread:"Add PGDLLIMPORT lines to some variables".

I would like to add PGDLLIMPORT to enable_hashagg for the same reason. I'm
adding a very simple patch. Please let me know if I'm missing anything given
that this is my first patch submission.

Seems weird to do it just for enable_hashagg and not for all of the
surrounding Booleans.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3legrand legrand
legrand_legrand@hotmail.com
In reply to: Robert Haas (#2)
Re: Add PGDLLIMPORT to enable_hashagg

Hello,
I know an other extension that would need the same for:

src/include/optimizer/paths.h
geqo_threshold
enable_geqo
min_parallel_index_scan_size
min_parallel_table_scan_size

src/include/optimizer/cost.h
max_parallel_workers_per_gather
enable_hashjoin
enable_mergejoin
enable_nestloop
enable_indexonlyscan
enable_tidscan
enable_bitmapscan
enable_indexscan
enable_seqscan
disable_cost

src/include/utils/guc.h
application_name

src/include/miscadmin.h
max_worker_processes

see https://github.com/ossc-db/pg_hint_plan/issues/8

Regards
PAscal

--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

#4Robert Haas
robertmhaas@gmail.com
In reply to: legrand legrand (#3)
Re: Add PGDLLIMPORT to enable_hashagg

On Tue, Feb 6, 2018 at 3:34 PM, legrand legrand
<legrand_legrand@hotmail.com> wrote:

Hello,
I know an other extension that would need the same for:

src/include/optimizer/paths.h
geqo_threshold
enable_geqo
min_parallel_index_scan_size
min_parallel_table_scan_size

src/include/optimizer/cost.h
max_parallel_workers_per_gather
enable_hashjoin
enable_mergejoin
enable_nestloop
enable_indexonlyscan
enable_tidscan
enable_bitmapscan
enable_indexscan
enable_seqscan
disable_cost

src/include/utils/guc.h
application_name

src/include/miscadmin.h
max_worker_processes

see https://github.com/ossc-db/pg_hint_plan/issues/8

Yeah, let's get them all into one list and I'll commit the whole thing together.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In reply to: Robert Haas (#4)
Re: Add PGDLLIMPORT to enable_hashagg

On Tue, Feb 6, 2018 at 12:39 PM, Robert Haas <robertmhaas@gmail.com> wrote:

Yeah, let's get them all into one list and I'll commit the whole thing together.

+1

--
Peter Geoghegan

#6Metin Doslu
metin@citusdata.com
In reply to: Peter Geoghegan (#5)
1 attachment(s)
Re: Add PGDLLIMPORT to enable_hashagg

Hey all,

I'm attaching the updated patch, it includes

i. The list of Pascal (max_worker_processes was already with
PGDLLIMPORT, so I also added to max_parallel_workers)
ii. Some others in cost.h to make the file more readable.

Best,
Metin

Show quoted text

On Tue, Feb 6, 2018 at 10:40 PM, Peter Geoghegan <pg@bowt.ie> wrote:

On Tue, Feb 6, 2018 at 12:39 PM, Robert Haas <robertmhaas@gmail.com> wrote:

Yeah, let's get them all into one list and I'll commit the whole thing together.

+1

--
Peter Geoghegan

Attachments:

add_pgdllimport-v2.patchapplication/octet-stream; name=add_pgdllimport-v2.patchDownload
From 888dae49eaaa4348ad2e3c80703de7bdf97f96e9 Mon Sep 17 00:00:00 2001
From: Metin Doslu <metin@citusdata.com>
Date: Mon, 5 Feb 2018 13:03:41 +0200
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.

MSDN documentation:
https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-declspec-dllexport
---
 src/include/miscadmin.h       |  2 +-
 src/include/optimizer/cost.h  | 36 ++++++++++++++++++------------------
 src/include/optimizer/paths.h |  8 ++++----
 src/include/utils/guc.h       |  2 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 429c055..a4574cd 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -158,7 +158,7 @@ extern PGDLLIMPORT int NBuffers;
 extern PGDLLIMPORT int MaxBackends;
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
-extern int	max_parallel_workers;
+extern PGDLLIMPORT int max_parallel_workers;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index d2fff76..0e9f858 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -53,24 +53,24 @@ extern PGDLLIMPORT double cpu_operator_cost;
 extern PGDLLIMPORT double parallel_tuple_cost;
 extern PGDLLIMPORT double parallel_setup_cost;
 extern PGDLLIMPORT int effective_cache_size;
-extern Cost disable_cost;
-extern int	max_parallel_workers_per_gather;
-extern bool enable_seqscan;
-extern bool enable_indexscan;
-extern bool enable_indexonlyscan;
-extern bool enable_bitmapscan;
-extern bool enable_tidscan;
-extern bool enable_sort;
-extern bool enable_hashagg;
-extern bool enable_nestloop;
-extern bool enable_material;
-extern bool enable_mergejoin;
-extern bool enable_hashjoin;
-extern bool enable_gathermerge;
-extern bool enable_partition_wise_join;
-extern bool enable_parallel_append;
-extern bool enable_parallel_hash;
-extern int	constraint_exclusion;
+extern PGDLLIMPORT Cost disable_cost;
+extern PGDLLIMPORT int	max_parallel_workers_per_gather;
+extern PGDLLIMPORT bool enable_seqscan;
+extern PGDLLIMPORT bool enable_indexscan;
+extern PGDLLIMPORT bool enable_indexonlyscan;
+extern PGDLLIMPORT bool enable_bitmapscan;
+extern PGDLLIMPORT bool enable_tidscan;
+extern PGDLLIMPORT bool enable_sort;
+extern PGDLLIMPORT bool enable_hashagg;
+extern PGDLLIMPORT bool enable_nestloop;
+extern PGDLLIMPORT bool enable_material;
+extern PGDLLIMPORT bool enable_mergejoin;
+extern PGDLLIMPORT bool enable_hashjoin;
+extern PGDLLIMPORT bool enable_gathermerge;
+extern PGDLLIMPORT bool enable_partition_wise_join;
+extern PGDLLIMPORT bool enable_parallel_append;
+extern PGDLLIMPORT bool enable_parallel_hash;
+extern PGDLLIMPORT int	constraint_exclusion;
 
 extern double clamp_row_est(double nrows);
 extern double index_pages_fetched(double tuples_fetched, BlockNumber pages,
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index b6be259..4708443 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -20,10 +20,10 @@
 /*
  * allpaths.c
  */
-extern bool enable_geqo;
-extern int	geqo_threshold;
-extern int	min_parallel_table_scan_size;
-extern int	min_parallel_index_scan_size;
+extern PGDLLIMPORT bool enable_geqo;
+extern PGDLLIMPORT int	geqo_threshold;
+extern PGDLLIMPORT int	min_parallel_table_scan_size;
+extern PGDLLIMPORT int	min_parallel_index_scan_size;
 
 /* Hook for plugins to get control in set_rel_pathlist() */
 typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 77daa5a..2e03640 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -263,7 +263,7 @@ extern char *HbaFileName;
 extern char *IdentFileName;
 extern char *external_pid_file;
 
-extern char *application_name;
+extern PGDLLIMPORT char *application_name;
 
 extern int	tcp_keepalives_idle;
 extern int	tcp_keepalives_interval;
-- 
2.8.4 (Apple Git-73)

#7legrand legrand
legrand_legrand@hotmail.com
In reply to: Metin Doslu (#6)
Re: Add PGDLLIMPORT to enable_hashagg

Thank you Metin !

Regards
PAscal

--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

#8Robert Haas
robertmhaas@gmail.com
In reply to: Metin Doslu (#6)
Re: Add PGDLLIMPORT to enable_hashagg

On Wed, Feb 7, 2018 at 6:32 AM, Metin Doslu <metin@citusdata.com> wrote:

i. The list of Pascal (max_worker_processes was already with
PGDLLIMPORT, so I also added to max_parallel_workers)
ii. Some others in cost.h to make the file more readable.

Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#9Brian Cloutier
brian@citusdata.com
In reply to: Robert Haas (#8)
Re: Add PGDLLIMPORT to enable_hashagg

On Fri, Feb 9, 2018 at 1:01 PM, Robert Haas <robertmhaas@gmail.com> wrote:

Committed.

Thanks for committing this! We forgot to ask though, could you please
backport this patch to 10 and maybe even 9.6? As-is I don't think these
variables will be available until PG 11.

#10Andres Freund
andres@anarazel.de
In reply to: Brian Cloutier (#9)
Re: Add PGDLLIMPORT to enable_hashagg

Hi,

On 2018-02-16 12:57:40 -0800, Brian Cloutier wrote:

Thanks for committing this! We forgot to ask though, could you please
backport this patch to 10 and maybe even 9.6? As-is I don't think these
variables will be available until PG 11.

I think these days our policy is to do that on request, which e.g. was
done on c572599c. Could you take the relevant commit, backport it to the
relevant branches, resolve conflicts, make possibly appropriate
adaptions, and post?

Greetings,

Andres Freund

#11Brian Cloutier
brian@citusdata.com
In reply to: Andres Freund (#10)
2 attachment(s)
Re: Add PGDLLIMPORT to enable_hashagg

On Wed, Feb 21, 2018 at 10:14 AM, Andres Freund <andres@anarazel.de> wrote:

Could you take the relevant commit, backport it to the
relevant branches, resolve conflicts, make possibly appropriate
adaptions, and post?

The original commit touched some new variables and therefore didn't apply
cleanly. Attached are equivalent patches for REL_10_STABLE and
REL9_6_STABLE.

Attachments:

add-PGDLLIMPORT-REL_10_STABLE-v1.patchtext/x-patch; charset=US-ASCII; name=add-PGDLLIMPORT-REL_10_STABLE-v1.patchDownload
From 3e2c0a444a0e07792408841a629d83797ff5883a Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 9 Feb 2018 15:54:45 -0500
Subject: [PATCH] Mark assorted GUC variables as PGDLLIMPORT.

This makes life easier for extension authors.
---
 src/include/miscadmin.h       |  2 +-
 src/include/optimizer/cost.h  | 30 +++++++++++++++---------------
 src/include/optimizer/paths.h |  8 ++++----
 src/include/utils/guc.h       |  2 +-
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 6eacd2a..e76b4b9 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -158,7 +158,7 @@ extern PGDLLIMPORT int NBuffers;
 extern PGDLLIMPORT int MaxBackends;
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
-extern int	max_parallel_workers;
+extern PGDLLIMPORT int max_parallel_workers;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 63feba0..7925e4c 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -53,21 +53,21 @@ extern PGDLLIMPORT double cpu_operator_cost;
 extern PGDLLIMPORT double parallel_tuple_cost;
 extern PGDLLIMPORT double parallel_setup_cost;
 extern PGDLLIMPORT int effective_cache_size;
-extern Cost disable_cost;
-extern int	max_parallel_workers_per_gather;
-extern bool enable_seqscan;
-extern bool enable_indexscan;
-extern bool enable_indexonlyscan;
-extern bool enable_bitmapscan;
-extern bool enable_tidscan;
-extern bool enable_sort;
-extern bool enable_hashagg;
-extern bool enable_nestloop;
-extern bool enable_material;
-extern bool enable_mergejoin;
-extern bool enable_hashjoin;
-extern bool enable_gathermerge;
-extern int	constraint_exclusion;
+extern PGDLLIMPORT Cost disable_cost;
+extern PGDLLIMPORT int	max_parallel_workers_per_gather;
+extern PGDLLIMPORT bool enable_seqscan;
+extern PGDLLIMPORT bool enable_indexscan;
+extern PGDLLIMPORT bool enable_indexonlyscan;
+extern PGDLLIMPORT bool enable_bitmapscan;
+extern PGDLLIMPORT bool enable_tidscan;
+extern PGDLLIMPORT bool enable_sort;
+extern PGDLLIMPORT bool enable_hashagg;
+extern PGDLLIMPORT bool enable_nestloop;
+extern PGDLLIMPORT bool enable_material;
+extern PGDLLIMPORT bool enable_mergejoin;
+extern PGDLLIMPORT bool enable_hashjoin;
+extern PGDLLIMPORT bool enable_gathermerge;
+extern PGDLLIMPORT int	constraint_exclusion;
 
 extern double clamp_row_est(double nrows);
 extern double index_pages_fetched(double tuples_fetched, BlockNumber pages,
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 4e06b2e..f22fe0a 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -20,10 +20,10 @@
 /*
  * allpaths.c
  */
-extern bool enable_geqo;
-extern int	geqo_threshold;
-extern int	min_parallel_table_scan_size;
-extern int	min_parallel_index_scan_size;
+extern PGDLLIMPORT bool enable_geqo;
+extern PGDLLIMPORT int	geqo_threshold;
+extern PGDLLIMPORT int	min_parallel_table_scan_size;
+extern PGDLLIMPORT int	min_parallel_index_scan_size;
 
 /* Hook for plugins to get control in set_rel_pathlist() */
 typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index b89e8e8..6bcc904 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -262,7 +262,7 @@ extern char *HbaFileName;
 extern char *IdentFileName;
 extern char *external_pid_file;
 
-extern char *application_name;
+extern PGDLLIMPORT char *application_name;
 
 extern int	tcp_keepalives_idle;
 extern int	tcp_keepalives_interval;
-- 
2.7.4

add-PGDLLIMPORT-REL9_6_STABLE-v1.patchtext/x-patch; charset=US-ASCII; name=add-PGDLLIMPORT-REL9_6_STABLE-v1.patchDownload
From 4d7e9c6dd32d68e2b9d87325f5dbbedbef6ce886 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 9 Feb 2018 15:54:45 -0500
Subject: [PATCH] Mark assorted GUC variables as PGDLLIMPORT.

This makes life easier for extension authors.
---
 src/include/optimizer/cost.h  | 28 ++++++++++++++--------------
 src/include/optimizer/paths.h |  6 +++---
 src/include/utils/guc.h       |  2 +-
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 2a4df2f..214e868 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -53,20 +53,20 @@ extern PGDLLIMPORT double cpu_operator_cost;
 extern PGDLLIMPORT double parallel_tuple_cost;
 extern PGDLLIMPORT double parallel_setup_cost;
 extern PGDLLIMPORT int effective_cache_size;
-extern Cost disable_cost;
-extern int	max_parallel_workers_per_gather;
-extern bool enable_seqscan;
-extern bool enable_indexscan;
-extern bool enable_indexonlyscan;
-extern bool enable_bitmapscan;
-extern bool enable_tidscan;
-extern bool enable_sort;
-extern bool enable_hashagg;
-extern bool enable_nestloop;
-extern bool enable_material;
-extern bool enable_mergejoin;
-extern bool enable_hashjoin;
-extern int	constraint_exclusion;
+extern PGDLLIMPORT Cost disable_cost;
+extern PGDLLIMPORT int	max_parallel_workers_per_gather;
+extern PGDLLIMPORT bool enable_seqscan;
+extern PGDLLIMPORT bool enable_indexscan;
+extern PGDLLIMPORT bool enable_indexonlyscan;
+extern PGDLLIMPORT bool enable_bitmapscan;
+extern PGDLLIMPORT bool enable_tidscan;
+extern PGDLLIMPORT bool enable_sort;
+extern PGDLLIMPORT bool enable_hashagg;
+extern PGDLLIMPORT bool enable_nestloop;
+extern PGDLLIMPORT bool enable_material;
+extern PGDLLIMPORT bool enable_mergejoin;
+extern PGDLLIMPORT bool enable_hashjoin;
+extern PGDLLIMPORT int	constraint_exclusion;
 
 extern double clamp_row_est(double nrows);
 extern double index_pages_fetched(double tuples_fetched, BlockNumber pages,
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 44abe83..6048a3e 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -20,9 +20,9 @@
 /*
  * allpaths.c
  */
-extern bool enable_geqo;
-extern int	geqo_threshold;
-extern int	min_parallel_relation_size;
+extern PGDLLIMPORT bool enable_geqo;
+extern PGDLLIMPORT int	geqo_threshold;
+extern PGDLLIMPORT int	min_parallel_relation_size;
 
 /* Hook for plugins to get control in set_rel_pathlist() */
 typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 725535a..68c9110 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -263,7 +263,7 @@ extern char *HbaFileName;
 extern char *IdentFileName;
 extern char *external_pid_file;
 
-extern char *application_name;
+extern PGDLLIMPORT char *application_name;
 
 extern int	tcp_keepalives_idle;
 extern int	tcp_keepalives_interval;
-- 
2.7.4

#12Andres Freund
andres@anarazel.de
In reply to: Brian Cloutier (#11)
Re: Add PGDLLIMPORT to enable_hashagg

On 2018-02-21 11:41:31 -0800, Brian Cloutier wrote:

On Wed, Feb 21, 2018 at 10:14 AM, Andres Freund <andres@anarazel.de> wrote:

Could you take the relevant commit, backport it to the
relevant branches, resolve conflicts, make possibly appropriate
adaptions, and post?

The original commit touched some new variables and therefore didn't apply
cleanly. Attached are equivalent patches for REL_10_STABLE and
REL9_6_STABLE.

Pushed.

- Andres