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.