maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #05280
Re: Rev 3701: Fixed bug mdev-4311
Hi, Igor!
On Mar 22, Igor Babaev wrote:
> At file:///home/igor/maria/maria-5.5-bug4311/
>
> ------------------------------------------------------------
> revno: 3701
> revision-id: igor@xxxxxxxxxxxx-20130322224651-r2tpn9in5i1ifv7a
> parent: sergii@xxxxxxxxx-20130317104125-yyp99euwpir5ueho
> committer: Igor Babaev <igor@xxxxxxxxxxxx>
> branch nick: maria-5.5-bug4311
> timestamp: Fri 2013-03-22 15:46:51 -0700
> message:
> Fixed bug mdev-4311 (bug #68749).
> This bug was introduced by the patch for WL#3220.
> If the memory allocated for the tree to store unique elements
> to be counted is not big enough to include all of them then
> an external file is used to store the elements.
> The unique elements are guaranteed not to be nulls. So, when
> reading them from the file we don't have to care about the null
> flags of the read values. However, we should remove the flag
> at the very beginning of the process. If we don't do it and
> if the last value written into the record buffer for the field
> whose distinct values needs to be counted happens to be null,
> then all values read from the file are considered to be nulls
> and are not counted in.
> The fix does not remove a possible null flag for the read values.
> Rather it just counts the values in the same way it was done
> before WL #3220.
> === modified file 'sql/item_sum.cc'
> --- a/sql/item_sum.cc 2013-02-28 17:42:49 +0000
> +++ b/sql/item_sum.cc 2013-03-22 22:46:51 +0000
> @@ -1460,6 +1460,12 @@
>
> bool Aggregator_distinct::unique_walk_function(void *element)
> {
> + if (item_sum->sum_func() == Item_sum::COUNT_DISTINCT_FUNC)
> + {
> + Item_sum_count *sum= (Item_sum_count *)item_sum;
> + sum->count++;
> + return 0;
> + }
> memcpy(table->field[0]->ptr, element, tree_key_length);
> item_sum->add();
> return 0;
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
Follow ups