truncation refactoring and partial decimal truncation implementation (#3230)

Author: jycor

enginetest/enginetests.go

@@ -3961,9 +3961,9 @@ func TestWindowRangeFrames(t *testing.T, harness Harness) {
 	TestQueryWithContext(t, ctx, e, harness, `SELECT sum(y) over (partition by z order by date range between unbounded preceding and interval '1' DAY following) FROM c order by x`, []sql.Row{{float64(1)}, {float64(1)}, {float64(1)}, {float64(1)}, {float64(5)}, {float64(5)}, {float64(10)}, {float64(10)}, {float64(10)}, {float64(10)}}, nil, nil, nil)
 	TestQueryWithContext(t, ctx, e, harness, `SELECT count(y) over (partition by z order by date range between interval '1' DAY following and interval '2' DAY following) FROM c order by x`, []sql.Row{{1}, {1}, {1}, {1}, {1}, {0}, {2}, {2}, {0}, {0}}, nil, nil, nil)
 	TestQueryWithContext(t, ctx, e, harness, `SELECT count(y) over (partition by z order by date range between interval '1' DAY preceding and interval '2' DAY following) FROM c order by x`, []sql.Row{{4}, {4}, {4}, {5}, {2}, {2}, {4}, {4}, {4}, {4}}, nil, nil, nil)
+	TestQueryWithContext(t, ctx, e, harness, "SELECT sum(y) over (partition by z order by date range interval 'e' DAY preceding) FROM c order by x", []sql.Row{{float64(0)}, {float64(0)}, {float64(0)}, {float64(1)}, {float64(1)}, {float64(3)}, {float64(1)}, {float64(1)}, {float64(4)}, {float64(4)}}, nil, nil, nil)
 
 	AssertErr(t, e, harness, "SELECT sum(y) over (partition by z range between unbounded preceding and interval '1' DAY following) FROM c order by x", nil, aggregation.ErrRangeInvalidOrderBy)
-	AssertErr(t, e, harness, "SELECT sum(y) over (partition by z order by date range interval 'e' DAY preceding) FROM c order by x", nil, sql.ErrInvalidValue)
 }
 
 func TestNamedWindows(t *testing.T, harness Harness) {

enginetest/evaluation.go

@@ -129,9 +129,18 @@ func TestScriptWithEngine(t *testing.T, e QueryEngine, harness Harness, script q
 				} else if assertion.ExpectedErrStr != "" {
 					AssertErrWithCtx(t, e, harness, ctx, assertion.Query, assertion.Bindings, nil, assertion.ExpectedErrStr)
 				} else if assertion.ExpectedWarning != 0 {
-					AssertWarningAndTestQuery(t, e, nil, harness, assertion.Query,
-						assertion.Expected, nil, assertion.ExpectedWarning, assertion.ExpectedWarningsCount,
-						assertion.ExpectedWarningMessageSubstring, assertion.SkipResultsCheck)
+					if IsServerEngine(e) && assertion.SkipResultCheckOnServerEngine {
+						t.Skip()
+					}
+					AssertWarningAndTestQuery(t, e, nil, harness,
+						assertion.Query,
+						assertion.Expected,
+						nil,
+						assertion.ExpectedWarning,
+						assertion.ExpectedWarningsCount,
+						assertion.ExpectedWarningMessageSubstring,
+						assertion.SkipResultsCheck,
+					)
 				} else if assertion.SkipResultsCheck {
 					RunQueryWithContext(t, e, harness, nil, assertion.Query)
 				} else if assertion.CheckIndexedAccess {

enginetest/queries/function_queries.go

@@ -1052,7 +1052,7 @@ var FunctionQueryTests = []QueryTest{
 	{
 		Query: `SELECT FLOOR(15728640/1024/1030)`,
 		Expected: []sql.Row{
-			{"14"},
+			{14},
 		},
 	},
 	{

enginetest/queries/json_table_queries.go

@@ -571,7 +571,7 @@ var JSONTableScriptTests = []ScriptTest{
 			},
 			{
 				Query:          "SELECT * FROM JSON_TABLE('{\"c1\":\"abc\"}', '$' COLUMNS(c1 INT PATH '$.c1' DEFAULT 'def' ON ERROR)) as jt;",
-				ExpectedErrStr: "error: 'def' is not a valid value for 'int'",
+				ExpectedErrStr: "Invalid JSON text in argument 1 to function JSON_TABLE: \"Invalid value.\"",
 			},
 		},
 	},
@@ -612,7 +612,7 @@ var JSONTableScriptTests = []ScriptTest{
 			},
 			{
 				Query:          "SELECT * FROM JSON_TABLE('{\"c1\":\"abc\"}', '$' COLUMNS(c1 INT PATH '$.c1' ERROR ON ERROR)) as jt;",
-				ExpectedErrStr: "error: 'abc' is not a valid value for 'int'",
+				ExpectedErrStr: "Invalid JSON text in argument 1 to function JSON_TABLE: \"Invalid value.\"",
 			},
 		},
 	},

enginetest/queries/queries.go

@@ -5509,11 +5509,11 @@ SELECT * FROM cte WHERE  d = 2;`,
 	},
 	{
 		Query:    "select ceil(i + 0.5) from mytable order by 1",
-		Expected: []sql.Row{{"2"}, {"3"}, {"4"}},
+		Expected: []sql.Row{{2}, {3}, {4}},
 	},
 	{
 		Query:    "select floor(i + 0.5) from mytable order by 1",
-		Expected: []sql.Row{{"1"}, {"2"}, {"3"}},
+		Expected: []sql.Row{{1}, {2}, {3}},
 	},
 	{
 		Query:    "select round(i + 0.55, 1) from mytable order by 1",

enginetest/queries/script_queries.go

@@ -501,12 +501,12 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 			},
 			{
 				// TODO: 123.456 is converted to a DECIMAL by Builder.ConvertVal, when it should be a DOUBLE
-				Skip:                            true,
+				SkipResultCheckOnServerEngine:   true, // TODO: warnings do not make it to server engine
 				Query:                           "SELECT '123.456ABC' = 123.456;",
 				Expected:                        []sql.Row{{true}},
 				ExpectedWarningsCount:           1,
 				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
-				ExpectedWarningMessageSubstring: "Truncated incorrect double value: 123A",
+				ExpectedWarningMessageSubstring: "Truncated incorrect decimal(65,30) value: 123.456ABC",
 			},
 			{
 				Query:    "SELECT '123.456e2' = 12345.6;",
@@ -528,20 +528,18 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 			},
 			{
 				// Valid float strings used as arguments to functions are truncated not rounded
-				Skip:                  true,
 				Query:                 "SELECT LENGTH(SPACE('1.9'));",
 				Expected:              []sql.Row{{1}},
-				ExpectedWarningsCount: 2, // MySQL throws two warnings for some reason
+				ExpectedWarningsCount: 1, // TODO: MySQL throws two warnings for some reason
 				ExpectedWarning:       mysql.ERTruncatedWrongValue,
 			},
 			{
 				// TODO: 123.456 is converted to a DECIMAL by Builder.ConvertVal, when it should be a DOUBLE
-				Skip:                            true,
 				Query:                           "SELECT -'+123.456ABC' = -123.456",
 				Expected:                        []sql.Row{{true}},
 				ExpectedWarningsCount:           1,
 				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
-				ExpectedWarningMessageSubstring: "Truncated incorrect double value: +123.456ABC",
+				ExpectedWarningMessageSubstring: "Truncated incorrect decimal(65,30) value: +123.456ABC",
 			},
 			{
 				Query:                           "SELECT '0xBEEF' = 0;",
@@ -608,12 +606,12 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 			},
 			{
 				// TODO: 123.456 is converted to a DECIMAL by Builder.ConvertVal, when it should be a DOUBLE
-				Skip:                            true,
+				SkipResultCheckOnServerEngine:   true, // TODO: warnings do not make it to server engine
 				Query:                           "SELECT '123.456ABC' in (123.456);",
 				Expected:                        []sql.Row{{true}},
 				ExpectedWarningsCount:           1,
 				ExpectedWarning:                 mysql.ERTruncatedWrongValue,
-				ExpectedWarningMessageSubstring: "Truncated incorrect double value: 123A",
+				ExpectedWarningMessageSubstring: "Truncated incorrect decimal(65,30) value: 123.456ABC",
 			},
 			{
 				Query:    "SELECT '123.456e2' in (12345.6);",
@@ -975,13 +973,13 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 		Dialect: "mysql",
 		Name:    "strings cast to numbers",
 		SetUpScript: []string{
-			"create table test01(pk varchar(20) primary key)",
+			"create table test01(pk varchar(20) primary key);",
 			`insert into test01 values ('  3 12 4'),
                           ('  3.2 12 4'),('-3.1234'),('-3.1a'),('-5+8'),('+3.1234'),
                           ('11d'),('11wha?'),('11'),('12'),('1a1'),('a1a1'),('11-5'),
-                          ('3. 12 4'),('5.932887e+07'),('5.932887e+07abc'),('5.932887e7'),('5.932887e7abc')`,
-			"create table test02(pk int primary key)",
-			"insert into test02 values(11),(12),(13),(14),(15)",
+                          ('3. 12 4'),('5.932887e+07'),('5.932887e+07abc'),('5.932887e7'),('5.932887e7abc');`,
+			"create table test02(pk int primary key);",
+			"insert into test02 values(11),(12),(13),(14),(15);",
 		},
 		Assertions: []ScriptTestAssertion{
 			{
@@ -1082,10 +1080,8 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 					{"5.932887e7abc", uint64(5)},
 					{"a1a1", uint64(0)},
 				},
-				// TODO: Should be 19. Missing warnings for "Cast to unsigned converted negative integer to its positive
-				//  complement" (1105) https://github.com/dolthub/dolt/issues/9840
-				ExpectedWarningsCount: 16,
-				ExpectedWarning:       mysql.ERTruncatedWrongValue,
+				ExpectedWarningsCount: 19,
+				// Can't check multiple different warnings
 			},
 			{
 				Query: "select pk, cast(pk as decimal(12,3)) from test01",
@@ -1119,7 +1115,6 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 				ExpectedWarningsCount: 0,
 			},
 
