dolthub/dolt#7128 - String to number conversion #3136
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. |
elianddb
force-pushed
the
elianddb/7128-fix-string-double-truncation
branch
from
60cefe8 to
a8089e1
Compare
elianddb
force-pushed
the
elianddb/7128-fix-string-double-truncation
branch
from
a8089e1 to
9a292fc
Compare
| Name: "mysql_string_to_number system variable", | ||
| Assertions: []ScriptTestAssertion{ | ||
| { | ||
| Query: "SELECT @@mysql_string_to_number", |
There was a problem hiding this comment.
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.
| }, | ||
| { | ||
| Query: "SELECT CAST('123abc' AS SIGNED)", | ||
| Expected: []sql.Row{{int64(0)}}, |
There was a problem hiding this comment.
The original behavior throws an error, right? This test seems to imply that it returns 0.
|
|
||
| // Handle string-to-decimal conversion for mysql_string_to_number mode | ||
| if strVal, ok := val.(string); ok && sql.ValidateStringToNumberMode(ctx) { | ||
| if convertedVal, _, err := types.Float64.Convert(ctx, strVal); err == nil { |
There was a problem hiding this comment.
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:
The others might have their own ideas, so maybe we should ask them. |
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.