Re: [HACKERS] [COMMITTERS] pgsql: pgbench: Support double constants and functions.

2016-03-29 Thread Robert Haas 
On Tue, Mar 29, 2016 at 12:56 AM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Tue, Mar 29, 2016 at 9:52 AM, Robert Haas  wrote:
>>> pgbench: Support double constants and functions.
>
>> Instead of INT64_MIN and INT64_MAX, I think that it would be better to
>> use PG_INT64_MIN and PG_INT64_MAX.
>
> Indeed.
>
>> Note as well that DOUBLE is an
>> existing variable type in VS (while INTEGER is not),
>
> Ooops; that one was harder to foresee.
>
>> A way to fix compilation here is to rename those tokens to something
>> that will never conflict, like in the attached to DOUBLE_VAR and
>> INTEGER_VAR, both exprparse.y and exprparse.l need an update.
>
> Agreed, but these symbols represent constants not variables, so
> I used DOUBLE_CONST and INTEGER_CONST instead.  Pushed with those
> corrections, so that we can get back to looking at my own bugs :-(

Oops.  Thanks for cleaning up my mess.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: pgbench: Support double constants and functions.

2016-03-28 Thread Michael Paquier 
On Tue, Mar 29, 2016 at 1:56 PM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Tue, Mar 29, 2016 at 9:52 AM, Robert Haas  wrote:
>>> pgbench: Support double constants and functions.
>
>> Instead of INT64_MIN and INT64_MAX, I think that it would be better to
>> use PG_INT64_MIN and PG_INT64_MAX.
>
> Indeed.
>
>> Note as well that DOUBLE is an
>> existing variable type in VS (while INTEGER is not),
>
> Ooops; that one was harder to foresee.

Thanks for the push.

>> A way to fix compilation here is to rename those tokens to something
>> that will never conflict, like in the attached to DOUBLE_VAR and
>> INTEGER_VAR, both exprparse.y and exprparse.l need an update.
>
> Agreed, but these symbols represent constants not variables, so
> I used DOUBLE_CONST and INTEGER_CONST instead.  Pushed with those
> corrections, so that we can get back to looking at my own bugs :-(

I think I know what's going on here...
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: pgbench: Support double constants and functions.

2016-03-28 Thread Tom Lane 
Michael Paquier  writes:
> On Tue, Mar 29, 2016 at 9:52 AM, Robert Haas  wrote:
>> pgbench: Support double constants and functions.

> Instead of INT64_MIN and INT64_MAX, I think that it would be better to
> use PG_INT64_MIN and PG_INT64_MAX.

Indeed.

> Note as well that DOUBLE is an
> existing variable type in VS (while INTEGER is not),

Ooops; that one was harder to foresee.

> A way to fix compilation here is to rename those tokens to something
> that will never conflict, like in the attached to DOUBLE_VAR and
> INTEGER_VAR, both exprparse.y and exprparse.l need an update.

Agreed, but these symbols represent constants not variables, so
I used DOUBLE_CONST and INTEGER_CONST instead.  Pushed with those
corrections, so that we can get back to looking at my own bugs :-(

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: pgbench: Support double constants and functions.

2016-03-28 Thread Michael Paquier 
On Tue, Mar 29, 2016 at 9:52 AM, Robert Haas  wrote:
> pgbench: Support double constants and functions.
>
> The new functions are pi(), random(), random_exponential(),
> random_gaussian(), and sqrt().  I was worried that this would be
> slower than before, but, if anything, it actually turns out to be
> slightly faster, because we now express the built-in pgbench scripts
> using fewer lines; each \setrandom can be merged into a subsequent
> \set.

Buildfarm is complaining about a couple of things here:
  src/bin/pgbench/exprparse.c(139): error C2365: 'DOUBLE' :
redefinition; previous definition was 'typedef'
[C:\buildfarm\buildenv\HEAD\pgsql.build\pgbench.vcxproj]
  src/bin/pgbench/pgbench.c(1046): error C2065: 'INT64_MIN' :
undeclared identifier
[C:\buildfarm\buildenv\HEAD\pgsql.build\pgbench.vcxproj]
  src/bin/pgbench/exprparse.c(139): error C2086: 'yytokentype DOUBLE'
: redefinition [C:\buildfarm\buildenv\HEAD\pgsql.build\pgbench.vcxproj]
  src/bin/pgbench/pgbench.c(1046): error C2065: 'INT64_MAX' :
undeclared identifier
[C:\buildfarm\buildenv\HEAD\pgsql.build\pgbench.vcxproj]
  src/bin/pgbench/exprscan.l(131): error C2275: 'DOUBLE' : illegal use
of this type as an expression (src/bin/pgbench/exprparse.c)
[C:\buildfarm\buildenv\HEAD\pgsql.build\pgbench.vcxproj]

Instead of INT64_MIN and INT64_MAX, I think that it would be better to
use PG_INT64_MIN and PG_INT64_MAX. Note as well that DOUBLE is an
existing variable type in VS (while INTEGER is not), and it is true
that it is not directly documented:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms221627%28v=vs.85%29.aspx
A way to fix compilation here is to rename those tokens to something
that will never conflict, like in the attached to DOUBLE_VAR and
INTEGER_VAR, both exprparse.y and exprparse.l need an update.
-- 
Michael


pgbench-fix-vs.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers