From 1881af706e2ac07117b717feebd7c4ea03c888d7 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Wed, 8 Sep 2021 19:32:58 +0800 Subject: [PATCH] executor: make `group_concat` function consider the collation (#27490) (#27835) --- executor/aggfuncs/func_group_concat.go | 21 ++++++++++++++++----- executor/analyze_test.go | 13 +++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/executor/aggfuncs/func_group_concat.go b/executor/aggfuncs/func_group_concat.go index c2d3b745d12fc..7d892adb54a5c 100644 --- a/executor/aggfuncs/func_group_concat.go +++ b/executor/aggfuncs/func_group_concat.go @@ -26,8 +26,8 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/codec" + "github.com/pingcap/tidb/util/collate" "github.com/pingcap/tidb/util/dbterror" - "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/set" ) @@ -175,10 +175,16 @@ func (e *groupConcatDistinct) ResetPartialResult(pr PartialResult) { func (e *groupConcatDistinct) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup []chunk.Row, pr PartialResult) (err error) { p := (*partialResult4GroupConcatDistinct)(pr) v, isNull := "", false + + collators := make([]collate.Collator, 0, len(e.args)) + for _, arg := range e.args { + collators = append(collators, collate.GetCollator(arg.GetType().Collate)) + } + for _, row := range rowsInGroup { p.valsBuf.Reset() p.encodeBytesBuffer = p.encodeBytesBuffer[:0] - for _, arg := range e.args { + for i, arg := range e.args { v, isNull, err = arg.EvalString(sctx, row) if err != nil { return err @@ -186,7 +192,7 @@ func (e *groupConcatDistinct) UpdatePartialResult(sctx sessionctx.Context, rowsI if isNull { break } - p.encodeBytesBuffer = codec.EncodeBytes(p.encodeBytesBuffer, hack.Slice(v)) + p.encodeBytesBuffer = codec.EncodeBytes(p.encodeBytesBuffer, collators[i].Key(v)) p.valsBuf.WriteString(v) } if isNull { @@ -478,10 +484,15 @@ func (e *groupConcatDistinctOrder) UpdatePartialResult(sctx sessionctx.Context, p := (*partialResult4GroupConcatOrderDistinct)(pr) p.topN.sctx = sctx v, isNull := "", false + collators := make([]collate.Collator, 0, len(e.args)) + for _, arg := range e.args { + collators = append(collators, collate.GetCollator(arg.GetType().Collate)) + } + for _, row := range rowsInGroup { buffer := new(bytes.Buffer) p.encodeBytesBuffer = p.encodeBytesBuffer[:0] - for _, arg := range e.args { + for i, arg := range e.args { v, isNull, err = arg.EvalString(sctx, row) if err != nil { return err @@ -489,7 +500,7 @@ func (e *groupConcatDistinctOrder) UpdatePartialResult(sctx sessionctx.Context, if isNull { break } - p.encodeBytesBuffer = codec.EncodeBytes(p.encodeBytesBuffer, hack.Slice(v)) + p.encodeBytesBuffer = codec.EncodeBytes(p.encodeBytesBuffer, collators[i].Key(v)) buffer.WriteString(v) } if isNull { diff --git a/executor/analyze_test.go b/executor/analyze_test.go index 764b1c38ce692..4573394690d36 100644 --- a/executor/analyze_test.go +++ b/executor/analyze_test.go @@ -648,6 +648,19 @@ func (s *testSuite1) TestDefaultValForAnalyze(c *C) { "└─IndexRangeScan_5 1.00 cop[tikv] table:t, index:a(a) range:[1,1], keep order:false")) } +func (s *testSerialSuite2) TestIssue27429(c *C) { + collate.SetNewCollationEnabledForTest(true) + defer collate.SetNewCollationEnabledForTest(false) + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table test.t(id int, value varchar(20) charset utf8mb4 collate utf8mb4_general_ci, value1 varchar(20) charset utf8mb4 collate utf8mb4_bin)") + tk.MustExec("insert into test.t values (1, 'abc', 'abc '),(4, 'Abc', 'abc'),(3,'def', 'def ');") + + tk.MustQuery("select upper(group_concat(distinct value order by 1)) from test.t;").Check(testkit.Rows("ABC,DEF")) + tk.MustQuery("select upper(group_concat(distinct value)) from test.t;").Check(testkit.Rows("ABC,DEF")) +} + func (s *testSerialSuite2) TestIssue20874(c *C) { collate.SetNewCollationEnabledForTest(true) defer collate.SetNewCollationEnabledForTest(false)