-			// TODO: these are not directly testing casting
 			{
 				// https://github.com/dolthub/dolt/issues/9739
 				Skip:    true,
@@ -1163,19 +1158,15 @@ FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INN
 				ExpectedWarning:       mysql.ERTruncatedWrongValue,
 			},
 			{
-				// https://github.com/dolthub/dolt/issues/9739
-				Skip:                  true,
 				Dialect:               "mysql",
-				Query:                 "select * from test02 where pk in ('11asdf')",
-				Expected:              []sql.Row{{"11"}},
+				Query:                 "select * from test02 where pk in ('11asdf');",
+				Expected:              []sql.Row{{11}},
 				ExpectedWarningsCount: 1,
 				ExpectedWarning:       mysql.ERTruncatedWrongValue,
 			},
 			{
-				// https://github.com/dolthub/dolt/issues/9739
-				Skip:                  true,
 				Dialect:               "mysql",
-				Query:                 "select * from test02 where pk='11.12asdf'",
+				Query:                 "select * from test02 where pk='11.12asdf';",
 				Expected:              []sql.Row{},
 				ExpectedWarningsCount: 1,
 				ExpectedWarning:       mysql.ERTruncatedWrongValue,

server/handler_test.go

@@ -1572,7 +1572,7 @@ func TestStatusVariableMaxUsedConnections(t *testing.T) {
 	}
 
 	checkGlobalStatVar(t, "Max_used_connections", uint64(0))
-	checkGlobalStatVar(t, "Max_used_connections_time", "")
+	checkGlobalStatVar(t, "Max_used_connections_time", uint64(0))
 
 	conn1 := newConn(1)
 	handler.NewConnection(conn1)

sql/columndefault.go

@@ -82,9 +82,15 @@ func (e *ColumnDefaultValue) Eval(ctx *Context, r Row) (interface{}, error) {
 
 	if e.OutType != nil {
 		var inRange ConvertInRange
-		if val, inRange, err = e.OutType.Convert(ctx, val); err != nil {
+		if roundType, isRoundType := e.OutType.(RoundingNumberType); isRoundType {
+			val, inRange, err = roundType.ConvertRound(ctx, val)
+		} else {
+			val, inRange, err = e.OutType.Convert(ctx, val)
+		}
+		if err != nil {
 			return nil, ErrIncompatibleDefaultType.New()
-		} else if !inRange {
+		}
+		if !inRange {
 			return nil, ErrValueOutOfRange.New(val, e.OutType)
 		}
 	}
@@ -227,13 +233,18 @@ func (e *ColumnDefaultValue) CheckType(ctx *Context) error {
 		if val == nil && !e.ReturnNil {
 			return ErrIncompatibleDefaultType.New()
 		}
-		_, inRange, err := e.OutType.Convert(ctx, val)
+		var inRange ConvertInRange
+		if roundType, isRoundType := e.OutType.(RoundingNumberType); isRoundType {
+			val, inRange, err = roundType.ConvertRound(ctx, val)
+		} else {
+			val, inRange, err = e.OutType.Convert(ctx, val)
+		}
 		if err != nil {
 			return ErrIncompatibleDefaultType.Wrap(err)
-		} else if !inRange {
+		}
+		if !inRange {
 			return ErrIncompatibleDefaultType.Wrap(ErrValueOutOfRange.New(val, e.Expr))
 		}
-
 	}
 	return nil
 }

sql/core.go

@@ -24,10 +24,8 @@ import (
 	"strings"
 	"sync/atomic"
 	"time"
-	"unicode"
 	"unsafe"
 
-	"github.com/dolthub/vitess/go/mysql"
 	"github.com/shopspring/decimal"
 	"gopkg.in/src-d/go-errors.v1"
 
@@ -340,48 +338,6 @@ const (
 	NumericCutSet = " \t\n\r"
 )
 
-// TODO: type processing logic should all be in the types package
-func TrimStringToNumberPrefix(ctx *Context, s string, isInt bool) string {
-	if isInt {
-		s = strings.TrimLeft(s, IntCutSet)
-	} else {
-		s = strings.TrimLeft(s, NumericCutSet)
-	}
-
-	seenDigit := false
-	seenDot := false
-	seenExp := false
-	signIndex := 0
-
-	for i := 0; i < len(s); i++ {
-		char := rune(s[i])
-		if unicode.IsDigit(char) {
-			seenDigit = true
-		} else if char == '.' && !seenDot && !isInt {
-			seenDot = true
-		} else if (char == 'e' || char == 'E') && !seenExp && seenDigit && !isInt {
-			seenExp = true
-			signIndex = i + 1
-		} else if !((char == '-' || char == '+') && i == signIndex) {
-			// TODO: this should not happen here, and it should use sql.ErrIncorrectTruncation
-			if isInt {
-				ctx.Warn(mysql.ERTruncatedWrongValue, "Truncated incorrect INTEGER value: '%s'", s)
-			} else {
-				ctx.Warn(mysql.ERTruncatedWrongValue, "Truncated incorrect DOUBLE value: '%s'", s)
-			}
-			return convertEmptyStringToZero(s[:i])
-		}
-	}
-	return convertEmptyStringToZero(s)
-}
-
-func convertEmptyStringToZero(s string) string {
-	if s == "" {
-		return "0"
-	}
-	return s
-}
-
 var ErrVectorInvalidBinaryLength = errors.NewKind("cannot convert BINARY(%d) to vector, byte length must be a multiple of 4 bytes")
 
 // DecodeVector decodes a byte slice that represents a vector. This is needed for distance functions.

sql/expression/arithmetic.go

@@ -691,9 +691,12 @@ func (e *UnaryMinus) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 	}
 
 	if !types.IsNumber(e.Child.Type()) {
-		child, err = decimal.NewFromString(fmt.Sprintf("%v", child))
+		child, _, err = types.InternalDecimalType.Convert(ctx, child)
 		if err != nil {
-			child = 0.0
+			if !sql.ErrTruncatedIncorrect.Is(err) {
+				child = 0.0
+			}
+			ctx.Warn(mysql.ERTruncatedWrongValue, "%s", err.Error())
 		}
 	}
 
@@ -735,7 +738,7 @@ func (e *UnaryMinus) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 	case uint64:
 		return -int64(n), nil
 	case decimal.Decimal:
-		return n.Neg(), err
+		return n.Neg(), nil
 	case string:
 		// try getting int out of string value
 		i, iErr := strconv.ParseInt(n, 10, 64)