Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions enginetest/queries/variable_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,153 @@ var VariableQueries = []ScriptTest{
},
},
},
{
Name: "mysql_string_to_number system variable",
Assertions: []ScriptTestAssertion{
{
Query: "SELECT @@mysql_string_to_number",
Copy link
Contributor

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.

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)}},
Copy link
Contributor

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.

},
{
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",
Expand Down
15 changes: 14 additions & 1 deletion sql/expression/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

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.

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
Expand Down
13 changes: 13 additions & 0 deletions sql/sql_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,16 @@ func ValidateStrictMode(ctx *Context) bool {
sqlMode := LoadSqlMode(ctx)
return sqlMode.ModeEnabled("STRICT_TRANS_TABLES") || sqlMode.ModeEnabled("STRICT_ALL_TABLES")
}

// ValidateStringToNumberMode returns true if mysql_string_to_number is enabled
func ValidateStringToNumberMode(ctx *Context) bool {
if ctx == nil {
return false
}
if val, err := ctx.GetSessionVariable(ctx, "mysql_string_to_number"); err == nil {
if mysqlCompat, ok := val.(int8); ok {
return mysqlCompat != 0
}
}
return false
}
Loading
Loading