Re: pgbench's expression parsing & negative numbers

Hello Andres,

> working on overflow correctness in pg I noticed that pgbench isn't quite
> there.

Indeed.

> I assume it won't matter terribly often, but the way it parses integers 
> makes it incorrect for, at least, the negativemost number. [...] but 
> that unfortunately means that the sign is no included in the call to 
> strtoint64.

The strtoint64 function is indeed a mess. It does not really report errors 
(more precisely, an error message is printed out, but the execution goes 
on nevertheless...).

> Which in turn means you can't correctly parse PG_INT64_MIN,
> 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.

I'm not sure I get it, because then "1 -2" would be scanned as "int 
signed_int", which requires to add some fine rules to the parser to handle 
that as an addition, and I would be unclear about the priority handling,
eg "1 -2 * 3". But I agree that it can be made to work with transfering
the conversion to the parser and maybe some careful tweaking there.

> 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?
>
> There's plenty other things wrong with overflow handling too, especially
> evalFunc() basically just doesn't care.

Indeed.

Some overflow issues are easy to fix with the existing pg_*_s64_overflow 
macros that you provided. It should also handle double2int64 overflows.

I have tried to trigger errors with a -fsanitize=signed-integer-overflow 
compilation, without much success, but ISTM that you suggested that 
pgbench does not work with that in another thread. Could you be more 
precise?

> I'll come up with a patch for that sometime soon.

ISTM that you have not sent any patch on the subject, otherwise I would 
have reviewed it. Maybe I could do one some time later, unless you think 
that I should not.

-- 
Fabien.