fix string to boolean comparison for HashInTuple expressions (#3237)

Author: jycor

enginetest/queries/script_queries.go

@@ -403,8 +403,8 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 		},
 	},
 	{
-		Name:    "string to number comparison correctly truncates",
 		Dialect: "mysql",
+		Name:    "string to number comparison correctly truncates",
 		Assertions: []ScriptTestAssertion{
 			{
 				Query:                           "SELECT 'A' = 0;",
@@ -517,12 +517,10 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 				Expected: []sql.Row{{true}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT '1.9a' = 1.9;",
 				Expected: []sql.Row{{true}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT 1 where '1.9a' = 1.9;",
 				Expected: []sql.Row{{1}},
 			},
@@ -590,11 +588,25 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 				ExpectedWarningMessageSubstring: "Truncated incorrect double value: A123",
 			},
 			{
-				Query:                           "SELECT 0 in ('A123');",
+				Query:                           "SELECT '123abc' in ('string', 1, 2, 123);",
+				Expected:                        []sql.Row{{true}},
+				ExpectedWarningsCount:           3, // MySQL only throws 1 warning
+				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
+				ExpectedWarningMessageSubstring: "Truncated incorrect double value",
+			},
+			{
+				Query:                           "SELECT 123 in ('string', 1, 2, '123abc');",
+				Expected:                        []sql.Row{{true}},
+				ExpectedWarningsCount:           2,
+				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
+				ExpectedWarningMessageSubstring: "Truncated incorrect double value",
+			},
+			{
+				Query:                           "SELECT '123A' in (123);",
 				Expected:                        []sql.Row{{true}},
 				ExpectedWarningsCount:           1,
 				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
-				ExpectedWarningMessageSubstring: "Truncated incorrect double value: A123",
+				ExpectedWarningMessageSubstring: "Truncated incorrect double value: 123A",
 			},
 			{
 				Query:    "SELECT '123.456' in (123);",
@@ -605,7 +617,26 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 				Expected: []sql.Row{{true}},
 			},
 			{
-				// TODO: 123.456 is converted to a DECIMAL by Builder.ConvertVal, when it should be a DOUBLE
+				Query:    "SELECT 123.456 in (123.456);",
+				Expected: []sql.Row{{true}},
+			},
+			{
+				Query:    "SELECT 123.45 in (123.4);",
+				Expected: []sql.Row{{false}},
+			},
+			{
+				Query:    "SELECT 123.45 in (123.5);",
+				Expected: []sql.Row{{false}},
+			},
+			{
+				Query:    "SELECT '123.45a' in (123.5);",
+				Expected: []sql.Row{{false}},
+			},
+			{
+				Query:    "SELECT '123.45a' in (123.4);",
+				Expected: []sql.Row{{false}},
+			},
+			{
 				SkipResultCheckOnServerEngine:   true, // TODO: warnings do not make it to server engine
 				Query:                           "SELECT '123.456ABC' in (123.456);",
 				Expected:                        []sql.Row{{true}},
@@ -1229,6 +1260,59 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 			},
 		},
 	},
+	{
+		Dialect: "mysql",
+		Name:    "complicated string to numeric conversion",
+		SetUpScript: []string{
+			"CREATE TABLE t0(c INT);",
+			"INSERT INTO t0 VALUES (1);",
+			"CREATE TABLE t1(c VARCHAR(500));",
+			"INSERT INTO t1 VALUES ('1a');",
+			"CREATE TABLE t2(c0 INT , c1 BOOLEAN , c2 BOOLEAN , c3 INT , placeholder0 INT , placeholder1 VARCHAR(500) , placeholder2 VARCHAR(500) , PRIMARY KEY(placeholder0));",
+			"CREATE TABLE t3(c0 INT , c1 VARCHAR(500) , c2 BOOLEAN , c3 VARCHAR(500) , placeholder0 BOOLEAN , placeholder1 INT , placeholder2 VARCHAR(500));",
+			"INSERT INTO t3 VALUES (7, '0y4', TRUE, '5y', TRUE, 5, 'p9c');",
+			"INSERT INTO t3 VALUES (1, '4', TRUE, '4H', FALSE, 9, 'Zy4');",
+			"INSERT INTO t3 VALUES (10, '1a', FALSE, 'pYE', FALSE, 3, '0awX');",
+			"INSERT INTO t3 VALUES (8, 'J', TRUE, 'LE', TRUE, 9, 'YEqQ');",
+			"INSERT INTO t2 VALUES (10, FALSE, TRUE, 2, 2, 'nfxF', 'xvC');",
+			"INSERT INTO t2 VALUES (10, TRUE, TRUE, 10, 1, 'rlQT', 'W');",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: "SELECT * FROM t0, t1 WHERE (t1.c IN (true));",
+				Expected: []sql.Row{
+					{1, "1a"},
+				},
+			},
+			{
+				Query: "SELECT * FROM t3 INNER JOIN t2 ON ((((t3.c0) = ((EXTRACT(YEAR FROM DATE_ADD(DATE '2000-01-01', INTERVAL ( BIT_LENGTH(( MOD(t2.c3 + ( t2.c3 + ( BIT_COUNT(t2.c3) ) * 3 - CAST(( NOT (t2.c0 XOR t2.c2) ) AS SIGNED) ) * 2, 100 + t2.c3) ) ^ t2.c3) ) DAY)) % (t2.c3 + 1))))) >= (((t3.c2) < ((((((('Bs./')OR('wZ')) IN ((('1066274936')OR('')))))OR((((t3.c1 IN (true)))<>(((t3.c0)OR(( COALESCE(NULLIF(t3.c3, ''), t3.c1) ))))))))))));",
+				Expected: []sql.Row{
+					{7, "0y4", 1, "5y", 1, 5, "p9c", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{1, "4", 1, "4H", 0, 9, "Zy4", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{10, "1a", 0, "pYE", 0, 3, "0awX", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{8, "J", 1, "LE", 1, 9, "YEqQ", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{7, "0y4", 1, "5y", 1, 5, "p9c", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{1, "4", 1, "4H", 0, 9, "Zy4", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{10, "1a", 0, "pYE", 0, 3, "0awX", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{8, "J", 1, "LE", 1, 9, "YEqQ", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+				},
+			},
+			{
+				Query: "SELECT * FROM t3 CROSS JOIN t2 WHERE ((((t3.c0) = ((EXTRACT(YEAR FROM DATE_ADD(DATE '2000-01-01', INTERVAL ( BIT_LENGTH(( MOD(t2.c3 + ( t2.c3 + ( BIT_COUNT(t2.c3) ) * 3 - CAST(( NOT (t2.c0 XOR t2.c2) ) AS SIGNED) ) * 2, 100 + t2.c3) ) ^ t2.c3) ) DAY)) % (t2.c3 + 1))))) >= (((t3.c2) < ((((((('Bs./')OR('wZ')) IN ((('1066274936')OR('')))))OR((((t3.c1 IN (true)))<>(((t3.c0)OR(( COALESCE(NULLIF(t3.c3, ''), t3.c1) ))))))))))));",
+				Expected: []sql.Row{
+					{7, "0y4", 1, "5y", 1, 5, "p9c", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{1, "4", 1, "4H", 0, 9, "Zy4", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{10, "1a", 0, "pYE", 0, 3, "0awX", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{8, "J", 1, "LE", 1, 9, "YEqQ", 10, 1, 1, 10, 1, "rlQT", "W"},
+					{7, "0y4", 1, "5y", 1, 5, "p9c", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{1, "4", 1, "4H", 0, 9, "Zy4", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{10, "1a", 0, "pYE", 0, 3, "0awX", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+					{8, "J", 1, "LE", 1, 9, "YEqQ", 10, 0, 1, 2, 2, "nfxF", "xvC"},
+				},
+			},
+		},
+	},
+
 	{
 		// https://github.com/dolthub/dolt/issues/9794
 		Name: "UPDATE with TRIM function on TEXT column",
@@ -7277,6 +7361,13 @@ CREATE TABLE tab3 (
 				Query:    "select * from t where (f in (null, 0.8));",
 				Expected: []sql.Row{},
 			},
+			{
+				// This actually matches MySQL behavior
+				Query: "select count(*) from t where (f in (null, 0.8));",
+				Expected: []sql.Row{
+					{0},
+				},
+			},
 			{
 				// select count to avoid floating point comparison
 				Query: "select count(*) from t where (f in (null, cast(0.8 as float)));",
@@ -7323,6 +7414,32 @@ CREATE TABLE tab3 (
 			},
 		},
 	},
+	{
+		Name:    "hash in tuple picks correct type and skips mixed types",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"create table t (v varchar(10));",
+			"insert into t values ('abc'), ('def'), ('ghi');",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query:    "select * from t where (v in ('xyz')) order by v;",
+				Expected: []sql.Row{},
+			},
+			{
+				Query: "select * from t where (v in (0, 'xyz')) order by v;",
+				Expected: []sql.Row{
+					{"abc"},
+					{"def"},
+					{"ghi"},
+				},
+			},
+			{
+				Query:    "select * from t where (v in (1, 'xyz')) order by v;",
+				Expected: []sql.Row{},
+			},
+		},
+	},
 	{
 		Name:    "strings in tuple are properly hashed",
 		Dialect: "mysql",

sql/analyzer/apply_hash_in.go

@@ -19,6 +19,7 @@ import (
 	"github.com/dolthub/go-mysql-server/sql/expression"
 	"github.com/dolthub/go-mysql-server/sql/plan"
 	"github.com/dolthub/go-mysql-server/sql/transform"
+	"github.com/dolthub/go-mysql-server/sql/types"
 )
 
 func applyHashIn(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, sel RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) {
@@ -29,9 +30,7 @@ func applyHashIn(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, s
 		}
 
 		e, same, err := transform.Expr(filter.Expression, func(expr sql.Expression) (sql.Expression, transform.TreeIdentity, error) {
-			if e, ok := expr.(*expression.InTuple); ok &&
-				hasSingleOutput(e.Left()) &&
-				isStatic(e.Right()) {
+			if e, ok := expr.(*expression.InTuple); ok && hasSingleOutput(e.Left()) && isStatic(e.Right()) && isConsistentType(e.Right()) {
 				newe, err := expression.NewHashInTuple(ctx, e.Left(), e.Right())
 				if err != nil {
 					return nil, transform.SameTree, err
@@ -77,3 +76,24 @@ func isStatic(e sql.Expression) bool {
 		}
 	})
 }
+
+func isConsistentType(expr sql.Expression) bool {
+	tup, isTup := expr.(expression.Tuple)
+	if !isTup {
+		return true
+	}
+	var hasNumeric, hasString, hasTime bool
+	for _, elem := range tup {
+		eType := elem.Type()
+		if types.IsNumber(eType) {
+			hasNumeric = true
+		} else if types.IsText(eType) {
+			hasString = true
+		} else if types.IsTime(eType) {
+			hasTime = true
+		}
+	}
+	// if there is a mixture of types, we cannot use hash
+	// must have exactly one true
+	return !((hasNumeric && hasString) || (hasNumeric && hasTime) || (hasString && hasTime))
+}

sql/expression/function/ceil_round_floor.go

@@ -179,7 +179,6 @@ func (f *Floor) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 			ctx.Warn(mysql.ERTruncatedWrongValue, "%s", err.Error())
 		}
 	}
-
 	// if it's number type and not float value, it does not need ceil-ing
 	switch num := child.(type) {
 	case float32: