Re: extend pgbench expressions with functions

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-01-29 06:40:50
Message-ID: alpine.DEB.2.10.1601290717260.718@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Michaël,

v23 attached, which does not change the message but does the other fixes.

> + if (coerceToInt(&lval) == INT64_MIN && coerceToInt(&rval) == -1)
> + {
> + fprintf(stderr, "cannot divide INT64_MIN by -1\n");
> + return false;
> + }
> Bike-shedding: "bigint out of range" to match what is done in the backend?

ISTM that it is clearer for the user to say that the issue is with the
division? Otherwise the message does not help much. Well, not that it
would be printed often...

> +/*
> + * Recursive evaluation of an expression in a pgbench script
> + * using the current state of variables.
> + * Returns whether the evaluation was ok,
> + * the value itself is returned through the retval pointer.
> + */
> Could you reformat this comment?

I can.

> fprintf(stderr,
> - "exponential parameter must be
> greater than zero (not \"%s\")\n",
> - argv[5]);
> + "exponential parameter must be greater than
> zero (not \"%s\")\n",
> + argv[5]);
> This is some unnecessary diff noise.

Indeed.

> + setIntValue(retval, getGaussianRand(thread, arg1, arg2,
> dparam));
> Some portions of the code are using tabs instead of spaces between
> function arguments.

Indeed.

> I would as well suggest fixing first the (INT64_MAX / -1) crash on HEAD
> and back-branches with something like the patch attached, inspired from
> int8.c.

I think it is overkill, but do as you feel.

Note that it must also handle modulo, but the code you suggest cannot be
used for that.

#include <stdint.h>
int main(int argc, char* argv[])
{
int64_t l = INT64_MIN;
int64_t r = -1;
int64_t d = l % r;
return 0;
}
// => Floating point exception (core dumped)

ISTM that the second condition can be simplified, as there is no possible
issue if lval is positive?

if (lval < 0 && *resval < 0) { ... }

--
Fabien.

Attachment Content-Type Size
pgbench-funcs-23.patch text/x-diff 40.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-01-29 06:51:04 Re: insufficient qualification of some objects in dump files
Previous Message Michael Paquier 2016-01-29 06:24:52 Re: insufficient qualification of some objects in dump files