From c80edce5ace915e6629defe42a18e2b2823fa6a5 Mon Sep 17 00:00:00 2001 From: tender-boluo <37445736+tender-boluo@users.noreply.github.com> Date: Fri, 25 Jan 2019 17:22:56 +0800 Subject: [PATCH 01/10] expression: function `format` incompatible function `format` incompatible with MySQL for round up --- expression/builtin_string.go | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index 7f5d58aae79c..ebb32a16f1be 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -3019,6 +3019,54 @@ func (b *builtinFormatSig) evalString(row chunk.Row) (string, bool, error) { if isNull || err != nil { return "", isNull, err } + if strings.Contains(x, ".") { + xArr := strings.Split(x, ".") + x1 := xArr[0] + x2 := xArr[1] + digit, err := strconv.Atoi(d) + if err != nil { + return "", isNull, err + } + if len(x2) > digit { + t := []byte(x2) + carry := false + if t[digit] >= '5' { + carry = true + } + if carry == true { + for i := digit - 1; i >= 0; i-- { + if t[i] == '9' && carry == true { + t[i] = '0' + carry = true + } else if carry == true { + t[i] = t[i] + 1 + carry = false + } + } + } + x2 = string(t) + t = []byte(x1) + if carry == true { + for i := len(x1) - 1; i >= 0; i-- { + if t[i] == '9' && carry == true { + t[i] = '0' + carry = true + } else if carry == true { + t[i] = t[i] + 1 + carry = false + } + } + } + if carry == true { + x1 = "1" + string(t) + } else { + x1 = string(t) + } + } + + x = x1 + "." + x2 + } + formatString, err := mysql.GetLocaleFormatFunction("en_US")(x, d) return formatString, false, err } From 3e18dc12f34ad0c8cd21f4e1724555b85339169c Mon Sep 17 00:00:00 2001 From: tender-boluo <37445736+tender-boluo@users.noreply.github.com> Date: Fri, 25 Jan 2019 17:28:20 +0800 Subject: [PATCH 02/10] add unit test; --- expression/builtin_string_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/expression/builtin_string_test.go b/expression/builtin_string_test.go index 5de8bcdbf5e2..dcf294c61b12 100644 --- a/expression/builtin_string_test.go +++ b/expression/builtin_string_test.go @@ -1608,6 +1608,12 @@ func (s *testEvaluatorSuite) TestFormat(c *C) { ret interface{} warnings int }{ + // issue #8796 + {1.12345, 4, "1,1235", 0}, + {9.99999, 4, "10.0000", 0}, + {1.99999, 4, "2.0000", 0}, + {1.09999, 4, "1.1000", 0}, + {12332.123444, 4, "12,332.1234", 0}, {12332.123444, 0, "12,332", 0}, {12332.123444, -4, "12,332", 0}, From 9ec12a8398cd10154ba631118fe29d00448f75b9 Mon Sep 17 00:00:00 2001 From: tender-boluo <37445736+tender-boluo@users.noreply.github.com> Date: Fri, 25 Jan 2019 17:30:45 +0800 Subject: [PATCH 03/10] fix typo --- expression/builtin_string_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/builtin_string_test.go b/expression/builtin_string_test.go index dcf294c61b12..4d5335f50af4 100644 --- a/expression/builtin_string_test.go +++ b/expression/builtin_string_test.go @@ -1609,7 +1609,7 @@ func (s *testEvaluatorSuite) TestFormat(c *C) { warnings int }{ // issue #8796 - {1.12345, 4, "1,1235", 0}, + {1.12345, 4, "1.1235", 0}, {9.99999, 4, "10.0000", 0}, {1.99999, 4, "2.0000", 0}, {1.09999, 4, "1.1000", 0}, From 72065c5b17ba1baf7fda976d968e44ab62ffcdf6 Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Fri, 25 Jan 2019 17:48:59 +0800 Subject: [PATCH 04/10] go fmt --- expression/builtin_string_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/builtin_string_test.go b/expression/builtin_string_test.go index 4d5335f50af4..6a259f3ca030 100644 --- a/expression/builtin_string_test.go +++ b/expression/builtin_string_test.go @@ -1613,7 +1613,7 @@ func (s *testEvaluatorSuite) TestFormat(c *C) { {9.99999, 4, "10.0000", 0}, {1.99999, 4, "2.0000", 0}, {1.09999, 4, "1.1000", 0}, - + {12332.123444, 4, "12,332.1234", 0}, {12332.123444, 0, "12,332", 0}, {12332.123444, -4, "12,332", 0}, From 5ed0aa475bed8121071cf7012e286338dbb3a2b0 Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Tue, 29 Jan 2019 14:42:59 +0800 Subject: [PATCH 05/10] update:accepted review --- expression/builtin_string.go | 115 +++++++++++++++++------------- expression/builtin_string_test.go | 1 + 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index ebb32a16f1be..ba19dff514f8 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2970,9 +2970,76 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool d = formatMaxDecimals } dStr := strconv.FormatInt(d, 10) + xStr, err = roundingNum(xStr, dStr) + if err != nil { + return "", "", isNull, err + } return xStr, dStr, false, nil } +func roundingNum(xStr, dStr string) (string, error) { + + if strings.Contains(xStr, ".") { + sign := false + if strings.HasPrefix(xStr, "-") { + xStr = strings.Trim(xStr, "-") + sign = true + } + + xArr := strings.Split(xStr, ".") + x1 := xArr[0] + x2 := xArr[1] + digit, err := strconv.Atoi(dStr) + if err != nil { + return "", err + } + if len(x2) > digit { + t := []byte(x2) + carry := false + if t[digit] >= '5' { + carry = true + } + if carry { + for i := digit - 1; i >= 0; i-- { + if t[i] == '9' && carry == true { + t[i] = '0' + carry = true + } else if carry == true { + t[i] = t[i] + 1 + carry = false + break + } + } + } + x2 = string(t) + t = []byte(x1) + if carry { + for i := len(x1) - 1; i >= 0; i-- { + if t[i] == '9' && carry == true { + t[i] = '0' + carry = true + } else if carry == true { + t[i] = t[i] + 1 + carry = false + break + } + } + } + if carry { + x1 = "1" + string(t) + } else { + x1 = string(t) + } + } + + xStr = x1 + "." + x2 + if sign { + xStr = "-" + xStr + } + } + return xStr, nil +} + type builtinFormatWithLocaleSig struct { baseBuiltinFunc } @@ -3019,54 +3086,6 @@ func (b *builtinFormatSig) evalString(row chunk.Row) (string, bool, error) { if isNull || err != nil { return "", isNull, err } - if strings.Contains(x, ".") { - xArr := strings.Split(x, ".") - x1 := xArr[0] - x2 := xArr[1] - digit, err := strconv.Atoi(d) - if err != nil { - return "", isNull, err - } - if len(x2) > digit { - t := []byte(x2) - carry := false - if t[digit] >= '5' { - carry = true - } - if carry == true { - for i := digit - 1; i >= 0; i-- { - if t[i] == '9' && carry == true { - t[i] = '0' - carry = true - } else if carry == true { - t[i] = t[i] + 1 - carry = false - } - } - } - x2 = string(t) - t = []byte(x1) - if carry == true { - for i := len(x1) - 1; i >= 0; i-- { - if t[i] == '9' && carry == true { - t[i] = '0' - carry = true - } else if carry == true { - t[i] = t[i] + 1 - carry = false - } - } - } - if carry == true { - x1 = "1" + string(t) - } else { - x1 = string(t) - } - } - - x = x1 + "." + x2 - } - formatString, err := mysql.GetLocaleFormatFunction("en_US")(x, d) return formatString, false, err } diff --git a/expression/builtin_string_test.go b/expression/builtin_string_test.go index 6a259f3ca030..d2015df5cd9e 100644 --- a/expression/builtin_string_test.go +++ b/expression/builtin_string_test.go @@ -1613,6 +1613,7 @@ func (s *testEvaluatorSuite) TestFormat(c *C) { {9.99999, 4, "10.0000", 0}, {1.99999, 4, "2.0000", 0}, {1.09999, 4, "1.1000", 0}, + {-2.5000, 0, "-3", 0}, {12332.123444, 4, "12,332.1234", 0}, {12332.123444, 0, "12,332", 0}, From c5202623f1ae0d88ab05b4eb41f1896d1ddec6d4 Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Tue, 29 Jan 2019 16:39:49 +0800 Subject: [PATCH 06/10] add comment and change code style --- expression/builtin_string.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index ba19dff514f8..5ae8f881bf5a 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2978,13 +2978,17 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool } func roundingNum(xStr, dStr string) (string, error) { - if strings.Contains(xStr, ".") { sign := false + // xStr can not has '+' prefix now. + // It is built in `evalNumDecArgsFormat` after evaluating `Evalxxx` method. if strings.HasPrefix(xStr, "-") { xStr = strings.Trim(xStr, "-") sign = true } + if strings.HasPrefix(xStr, "+") { + return "", errors.Errorf("xStr can not has `+` prefix") + } xArr := strings.Split(xStr, ".") x1 := xArr[0] @@ -3000,28 +3004,24 @@ func roundingNum(xStr, dStr string) (string, error) { carry = true } if carry { - for i := digit - 1; i >= 0; i-- { - if t[i] == '9' && carry == true { + for i := digit - 1; i >= 0 && carry; i-- { + if t[i] == '9' { t[i] = '0' - carry = true - } else if carry == true { + } else { t[i] = t[i] + 1 carry = false - break } } } x2 = string(t) t = []byte(x1) if carry { - for i := len(x1) - 1; i >= 0; i-- { - if t[i] == '9' && carry == true { + for i := len(x1) - 1; i >= 0 && carry; i-- { + if t[i] == '9' { t[i] = '0' - carry = true - } else if carry == true { + } else { t[i] = t[i] + 1 carry = false - break } } } From 5d0b340afc06f2a7a5cd17731e34717098b60465 Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Wed, 30 Jan 2019 10:46:24 +0800 Subject: [PATCH 07/10] update:change comment and function name --- expression/builtin_string.go | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index 5ae8f881bf5a..7f243df386cb 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2970,17 +2970,17 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool d = formatMaxDecimals } dStr := strconv.FormatInt(d, 10) - xStr, err = roundingNum(xStr, dStr) + xStr, err = roundFormatArgs(xStr, dStr) if err != nil { return "", "", isNull, err } return xStr, dStr, false, nil } -func roundingNum(xStr, dStr string) (string, error) { +func roundFormatArgs(xStr, dStr string) (string, error) { if strings.Contains(xStr, ".") { sign := false - // xStr can not has '+' prefix now. + // xStr cannot have '+' prefix now. // It is built in `evalNumDecArgsFormat` after evaluating `Evalxxx` method. if strings.HasPrefix(xStr, "-") { xStr = strings.Trim(xStr, "-") @@ -3003,26 +3003,22 @@ func roundingNum(xStr, dStr string) (string, error) { if t[digit] >= '5' { carry = true } - if carry { - for i := digit - 1; i >= 0 && carry; i-- { - if t[i] == '9' { - t[i] = '0' - } else { - t[i] = t[i] + 1 - carry = false - } + for i := digit - 1; i >= 0 && carry; i-- { + if t[i] == '9' { + t[i] = '0' + } else { + t[i] = t[i] + 1 + carry = false } } x2 = string(t) t = []byte(x1) - if carry { - for i := len(x1) - 1; i >= 0 && carry; i-- { - if t[i] == '9' { - t[i] = '0' - } else { - t[i] = t[i] + 1 - carry = false - } + for i := len(x1) - 1; i >= 0 && carry; i-- { + if t[i] == '9' { + t[i] = '0' + } else { + t[i] = t[i] + 1 + carry = false } } if carry { From ef11f736dca7c1d837967fc871a2b519138c35b5 Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Thu, 31 Jan 2019 14:54:08 +0800 Subject: [PATCH 08/10] change dStr to d --- expression/builtin_string.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index 7f243df386cb..4ff0a45c187a 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2969,15 +2969,15 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool } else if d > formatMaxDecimals { d = formatMaxDecimals } - dStr := strconv.FormatInt(d, 10) - xStr, err = roundFormatArgs(xStr, dStr) + xStr, err = roundFormatArgs(xStr, int(d)) if err != nil { return "", "", isNull, err } + dStr := strconv.FormatInt(d, 10) return xStr, dStr, false, nil } -func roundFormatArgs(xStr, dStr string) (string, error) { +func roundFormatArgs(xStr string, digit int) (string, error) { if strings.Contains(xStr, ".") { sign := false // xStr cannot have '+' prefix now. @@ -2993,10 +2993,7 @@ func roundFormatArgs(xStr, dStr string) (string, error) { xArr := strings.Split(xStr, ".") x1 := xArr[0] x2 := xArr[1] - digit, err := strconv.Atoi(dStr) - if err != nil { - return "", err - } + if len(x2) > digit { t := []byte(x2) carry := false From 873eb84deaf69302059f6ddad31a19a9b37f160d Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Thu, 31 Jan 2019 15:21:42 +0800 Subject: [PATCH 09/10] style:change variables name --- expression/builtin_string.go | 91 ++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index 4ff0a45c187a..040375ae5900 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2971,65 +2971,64 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool } xStr, err = roundFormatArgs(xStr, int(d)) if err != nil { - return "", "", isNull, err + return "", "", true, err } dStr := strconv.FormatInt(d, 10) return xStr, dStr, false, nil } -func roundFormatArgs(xStr string, digit int) (string, error) { - if strings.Contains(xStr, ".") { - sign := false - // xStr cannot have '+' prefix now. - // It is built in `evalNumDecArgsFormat` after evaluating `Evalxxx` method. - if strings.HasPrefix(xStr, "-") { - xStr = strings.Trim(xStr, "-") - sign = true - } - if strings.HasPrefix(xStr, "+") { - return "", errors.Errorf("xStr can not has `+` prefix") - } +func roundFormatArgs(xStr string, maxNumDecimals int) (string, error) { + if !strings.Contains(xStr, ".") { + return xStr, nil + } - xArr := strings.Split(xStr, ".") - x1 := xArr[0] - x2 := xArr[1] + sign := false + // xStr cannot have '+' prefix now. + // It is built in `evalNumDecArgsFormat` after evaluating `Evalxxx` method. + if strings.HasPrefix(xStr, "-") { + xStr = strings.Trim(xStr, "-") + sign = true + } - if len(x2) > digit { - t := []byte(x2) - carry := false - if t[digit] >= '5' { - carry = true - } - for i := digit - 1; i >= 0 && carry; i-- { - if t[i] == '9' { - t[i] = '0' - } else { - t[i] = t[i] + 1 - carry = false - } - } - x2 = string(t) - t = []byte(x1) - for i := len(x1) - 1; i >= 0 && carry; i-- { - if t[i] == '9' { - t[i] = '0' - } else { - t[i] = t[i] + 1 - carry = false - } + xArr := strings.Split(xStr, ".") + integerPart := xArr[0] + decimalPart := xArr[1] + + if len(decimalPart) > maxNumDecimals { + t := []byte(decimalPart) + carry := false + if t[maxNumDecimals] >= '5' { + carry = true + } + for i := maxNumDecimals - 1; i >= 0 && carry; i-- { + if t[i] == '9' { + t[i] = '0' + } else { + t[i] = t[i] + 1 + carry = false } - if carry { - x1 = "1" + string(t) + } + decimalPart = string(t) + t = []byte(integerPart) + for i := len(integerPart) - 1; i >= 0 && carry; i-- { + if t[i] == '9' { + t[i] = '0' } else { - x1 = string(t) + t[i] = t[i] + 1 + carry = false } } - - xStr = x1 + "." + x2 - if sign { - xStr = "-" + xStr + if carry { + integerPart = "1" + string(t) + } else { + integerPart = string(t) } } + + xStr = integerPart + "." + decimalPart + if sign { + xStr = "-" + xStr + } return xStr, nil } From fac06e34c25809cbfb3e95590f1dd0a8c0bf94ae Mon Sep 17 00:00:00 2001 From: Fengyi Jia <925591914@qq.com> Date: Thu, 31 Jan 2019 15:27:50 +0800 Subject: [PATCH 10/10] delete error return --- expression/builtin_string.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/expression/builtin_string.go b/expression/builtin_string.go index 040375ae5900..8df3bbba0109 100644 --- a/expression/builtin_string.go +++ b/expression/builtin_string.go @@ -2969,17 +2969,14 @@ func evalNumDecArgsForFormat(f builtinFunc, row chunk.Row) (string, string, bool } else if d > formatMaxDecimals { d = formatMaxDecimals } - xStr, err = roundFormatArgs(xStr, int(d)) - if err != nil { - return "", "", true, err - } + xStr = roundFormatArgs(xStr, int(d)) dStr := strconv.FormatInt(d, 10) return xStr, dStr, false, nil } -func roundFormatArgs(xStr string, maxNumDecimals int) (string, error) { +func roundFormatArgs(xStr string, maxNumDecimals int) string { if !strings.Contains(xStr, ".") { - return xStr, nil + return xStr } sign := false @@ -3029,7 +3026,7 @@ func roundFormatArgs(xStr string, maxNumDecimals int) (string, error) { if sign { xStr = "-" + xStr } - return xStr, nil + return xStr } type builtinFormatWithLocaleSig struct {