-
-
Couldn't load subscription status.
- Fork 239
dolthub/dolt#7128 - String to number conversion #3136
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
Conversation
A thought: compatibility with MySQL is important, but I also think that throwing an error is probably better behavior (plus having more MySQL-specific behavior in the engine will make it harder to support other dialects like Postgres.)
So I would propose instead to make this behavior configurable, keep the current behavior as the default, and have GMS use the MySQL-specific behavior when a configuration flag is set.
60cefe8 to
a8089e1
Compare
a8089e1 to
9a292fc
Compare
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.
Are these copied from another test? Do these different variables interact with each other?
I'm worried this is copy-pasting code and tests. If we have a lot of variables that all have global/session/local variables like this with specific rules, we should probably have a special test for those that doesn't duplicate code.
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.
The original behavior throws an error, right? This test seems to imply that it returns 0.
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.
So if the flag is set, we convert to a decimal by converting to a float first?
It's not clear from looking at this function why this produces the correct behavior.
Let's pick a more descriptive name for the variable.
Some possibilities:
@@truncate_char_cast_to_number(defaulting to false)@@strict_char_cast_to_number(defaulting to true)
The others might have their own ideas, so maybe we should ask them.
Uh oh!
There was an error while loading. Please reload this page.
Fixes dolthub/dolt#7128
Previously, strings like "123.456abc" would throw errors instead of converting to 123.456 as in MySQL. This change adds MySQL-compatible truncation logic that extracts valid numeric prefixes while handling edge cases like whitespace, scientific notation, and pure non-numeric strings.