Hello Andres,
> working on overflow correctness in pg I noticed that pgbench isn't quite
> there. I assume it won't matter terribly often, but the way it parses
> integers makes it incorrect for, at least, the negativemost number.
>
> It directly parses the integer input using:
> {digit}+ {
> yylval->ival = strtoint64(yytext);
> return INTEGER_CONST;
> }
>
> but that unfortunately means that the sign is no included in the call to
> strtoint64. Which in turn means you can't correctly parse PG_INT64_MIN,
Indeed. For a benchmarking script which generates a pseudo random load I
do not see as a big issue, but maybe I'm missing some use case.
> because that's not representable as a positive number on a two's
> complement machine (i.e. everywhere). Preliminary testing shows that
> that can relatively easily fixed by just prefixing [-+]? to the relevant
> exprscan.l rules.
Beware that it must handle the unary/binary plus/minus case consistently:
"-123" vs "a -123" vs "a + -123"
> But it might also not be a bad idea to simply defer parsing of the
> number until exprparse.y has its hand on the number?
It is usually simpler to let the lexer handle constants.
> There's plenty other things wrong with overflow handling too, especially
> evalFunc() basically just doesn't care.
Sure.
There are some overflow checking with div and double to int cast, which
were added because of previous complaints, but which are not very useful
to me.
ISTM that it is a voluntary feature, which handles int64 operations
with a modulo overflow like C. Generating exceptions on such overflows
does not look very attractive to me.
> I'll come up with a patch for that sometime soon.
I wish you could provide some motivation: why does it matter much to a
benchmarking script to handle overflows with more case?
> A third complaint I have is that the tap tests are pretty hard to parse
> for humans.
Probably.
Perl is pretty hard to humans to start with:-)
There is a lot of code factorization to cram many tests together, so that
one test is more or less one line and there are hundreds of them. Not sure
it would help if it was expanded.
There are a lot of regular expressions to check outputs, which does not
help.
I wanted to have the pgbench scripts outside but this has been rejected,
so the tested scripts themselves are in the middle of the perl code, which
I think has degraded the readability significantly.
--
Fabien.