Hi Varun,

revision-id: c408c41230c4a9de0114d13aaf861d3ced65f4c7 (mariadb-10.5.0-424-gc408c41230c) parent(s): 95da2113a050ad739fdaf60ee871329468a01554 author: Varun Gupta committer: Varun Gupta timestamp: 2020-03-31 11:57:37 +0530 message:

MDEV-11563: GROUP_CONCAT(DISTINCT ...) may produce a non-distinct list

The commit message should describe the error and the fix made. (especially important here as this patch fixes only a part of the issue with GROUP_CONCAT)

--- mysql-test/main/func_gconcat.result | 53 ++++++++++++++++++++++--------------- mysql-test/main/func_gconcat.test | 7 +++++ sql/item_sum.cc | 40 +++++++++++++++++----------- sql/item_sum.h | 3 ++- 4 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/sql/item_sum.h b/sql/item_sum.h index e221d04397d..b10d9f5974d 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -1879,7 +1879,8 @@ class Item_func_group_concat : public Item_sum bool warning_for_row; bool always_null; bool force_copy_fields; - bool no_appended; + /** True if result has been written to output buffer. */ + bool m_result_finalized;

Why does this variable use the m_ convention when others do not? I think this is confusing (I stop for a moment to wonder how is it different from all others every time I see the name). Please remove this. What's "output buffer"? this->result ? And what does "result" mean here - the entire return value of GROUP_CONCAT function or some part of it? Please improve the comment.

/** Limits the rows in the result */ Item *row_limit; /** Skips a particular number of rows in from the result*/

Ok to push after this is addressed. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog