From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: extend pgbench expressions with functions |
Date: | 2016-03-08 15:46:59 |
Message-ID: | CA+TgmoY0KG-e6DYPg5M703S3ZCB3n4Tv+WujopO2bewS7tO2Yg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 7, 2016 at 4:58 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>> - 32-b: add double functions, including double variables
>> - 32-c: remove \setrandom support (advice to use \set + random instead)
>
> Here is a rebased version after Tom's updates, 33-b & 33-c. I also extended
> the floating point syntax to signed accept signed exponents, and changed the
> regexpr style to match Toms changes.
Having a look at 33-b, this looks pretty good now, but:
// comments are not allowed. I'd just remove the two you have.
It make no sense to exit(1) and then return 0, so don't do that. I
might write this code as:
if (pval->type == PGBT_INT)
return pval->u.ival;
Assert(pval->type == PGBT_DOUBLE);
/* do double stuff */
This would get rid of the internal-error case here altogether in favor
of testing it via an assertion.
I think that coerceToInt() should not exit(1) when an overflow occurs;
instead, I think the function should be declared as bool
coerceToInt(PgBenchValue *pval, int64 *result), and the error return
should be propagated back to the caller, which can then return false
as we do for other error cases.
I think that some of the places you've used coerceToInt() are not
appropriate. Like, for example:
+ if
(coerceToInt(rval) == 0)
+ {
+
fprintf(stderr, "division by zero\n");
+ return false;
+ }
Now, if rval is out of range of an integer, that is going to overflow
while trying to see whether it should divide by zero. Please work a
little harder here and in similar cases. Maybe add a helper function
checkIntegerEquality(PgBenchValue *, int64).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Adrian Klaver | 2016-03-08 15:54:19 | Re: Exclude pg_largeobject form pg_dump |
Previous Message | Andreas Joseph Krogh | 2016-03-08 15:46:11 | Re: Exclude pg_largeobject form pg_dump |