Re: extend pgbench expressions with functions

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 22:14:40
Message-ID: CA+TgmoYdKHk2qe22=_EXC8-0W6XO8c4v4sWL17UVV=hAVBFrfQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 8, 2016 at 3:52 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> [ new patch and assorted color commentary ]

This is not acceptable:

+ /* guess double type (n for "inf",
"-inf" and "nan") */
+ if (strchr(var, '.') != NULL ||
strchr(var, 'n') != NULL)
+ {
+ double dv;
+ sscanf(var, "%lf", &dv);
+ setDoubleValue(retval, dv);
+ }
+ else
+ setIntValue(retval, strtoint64(var));

That totally breaks the error handling which someone carefully established here.

+ PgBenchValue *varg = & vargs[0];

Extra space.

+ if (!coerceToDouble(st, & vargs[0], &dval))
+ return false;

Another extra space.

- int nargs = 0;
- int64 iargs[MAX_FARGS];
- PgBenchExprLink *l = args;
+ int nargs = 0;
+ PgBenchValue vargs[MAX_FARGS];
+ PgBenchExprLink *l = args;

Completely unnecessary reindentation of the first and third lines.

+ setIntValue(retval, i < 0? -i: i);

Not project style.

Please fix the whitespace damage git diff --check complains about, and
check for other places where you haven't followed project style.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-03-08 22:16:34 Re: silent data loss with ext4 / all current versions
Previous Message Robert Haas 2016-03-08 22:03:31 Re: pgbench small bug fix