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-12 08:52:11 |
Message-ID: | alpine.DEB.2.10.1601120951560.26748@sto |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello Michaël,
> 1) When precising a negative parameter in the gaussian and exponential
> functions an assertion is triggered:
> Assertion failed: (parameter > 0.0), function getExponentialRand, file
> pgbench.c, line 494.
> Abort trap: 6 (core dumped)
> An explicit error is better.
Ok for assert -> error.
> 2) When precising a value between 0 and 2, pgbench will loop
> infinitely. For example:
> \set cid debug(random_gaussian(-100, 100, 0))
> In both cases we just lack sanity checks with PGBENCH_RANDOM,
> PGBENCH_RANDOM_EXPONENTIAL and PGBENCH_RANDOM_GAUSSIAN. I think that
> those checks would be better if moved into getExponentialRand & co
> with the assertions removed. I would personally have those functions
> return a boolean status and have the result in a pointer as a function
> argument.
ISTM that if pgbench is to be stopped, the simplest option is just to
abort with a nicer error message from the get*Rand function, there is no
need to change the function signature and transfer the error management
upwards.
> Another thing itching me is that ENODE_OPERATOR is actually useless
> now that there is a proper function layer. Removing it and replacing
> it with a set of functions would be more adapted and make the code
> simpler, at the cost of more functions and changing the parser so as
> an operator found is transformed into a function expression.
Hmmm. Why not, although the operator management is slightly more efficient
(eg always 2 operands...).
> I am attaching a patch where I tweaked a bit the docs and added some
> comments for clarity. Patch is switched to "waiting on author" for the
> time being.
Ok. I'm hesitating about removing the operator management, especially if
I'm told to put it back afterwards.
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Etsuro Fujita | 2016-01-12 09:00:02 | Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan |
Previous Message | Simon Riggs | 2016-01-12 08:26:32 | Re: Speedup twophase transactions |