← Back to team overview

maria-developers team mailing list archive

Re: Rev 3701: Fixed bug mdev-4311

 

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:
> 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:
> > On 03/24/2013 08:37 AM, Sergei Golubchik wrote:
> > > 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.
> > 
> > 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.
> > 
> > > 
> > > 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
> > 
> > 
> > _______________________________________________
> > Mailing list: https://launchpad.net/~maria-developers
> > Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~maria-developers
> > More help   : https://help.launchpad.net/ListHelp
> 
> -- 
> BR
>  Sergei
> -- 
> Sergei Petrunia, Software Developer
> Monty Program AB, http://askmonty.org
> Blog: http://s.petrunia.net/blog
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp

-- 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
Monty Program AB, http://askmonty.org
Blog: http://s.petrunia.net/blog


References