From dfd459168859f7b1f50d1691170b27015ac74c10 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 17:56:06 +0800 Subject: [PATCH 1/5] util,executor: use MutableString as key for DecimalSet --- executor/executor_test.go | 6 ++++++ util/set/decimal_set.go | 9 +++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 54f8b9730affa..42fbc656a39db 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1134,6 +1134,12 @@ func (s *testSuite) TestUnion(c *C) { tk.MustExec("analyze table t2") _, err = tk.Exec("(select a,b from t1 limit 2) union all (select a,b from t2 order by a limit 1) order by t1.b") c.Assert(err.Error(), Equals, "[planner:1250]Table 't1' from one of the SELECTs cannot be used in global ORDER clause") + + // #issue 9900 + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int, b decimal(6, 3))") + tk.MustExec("insert into t values(1, 1.000)") + tk.MustQuery("select count(distinct a), sum(distinct a) from (select a from t union all select b from t) tmp;").Check(testkit.Rows("1 1.000")) } func (s *testSuite) TestNeighbouringProj(c *C) { diff --git a/util/set/decimal_set.go b/util/set/decimal_set.go index beec6c3671532..2c7cfcd07f47e 100644 --- a/util/set/decimal_set.go +++ b/util/set/decimal_set.go @@ -15,23 +15,24 @@ package set import ( "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/hack" ) // DecimalSet is a decimal set. -type DecimalSet map[types.MyDecimal]struct{} +type DecimalSet map[hack.MutableString]struct{} // NewDecimalSet builds a decimal set. func NewDecimalSet() DecimalSet { - return make(map[types.MyDecimal]struct{}) + return make(map[hack.MutableString]struct{}) } // Exist checks whether `val` exists in `s`. func (s DecimalSet) Exist(val *types.MyDecimal) bool { - _, ok := s[*val] + _, ok := s[hack.String(val.ToString())] return ok } // Insert inserts `val` into `s`. func (s DecimalSet) Insert(val *types.MyDecimal) { - s[*val] = struct{}{} + s[hack.String(val.ToString())] = struct{}{} } From 8766901b9384f64770f4406f114ba527860eba0b Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 20:13:24 +0800 Subject: [PATCH 2/5] tiny change --- executor/aggfuncs/func_avg.go | 12 ++++++----- executor/aggfuncs/func_sum.go | 12 ++++++----- executor/executor_test.go | 2 +- util/set/decimal_set.go | 38 ----------------------------------- 4 files changed, 15 insertions(+), 49 deletions(-) delete mode 100644 util/set/decimal_set.go diff --git a/executor/aggfuncs/func_avg.go b/executor/aggfuncs/func_avg.go index b599d4f3e0779..870a1ab976851 100644 --- a/executor/aggfuncs/func_avg.go +++ b/executor/aggfuncs/func_avg.go @@ -20,6 +20,7 @@ import ( "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/set" ) @@ -153,7 +154,7 @@ func (e *avgPartial4Decimal) MergePartialResult(sctx sessionctx.Context, src Par type partialResult4AvgDistinctDecimal struct { partialResult4AvgDecimal - valSet set.DecimalSet + valSet set.StringSet } type avgOriginal4DistinctDecimal struct { @@ -162,7 +163,7 @@ type avgOriginal4DistinctDecimal struct { func (e *avgOriginal4DistinctDecimal) AllocPartialResult() PartialResult { p := &partialResult4AvgDistinctDecimal{ - valSet: set.NewDecimalSet(), + valSet: set.NewStringSet(), } return PartialResult(p) } @@ -171,7 +172,7 @@ func (e *avgOriginal4DistinctDecimal) ResetPartialResult(pr PartialResult) { p := (*partialResult4AvgDistinctDecimal)(pr) p.sum = *types.NewDecFromInt(0) p.count = int64(0) - p.valSet = set.NewDecimalSet() + p.valSet = set.NewStringSet() } func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup []chunk.Row, pr PartialResult) error { @@ -181,7 +182,8 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex if err != nil { return errors.Trace(err) } - if isNull || p.valSet.Exist(input) { + decStr := string(hack.String(input.ToString())) + if isNull || p.valSet.Exist(decStr) { continue } @@ -192,7 +194,7 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex } p.sum = *newSum p.count++ - p.valSet.Insert(input) + p.valSet.Insert(decStr) } return nil } diff --git a/executor/aggfuncs/func_sum.go b/executor/aggfuncs/func_sum.go index c2935231d5dfb..5cacf755fd839 100644 --- a/executor/aggfuncs/func_sum.go +++ b/executor/aggfuncs/func_sum.go @@ -18,6 +18,7 @@ import ( "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/set" ) @@ -38,7 +39,7 @@ type partialResult4SumDistinctFloat64 struct { type partialResult4SumDistinctDecimal struct { partialResult4SumDecimal - valSet set.DecimalSet + valSet set.StringSet } type baseSumAggFunc struct { @@ -222,14 +223,14 @@ type sum4DistinctDecimal struct { func (e *sum4DistinctDecimal) AllocPartialResult() PartialResult { p := new(partialResult4SumDistinctDecimal) p.isNull = true - p.valSet = set.NewDecimalSet() + p.valSet = set.NewStringSet() return PartialResult(p) } func (e *sum4DistinctDecimal) ResetPartialResult(pr PartialResult) { p := (*partialResult4SumDistinctDecimal)(pr) p.isNull = true - p.valSet = set.NewDecimalSet() + p.valSet = set.NewStringSet() } func (e *sum4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup []chunk.Row, pr PartialResult) error { @@ -239,10 +240,11 @@ func (e *sum4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsI if err != nil { return errors.Trace(err) } - if isNull || p.valSet.Exist(input) { + decStr := string(hack.String(input.ToString())) + if isNull || p.valSet.Exist(decStr) { continue } - p.valSet.Insert(input) + p.valSet.Insert(decStr) if p.isNull { p.val = *input p.isNull = false diff --git a/executor/executor_test.go b/executor/executor_test.go index 42fbc656a39db..fb9734da0ca3d 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1139,7 +1139,7 @@ func (s *testSuite) TestUnion(c *C) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int, b decimal(6, 3))") tk.MustExec("insert into t values(1, 1.000)") - tk.MustQuery("select count(distinct a), sum(distinct a) from (select a from t union all select b from t) tmp;").Check(testkit.Rows("1 1.000")) + tk.MustQuery("select count(distinct a), sum(distinct a), avg(distinct a) from (select a from t union all select b from t) tmp;").Check(testkit.Rows("1 1.000 1.0000000")) } func (s *testSuite) TestNeighbouringProj(c *C) { diff --git a/util/set/decimal_set.go b/util/set/decimal_set.go deleted file mode 100644 index 2c7cfcd07f47e..0000000000000 --- a/util/set/decimal_set.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2018 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// See the License for the specific language governing permissions and -// limitations under the License. - -package set - -import ( - "github.com/pingcap/tidb/types" - "github.com/pingcap/tidb/util/hack" -) - -// DecimalSet is a decimal set. -type DecimalSet map[hack.MutableString]struct{} - -// NewDecimalSet builds a decimal set. -func NewDecimalSet() DecimalSet { - return make(map[hack.MutableString]struct{}) -} - -// Exist checks whether `val` exists in `s`. -func (s DecimalSet) Exist(val *types.MyDecimal) bool { - _, ok := s[hack.String(val.ToString())] - return ok -} - -// Insert inserts `val` into `s`. -func (s DecimalSet) Insert(val *types.MyDecimal) { - s[hack.String(val.ToString())] = struct{}{} -} From 2398f74e737f5ad2ab4a9399babf1d92657fbbfa Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 20:32:20 +0800 Subject: [PATCH 3/5] make ci happy --- executor/aggfuncs/func_avg.go | 10 ++++++---- executor/aggfuncs/func_sum.go | 9 ++++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/executor/aggfuncs/func_avg.go b/executor/aggfuncs/func_avg.go index 870a1ab976851..52d50930d4007 100644 --- a/executor/aggfuncs/func_avg.go +++ b/executor/aggfuncs/func_avg.go @@ -182,11 +182,14 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex if err != nil { return errors.Trace(err) } - decStr := string(hack.String(input.ToString())) - if isNull || p.valSet.Exist(decStr) { + if isNull { continue } - + if decStr := string(hack.String(input.ToString())); p.valSet.Exist(decStr) { + continue + } else { + p.valSet.Insert(decStr) + } newSum := new(types.MyDecimal) err = types.DecimalAdd(&p.sum, input, newSum) if err != nil { @@ -194,7 +197,6 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex } p.sum = *newSum p.count++ - p.valSet.Insert(decStr) } return nil } diff --git a/executor/aggfuncs/func_sum.go b/executor/aggfuncs/func_sum.go index 5cacf755fd839..c10ec35926672 100644 --- a/executor/aggfuncs/func_sum.go +++ b/executor/aggfuncs/func_sum.go @@ -240,11 +240,14 @@ func (e *sum4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsI if err != nil { return errors.Trace(err) } - decStr := string(hack.String(input.ToString())) - if isNull || p.valSet.Exist(decStr) { + if isNull { + continue + } + if decStr := string(hack.String(input.ToString())); p.valSet.Exist(decStr) { continue + } else { + p.valSet.Insert(decStr) } - p.valSet.Insert(decStr) if p.isNull { p.val = *input p.isNull = false From ba8dbc56b150f3833be88a399b3bffbe86300f78 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 20:42:50 +0800 Subject: [PATCH 4/5] make golint happy --- executor/aggfuncs/func_avg.go | 6 +++--- executor/aggfuncs/func_sum.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/executor/aggfuncs/func_avg.go b/executor/aggfuncs/func_avg.go index 52d50930d4007..4001ab40692cd 100644 --- a/executor/aggfuncs/func_avg.go +++ b/executor/aggfuncs/func_avg.go @@ -185,11 +185,11 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex if isNull { continue } - if decStr := string(hack.String(input.ToString())); p.valSet.Exist(decStr) { + decStr := string(hack.String(input.ToString())) + if p.valSet.Exist(decStr) { continue - } else { - p.valSet.Insert(decStr) } + p.valSet.Insert(decStr) newSum := new(types.MyDecimal) err = types.DecimalAdd(&p.sum, input, newSum) if err != nil { diff --git a/executor/aggfuncs/func_sum.go b/executor/aggfuncs/func_sum.go index c10ec35926672..9857d49d6c17a 100644 --- a/executor/aggfuncs/func_sum.go +++ b/executor/aggfuncs/func_sum.go @@ -243,11 +243,11 @@ func (e *sum4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsI if isNull { continue } - if decStr := string(hack.String(input.ToString())); p.valSet.Exist(decStr) { + decStr := string(hack.String(input.ToString())) + if p.valSet.Exist(decStr) { continue - } else { - p.valSet.Insert(decStr) } + p.valSet.Insert(decStr) if p.isNull { p.val = *input p.isNull = false From ce0198de3fd7de844ad7e5369acdfdf7af772c45 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Fri, 29 Mar 2019 14:52:03 +0800 Subject: [PATCH 5/5] address comment --- executor/aggfuncs/func_avg.go | 6 +++++- executor/aggfuncs/func_sum.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/executor/aggfuncs/func_avg.go b/executor/aggfuncs/func_avg.go index 4001ab40692cd..26fe13b97fa13 100644 --- a/executor/aggfuncs/func_avg.go +++ b/executor/aggfuncs/func_avg.go @@ -185,7 +185,11 @@ func (e *avgOriginal4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Contex if isNull { continue } - decStr := string(hack.String(input.ToString())) + hash, err := input.ToHashKey() + if err != nil { + return err + } + decStr := string(hack.String(hash)) if p.valSet.Exist(decStr) { continue } diff --git a/executor/aggfuncs/func_sum.go b/executor/aggfuncs/func_sum.go index 9857d49d6c17a..b3f4424da4392 100644 --- a/executor/aggfuncs/func_sum.go +++ b/executor/aggfuncs/func_sum.go @@ -243,7 +243,11 @@ func (e *sum4DistinctDecimal) UpdatePartialResult(sctx sessionctx.Context, rowsI if isNull { continue } - decStr := string(hack.String(input.ToString())) + hash, err := input.ToHashKey() + if err != nil { + return err + } + decStr := string(hack.String(hash)) if p.valSet.Exist(decStr) { continue }