From 6334be868901aee2b3f20459dec94d40411fcf8e Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 May 2019 15:58:59 +0800 Subject: [PATCH 1/5] fix --- expression/builtin_time.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/expression/builtin_time.go b/expression/builtin_time.go index 6a442b8d2d08a..6473c2a26a05f 100644 --- a/expression/builtin_time.go +++ b/expression/builtin_time.go @@ -4734,6 +4734,10 @@ func (c *periodAddFunctionClass) getFunction(ctx sessionctx.Context, args []Expr return sig, nil } +func validPeriod(p int64) bool { + return !(p < 0 || p % 100 == 0 || p %100 >12) +} + // period2Month converts a period to months, in which period is represented in the format of YYMM or YYYYMM. // Note that the period argument is not a date value. func period2Month(period uint64) uint64 { @@ -4785,8 +4789,8 @@ func (b *builtinPeriodAddSig) evalInt(row chunk.Row) (int64, bool, error) { return 0, true, err } - if p == 0 { - return 0, false, nil + if !validPeriod(p) { + return 0, false, errIncorrectArgs.GenWithStackByArgs("period_add") } n, isNull, err := b.args[1].EvalInt(b.ctx, row) From 92422c0c5bb0ea1e67dacf2e9215be17239d608d Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 May 2019 16:22:53 +0800 Subject: [PATCH 2/5] fix --- expression/builtin_time.go | 8 ++++---- expression/integration_test.go | 14 ++++++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/expression/builtin_time.go b/expression/builtin_time.go index 6473c2a26a05f..2716b69666cf6 100644 --- a/expression/builtin_time.go +++ b/expression/builtin_time.go @@ -4789,15 +4789,15 @@ func (b *builtinPeriodAddSig) evalInt(row chunk.Row) (int64, bool, error) { return 0, true, err } - if !validPeriod(p) { - return 0, false, errIncorrectArgs.GenWithStackByArgs("period_add") - } - n, isNull, err := b.args[1].EvalInt(b.ctx, row) if isNull || err != nil { return 0, true, err } + if !validPeriod(p) { + return 0, false, errIncorrectArgs.GenWithStackByArgs("period_add") + } + sumMonth := int64(period2Month(uint64(p))) + n return int64(month2Period(uint64(sumMonth))), false, nil } diff --git a/expression/integration_test.go b/expression/integration_test.go index 89b230ab81ba1..1b0a35eebc636 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -1511,10 +1511,16 @@ func (s *testIntegrationSuite) TestTimeBuiltin(c *C) { result.Check(testkit.Rows("123456 10 ")) // for period_add - result = tk.MustQuery(`SELECT period_add(191, 2), period_add(191, -2), period_add(0, 20), period_add(0, 0);`) - result.Check(testkit.Rows("200809 200805 0 0")) - result = tk.MustQuery(`SELECT period_add(NULL, 2), period_add(-191, NULL), period_add(NULL, NULL), period_add(12.09, -2), period_add("21aa", "11aa"), period_add("", "");`) - result.Check(testkit.Rows(" 200010 200208 0")) + result = tk.MustQuery(`SELECT period_add(200807, 2), period_add(200807, -2);`) + result.Check(testkit.Rows("200809 200805")) + result = tk.MustQuery(`SELECT period_add(NULL, 2), period_add(-191, NULL), period_add(NULL, NULL), period_add(12.09, -2), period_add("200207aa", "1aa");`) + result.Check(testkit.Rows(" 200010 200208")) + for _, errPeriod := range []string{ + "period_add(0, 20)", "period_add(0, 0)", "period_add(-1, 1)", "period_add(200013, 1)", "period_add(-200012, 1)", "period_add('', '')", + } { + err := tk.QueryToErr(fmt.Sprintf("SELECT %v;", errPeriod)) + c.Assert(err.Error(), Equals, "[expression:1210]Incorrect arguments to period_add") + } // for period_diff result = tk.MustQuery(`SELECT period_diff(191, 2), period_diff(191, -2), period_diff(0, 0), period_diff(191, 191);`) From f56cb12a341b5548ba097a43f95cbf1c70f1c018 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 May 2019 16:24:59 +0800 Subject: [PATCH 3/5] update --- expression/builtin_time_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expression/builtin_time_test.go b/expression/builtin_time_test.go index 399629dd86835..d6e40953ab037 100644 --- a/expression/builtin_time_test.go +++ b/expression/builtin_time_test.go @@ -2171,8 +2171,8 @@ func (s *testEvaluatorSuite) TestPeriodAdd(c *C) { {201611, -13, true, 201510}, {1611, 3, true, 201702}, {7011, 3, true, 197102}, - {12323, 10, true, 12509}, - {0, 3, true, 0}, + {12323, 10, false, 0}, + {0, 3, false, 0}, } fc := funcs[ast.PeriodAdd] From 995bc6483c1f11b28f7c11ce260efbbf4b2e4e01 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 May 2019 16:28:41 +0800 Subject: [PATCH 4/5] refmt --- expression/builtin_time.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/builtin_time.go b/expression/builtin_time.go index 2716b69666cf6..3f49ca53138d0 100644 --- a/expression/builtin_time.go +++ b/expression/builtin_time.go @@ -4735,7 +4735,7 @@ func (c *periodAddFunctionClass) getFunction(ctx sessionctx.Context, args []Expr } func validPeriod(p int64) bool { - return !(p < 0 || p % 100 == 0 || p %100 >12) + return !(p < 0 || p%100 == 0 || p%100 > 12) } // period2Month converts a period to months, in which period is represented in the format of YYMM or YYYYMM. From 1fa56cff16e5855ae4b5bdcecdfa47e96fde112f Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 8 May 2019 16:43:56 +0800 Subject: [PATCH 5/5] add comments --- expression/builtin_time.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/expression/builtin_time.go b/expression/builtin_time.go index 3f49ca53138d0..883cad23a64de 100644 --- a/expression/builtin_time.go +++ b/expression/builtin_time.go @@ -4734,6 +4734,7 @@ func (c *periodAddFunctionClass) getFunction(ctx sessionctx.Context, args []Expr return sig, nil } +// validPeriod checks if this period is valid, it comes from MySQL 8.0+. func validPeriod(p int64) bool { return !(p < 0 || p%100 == 0 || p%100 > 12) } @@ -4794,6 +4795,7 @@ func (b *builtinPeriodAddSig) evalInt(row chunk.Row) (int64, bool, error) { return 0, true, err } + // in MySQL, if p is invalid but n is NULL, the result is NULL, so we have to check if n is NULL first. if !validPeriod(p) { return 0, false, errIncorrectArgs.GenWithStackByArgs("period_add") }