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
Improve parsing of numeric literals in the Sparql parser #726
Conversation
Yes, QLever will (for efficiency reasons) probably never implement arbitrary precision, and we currently even lose more than 1 bit (currently 4) in the implementation (but this might not be forever). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much,
I think all of my comments are minor, the general design is already very good!
Most notably, while reviewing this I have found once again how hard it is to do stuff properly .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice and much cleaner,
only two small cleanup suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, this looks much cleaner now.
I'll have a look at that test failure (I am sure that it's nor your fault),
and then I'll merge this.
Numeric Literals are now parsed into
variant<int64_t, double>
. Cleaned up the code that previously handled the numeric literalsAny
. AFAIK to be standard compliant we'd need integers with arbitrary size. So removing 1 Bit in the unsigned case shouldn't make that big of a difference. Even more so since they already were converted toint64_t
in some cases.TODO:
NumericLiterals
tests