Skip to content
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

fix #33 : int64 parse error #58

Closed
wants to merge 1 commit into from

Conversation

Al-Dyachkov
Copy link

Implemented #33 (comment)

@rivantsov
Copy link
Contributor

looking at it..

@rivantsov
Copy link
Contributor

rivantsov commented Jan 6, 2024

Sorry but this is incorrect fix - if there's a bug at all.
I just tried your fix, and 3 unit tests for number literal failed. You can see it in this PR below, failing check - appveyor found 3 failed unit tests.
This is clear indication that this second conversion call is needed.
I also tried parsing large number you provide (without minus) and it parsed correctly into Int64.
I suspect something is wrong with your setup of NumberLiteral in your grammar. Try using TerminalFactory.CreateCSharpNumber method to create terminal (note: it is unsigned int, minus is added in grammar rules I guess)

@Al-Dyachkov
Copy link
Author

Ok, additional setup seems to do the trick, thanks.

To clarify :

  • I ran into this issue using sample sql grammar
  • Found related open issue opened by other person
  • Forked and Implemented suggested fix, because the dialogue ending suggested that it should be done.

Maybe error message should suggest setting up number literal, because if you just take the sample and run into issues it is kind of unclear on how to proceed ?

@Al-Dyachkov Al-Dyachkov closed this Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants