New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing -9223372036854775808 fails #56

tgpfeiffer opened this Issue Apr 18, 2016 · 1 comment


None yet
1 participant

tgpfeiffer commented Apr 18, 2016

Bug report by @nobu-k:

Although -9223372036854775808 is the minimum integer that int64 can express, BQL cannot parse it correctly. That is because it tries to parse the number as an unsigned int64 value rather than a negative int value.

You are right, when parsing an expression, the parser checks the various expressions defined in bql.peg: is it an orExpr, is it an andExpr, ..., is it a minusExpr, and in fact if there is a - character discovered, it will be pushed on the stack and the remainder will be processed separately. Therefore the parser sees only 9223372036854775808 and this is too large, as you wrote.

As a side note: Our definition of NumericLiteral includes an optional - character, so

EVAL - -9223372036854775808;

is parsed as a minusExpr of the negative number -9223372036854775808 correctly. However, when the minus before is applied, we get 9223372036854775808 which overflows int64 and becomes -9223372036854775808 again.

I think the only possibility to fix this is to modify/extend the definition of minusExpr so that the minus is not split off if it is followed by a number or so. However, we need to be careful in order with respect to operator precedence, I think. At the moment, -1.5::int is understood as -(1.5::int) and we should either (a) make sure it stays that way or (b) prove that it doesn't ever make a difference for int/float conversion whether we do -(1.5::int) or (-1.5)::int.


This comment has been minimized.


tgpfeiffer commented Apr 18, 2016

I added a note to the documentation about this bug (in bql/types.rst). That note needs to be removed once this has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment