From 8a570498c83d641d8b06b735abe7d32c5378fba2 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 21:53:18 +0800 Subject: [PATCH 1/3] executor: fix group_concat for cases like group_concat(123,null) --- executor/aggfuncs/func_group_concat.go | 15 ++++++++------- executor/aggregate_test.go | 3 +++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/executor/aggfuncs/func_group_concat.go b/executor/aggfuncs/func_group_concat.go index 40ed32bce0093..80b668720d9b5 100644 --- a/executor/aggfuncs/func_group_concat.go +++ b/executor/aggfuncs/func_group_concat.go @@ -86,12 +86,12 @@ func (e *groupConcat) ResetPartialResult(pr PartialResult) { func (e *groupConcat) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup []chunk.Row, pr PartialResult) (err error) { p := (*partialResult4GroupConcat)(pr) - v, isNull, preLen := "", false, 0 + v, isNull := "", false for _, row := range rowsInGroup { if p.buffer != nil && p.buffer.Len() != 0 { - preLen = p.buffer.Len() p.buffer.WriteString(e.sep) } + rowBuffer := &bytes.Buffer{} for _, arg := range e.args { v, isNull, err = arg.EvalString(sctx, row) if err != nil { @@ -100,17 +100,18 @@ func (e *groupConcat) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup [ if isNull { break } - if p.buffer == nil { - p.buffer = &bytes.Buffer{} - } - p.buffer.WriteString(v) + rowBuffer.WriteString(v) } if isNull { if p.buffer != nil { - p.buffer.Truncate(preLen) + p.buffer.Truncate(p.buffer.Len() - rowBuffer.Len()) } continue } + if p.buffer == nil { + p.buffer = &bytes.Buffer{} + } + p.buffer.WriteString(v) } if p.buffer != nil { return e.truncatePartialResultIfNeed(sctx, p.buffer) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index 5f6bcb413a31a..0dcc91cd075fd 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -408,6 +408,9 @@ func (s *testSuite1) TestGroupConcatAggr(c *C) { result = tk.MustQuery("select id, group_concat(name SEPARATOR '123') from test group by id order by id") result.Check(testkit.Rows("1 101232012330", "2 20", "3 200123500")) + + // #issue 9920 + tk.MustQuery("select group_concat(123, null)").Check(testkit.Rows("")) } func (s *testSuite) TestSelectDistinct(c *C) { From db00ce068ba922c43b62d73d13816399d5e2980a Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Wed, 27 Mar 2019 22:36:31 +0800 Subject: [PATCH 2/3] tiny change --- executor/aggfuncs/func_group_concat.go | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/executor/aggfuncs/func_group_concat.go b/executor/aggfuncs/func_group_concat.go index 80b668720d9b5..68d4584f3d543 100644 --- a/executor/aggfuncs/func_group_concat.go +++ b/executor/aggfuncs/func_group_concat.go @@ -64,7 +64,8 @@ func (e *baseGroupConcat4String) truncatePartialResultIfNeed(sctx sessionctx.Con } type basePartialResult4GroupConcat struct { - buffer *bytes.Buffer + valsBuf *bytes.Buffer + buffer *bytes.Buffer } type partialResult4GroupConcat struct { @@ -76,7 +77,9 @@ type groupConcat struct { } func (e *groupConcat) AllocPartialResult() PartialResult { - return PartialResult(new(partialResult4GroupConcat)) + p := new(partialResult4GroupConcat) + p.valsBuf = &bytes.Buffer{} + return PartialResult(p) } func (e *groupConcat) ResetPartialResult(pr PartialResult) { @@ -88,10 +91,7 @@ func (e *groupConcat) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup [ p := (*partialResult4GroupConcat)(pr) v, isNull := "", false for _, row := range rowsInGroup { - if p.buffer != nil && p.buffer.Len() != 0 { - p.buffer.WriteString(e.sep) - } - rowBuffer := &bytes.Buffer{} + p.valsBuf.Reset() for _, arg := range e.args { v, isNull, err = arg.EvalString(sctx, row) if err != nil { @@ -100,18 +100,17 @@ func (e *groupConcat) UpdatePartialResult(sctx sessionctx.Context, rowsInGroup [ if isNull { break } - rowBuffer.WriteString(v) + p.valsBuf.WriteString(v) } if isNull { - if p.buffer != nil { - p.buffer.Truncate(p.buffer.Len() - rowBuffer.Len()) - } continue } if p.buffer == nil { p.buffer = &bytes.Buffer{} + } else { + p.buffer.WriteString(e.sep) } - p.buffer.WriteString(v) + p.buffer.WriteString(p.valsBuf.String()) } if p.buffer != nil { return e.truncatePartialResultIfNeed(sctx, p.buffer) @@ -145,8 +144,7 @@ func (e *groupConcat) GetTruncated() *int32 { type partialResult4GroupConcatDistinct struct { basePartialResult4GroupConcat - valsBuf *bytes.Buffer - valSet set.StringSet + valSet set.StringSet } type groupConcatDistinct struct { From fc282075eaddceeac615dd542ccdb04ad724cfe7 Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Thu, 28 Mar 2019 11:24:41 +0800 Subject: [PATCH 3/3] Update executor/aggregate_test.go Co-Authored-By: XuHuaiyu --- executor/aggregate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index 0dcc91cd075fd..a9d19cf643345 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -409,7 +409,7 @@ func (s *testSuite1) TestGroupConcatAggr(c *C) { result = tk.MustQuery("select id, group_concat(name SEPARATOR '123') from test group by id order by id") result.Check(testkit.Rows("1 101232012330", "2 20", "3 200123500")) - // #issue 9920 + // issue #9920 tk.MustQuery("select group_concat(123, null)").Check(testkit.Rows("")) }