Hello Michaël,
> 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.
Yep.
> [...] 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.
I think that the answer is yes:-)
Obviously all this is possible in the grand scale of things, but this is
also significant work for a tiny benefit, if any. I rather see "debug" as
a simple tool for debugging a script with "pgbench -t 1" (i.e. one or few
transactions), so that the trace length is not an issue.
Once the script is satisfactory, remove the debug and run it for real.
Note that it does not make much sense to run with "debug" calls for many
transactions because of the performance impact.
> Thoughts are welcome regarding that, it does not seem mandatory
Good.
> as of now as this patch is already doing much.
Yep.
> 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.
That is one good thing about this function infrastructure, adding a new
function has a small impact. Some functions are there more for the demo of
how to do it than useful as such (eg sqrt()), but that is not bad thing.
> 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.
ISTM that there is no allocation in the evaluation part, all is done in
stack, so there is no risk of a leak there. If there is a leak in the
parser we really don't care: this is run once when reading a script, and
then after a run pgbench exits anyway.
> And more comments for example in pgbench.h would be welcome to explain
> more the code.
Ok. I'm generally in favor of comments.
> 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?
I understand that you propose to add these comments?
I'm fine with that!:-)
If I misuderstood, tell me:-)
--
Fabien.
