-
-
Notifications
You must be signed in to change notification settings - Fork 231
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -633,6 +633,153 @@ var VariableQueries = []ScriptTest{ | |
}, | ||
}, | ||
}, | ||
{ | ||
Name: "mysql_string_to_number system variable", | ||
Assertions: []ScriptTestAssertion{ | ||
{ | ||
Query: "SELECT @@mysql_string_to_number", | ||
Expected: []sql.Row{{int8(0)}}, | ||
}, | ||
{ | ||
Query: "SET @@mysql_string_to_number = 1", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT @@mysql_string_to_number", | ||
Expected: []sql.Row{{int8(1)}}, | ||
}, | ||
{ | ||
Query: "SET @@mysql_string_to_number = 0", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT @@mysql_string_to_number", | ||
Expected: []sql.Row{{int8(0)}}, | ||
}, | ||
{ | ||
Query: "SET @@session.mysql_string_to_number = 1", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT @@session.mysql_string_to_number", | ||
Expected: []sql.Row{{int8(1)}}, | ||
}, | ||
{ | ||
Query: "SET @@global.mysql_string_to_number = 0", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT @@global.mysql_string_to_number", | ||
Expected: []sql.Row{{int8(0)}}, | ||
}, | ||
}, | ||
}, | ||
{ | ||
Name: "mysql_string_to_number conversion behavior", | ||
SetUpScript: []string{ | ||
"CREATE TABLE test_table (id INT PRIMARY KEY, int_col INT, float_col FLOAT)", | ||
"INSERT INTO test_table VALUES (1, 10, 10.5)", | ||
}, | ||
Assertions: []ScriptTestAssertion{ | ||
// Variable disabled - original behavior | ||
{ | ||
Query: "SET @@mysql_string_to_number = 0", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('123abc' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
}, | ||
{ | ||
Query: "SELECT CAST('_123_' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('abc123' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
}, | ||
// Variable enabled - extract numeric prefixes | ||
{ | ||
Query: "SET @@mysql_string_to_number = 1", | ||
Expected: []sql.Row{{types.NewOkResult(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('123abc' AS SIGNED)", | ||
Expected: []sql.Row{{int64(123)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('_123_' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('123_' AS SIGNED)", | ||
Expected: []sql.Row{{int64(123)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('abc123' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST(' 123 ' AS SIGNED)", | ||
Expected: []sql.Row{{int64(123)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST(' +45.67e2' AS SIGNED)", | ||
Expected: []sql.Row{{int64(45)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('-789.123' AS SIGNED)", | ||
Expected: []sql.Row{{int64(-789)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('123%' AS SIGNED)", | ||
Expected: []sql.Row{{int64(123)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('#123' AS SIGNED)", | ||
Expected: []sql.Row{{int64(0)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('123.456' AS SIGNED)", | ||
Expected: []sql.Row{{int64(123)}}, | ||
}, | ||
// Test decimal conversions | ||
{ | ||
Query: "SELECT CAST('123.456abc' AS DECIMAL(10,3))", | ||
Expected: []sql.Row{{"123.456"}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('_123.456_' AS DECIMAL(10,3))", | ||
Expected: []sql.Row{{"0.000"}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('45.67e2' AS DECIMAL(10,2))", | ||
Expected: []sql.Row{{"4567.00"}}, | ||
}, | ||
// Test unsigned type handling | ||
{ | ||
Query: "SELECT CAST('123abc' AS UNSIGNED)", | ||
Expected: []sql.Row{{uint64(123)}}, | ||
}, | ||
{ | ||
Query: "SELECT CAST('-123abc' AS UNSIGNED)", | ||
Expected: []sql.Row{{uint64(18446744073709551493)}}, | ||
}, | ||
// Test comparison operations from issue #7128 | ||
{ | ||
Query: "SELECT * FROM test_table WHERE int_col = '10abc'", | ||
Expected: []sql.Row{{1, 10, float32(10.5)}}, | ||
}, | ||
{ | ||
Query: "SELECT * FROM test_table WHERE float_col = '10.5xyz'", | ||
Expected: []sql.Row{{1, 10, float32(10.5)}}, | ||
}, | ||
{ | ||
Query: "SELECT * FROM test_table WHERE int_col = '_10_'", | ||
Expected: []sql.Row{}, | ||
}, | ||
}, | ||
}, | ||
//TODO: do not override tables with user-var-like names...but why would you do this?? | ||
//{ | ||
// Name: "user var table name no conflict", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -339,11 +339,24 @@ func convertValue(ctx *sql.Context, val interface{}, castTo string, originType s | |
} | ||
return d, nil | ||
case ConvertToDecimal: | ||
dt := createConvertedDecimalType(typeLength, typeScale, false) | ||
|
||
// 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. Choose a reason for hiding this commentThe 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. |
||
d, _, err := dt.Convert(ctx, convertedVal) | ||
if err != nil { | ||
return dt.Zero(), nil | ||
} | ||
return d, nil | ||
} | ||
} | ||
|
||
value, err := convertHexBlobToDecimalForNumericContext(val, originType) | ||
if err != nil { | ||
return nil, err | ||
} | ||
dt := createConvertedDecimalType(typeLength, typeScale, false) | ||
|
||
d, _, err := dt.Convert(ctx, value) | ||
if err != nil { | ||
return dt.Zero(), nil | ||
|
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.