Re: [Maria-developers] Rev 3701: Fixed bug mdev-4311

Hi, Igor! On Mar 22, Igor Babaev wrote:
I'm not sure I like it. You've added a special check for COUNT_DISTINCT_FUNC just before the call of item_sum->add() which is virtual, and Item_sum_count has its own implementation of it. Logically, if there's a virtual method, the check should be in there, not in the caller. And what about SUM(DISTINCT ...) ? Perhaps it's better to mark the item not null, as you've explained in the changeset comment? Regards, Sergei

On 03/24/2013 08:37 AM, Sergei Golubchik wrote:
I don't see any problem here because Aggregator_distinct::unique_walk_function is not virtual. Actually here we have two 'implementations' of the Aggregator_distinct::unique_walk_function processor. One of them is used only for COUNT(DISTINCT) items. Currently Sergey Petrunia is already fighting with unnecessary virtual functions in these classes. I remove one of them, that is good. Regards, Igor.

An alternative patch: === modified file 'sql/item_sum.cc' --- sql/item_sum.cc 2013-02-28 17:42:49 +0000 +++ sql/item_sum.cc 2013-03-25 16:15:05 +0000 @@ -1539,7 +1539,8 @@ bool Item_sum_count::add() { - if (!args[0]->maybe_null || !args[0]->is_null()) + //if (!args[0]->maybe_null || !args[0]->is_null()) + if (!aggr->arg_is_null()) count++; return 0; } Any reason this will not work? On Sun, Mar 24, 2013 at 10:20:41AM -0700, Igor Babaev wrote:
-- BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

On 03/25/2013 09:59 AM, Sergei Petrunia wrote:
Honestly, I did not check it. Yet: With this code you still have unnecessary copying of the elements themselves. Besides: For COUNT(DISTINCT ...) we already have a separate branch when Unique fits the allocated memory: to get the count in this case we just take it from the RB tree of the Unique object, bypassing the tree retrieval. Regards, Igor.

No, this one causes failures.. I'll try a bit more to get another one.. On Mon, Mar 25, 2013 at 08:59:40PM +0400, Sergei Petrunia wrote:
-- BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog
participants (3)
-
Igor Babaev
-
Sergei Golubchik
-
Sergei Petrunia