Add PGDLLIMPORT to enable_hashagg

Started by Metin Dosluabout 8 years ago12 messageshackers
Jump to latest
#1Metin Doslu
metin@citusdata.com

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+1-2
#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)
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+24-25
#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)
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+21-22
add-PGDLLIMPORT-REL9_6_STABLE-v1.patchtext/x-patch; charset=US-ASCII; name=add-PGDLLIMPORT-REL9_6_STABLE-v1.patchDownload+18-19
#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