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 _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp