On Tue, Dec 22, 2015 at 12:16 AM, Fabien COELHO <coe...@cri.ensmp.fr> wrote:
> So you would just like to remove double double(int) and double sqrt(double)
> from the patch, and basically that is all? int int(double)??
> debug??? (hmmm, useful debugging a non trivial expression).
OK, so I am finally back on this item, and after a close look at the
code I am throwing away my concerns.
- if (!evaluateExpr(st, expr, &result))
+ if (!evaluateExpr(thread, st, expr, &result))
{
st->ecnt++;
return true;
}
- sprintf(res, INT64_FORMAT, result);
+ sprintf(res, INT64_FORMAT, INT(result));
Based on that all the final results of a \set command will have an
integer format, still after going through this patch, allowing double
as return type for nested function calls (first time "nested" is
written on this thread) is actually really useful, and that's what
makes sense for this feature. I am not sure why I haven't thought
about that before as well... So, even if for example the final result
of a variable is an integer, it is possible to do very fancy things
like that:
\set aid debug(random_exponential(1, 100, pi()))
\set bid debug(random_exponential(101, 200, pi()))
\set cid debug(random_gaussian(:aid, :bid, double(:aid * pi())))
SELECT :cid;
That's actually where things like sqrt() and pi() gain a lot in power
by working directly on the integers returned by the random functions.
Something that bothered me though while testing: the debug() function
is useful, but it becomes difficult to see its results efficiently
when many outputs are stacking, so I think that it would be useful to
be able to pass a string prefix to have the possibility to annotate a
debug entry, say "debug('foo', 5.2)" outputs:
debug(script=0,command=1): note: foo, int/double X
I guess that it would be useful then to allow as well concatenation of
strings using "+" with a new PgBenchExprType as ENODE_STRING, but
perhaps I am asking too much. Thoughts are welcome regarding that, it
does not seem mandatory as of now as this patch is already doing much.
We could remove some of the functions in the first shot of this patch
to simplify it a bit, but that does not look like a win as their
footprint on the code is low.
I haven't noticed at quick glance any kind of memory leaks but this
deserves a closer look with valgrind for example, still the patch
looks in good shape to me. And more comments for example in pgbench.h
would be welcome to explain more the code. I am fine to take a final
look at that before handling it to a committer though. Does that sound
fine as a plan, Fabien?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
