List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:June 1 2011 1:23pm
Subject:Re: bzr commit into mysql-trunk branch (olav.sandstaa:3135) Bug#12355958
View as plain text  
Hi Roy,

On 01/06/2011 15:11, Roy Lyseng wrote:
> Hi Olav,
>
> I like this naming for the new access function.
>
> When shuffling fields in the Item struct, please make sure that the 
> size of the struct does not increase (I do not think it will, though).

Good idea. I have now verified (using Sun Studio on Solaris) that the 
size of the Item object is 80 byte both before and after this change (in 
a 32 bit binary). I will do the same test using gcc on Linux before 
pushing the fix.

One question: The type of the with_subselect variable is "my_bool". 
Should I have used the opportunity to change this to a real "bool"? (I 
have verified that that will not increase the size of the Item object)

>
> There's one suggestion for a comment change below.

I will change this as you suggested.

Thanks for the review.

Olav

>
> Thanks,
> Roy
>
> On 01.06.11 10.04, Olav Sandstaa wrote:
>> #Atfile:///export/home/tmp/mysql2/subselect/  based
> onrevid:davi.arnaut@stripped
>>
>>   3135 Olav Sandstaa	2011-06-01
>>        This is preparation patch for Bug#12355958 "FAILING ASSERTION:
>>        TRX->LOCK.N_ACTIVE_THRS == 1". It does not contain the fix for
>>        the bug but will make the bug fix simpler/better.
>>
>>        The Item::with_subselect member variable is public and is used
>>        directly in code outside the Item tree. This patch changes this
>>        to be protected and adds a new method Item::has_subquery() for
>>        getting the value of it.
>>
>>        The patch should not change any behavior.
>>       @ sql/item.cc
>>          Changes in order for initialization of with_subselect in
>>          Item constructors.
>>       @ sql/item.h
>>          Make Item::with_subselect protected and add a getter
>>          fuction Item::has_subquery().
>>       @ sql/item_cmpfunc.cc
>>          Use the new Item::has_subquery() function instead of accessing
>>          Item::with_subselect directly.
>>       @ sql/item_func.cc
>>          Use the new Item::has_subquery() function instead of accessing
>>          Item::with_subselect directly.
>>       @ sql/sql_select.cc
>>          Use the new Item::has_subquery() function instead of accessing
>>          Item::with_subselect directly.
>>
>>      modified:
>>        sql/item.cc
>>        sql/item.h
>>        sql/item_cmpfunc.cc
>>        sql/item_func.cc
>>        sql/sql_select.cc
>> === modified file 'sql/item.cc'
>>
>> === modified file 'sql/item.cc'
>> --- a/sql/item.cc	2011-05-26 15:20:09 +0000
>> +++ b/sql/item.cc	2011-06-01 08:03:48 +0000
>> @@ -419,12 +419,11 @@
>>   Item::Item():
>>     is_expensive_cache(-1), rsize(0), name(0), orig_name(0), name_length(0),
>>     fixed(0), is_autogenerated_name(TRUE),
>> -  collation(&my_charset_bin, DERIVATION_COERCIBLE)
>> +  collation(&my_charset_bin, DERIVATION_COERCIBLE), with_subselect(false)
>>   {
>>     marker= 0;
>>     maybe_null=null_value=with_sum_func=unsigned_flag=0;
>>     decimals= 0; max_length= 0;
>> -  with_subselect= 0;
>>     cmp_context= (Item_result)-1;
>>
>>     /* Put item in free list so that we can free all items at end */
>> @@ -470,8 +469,8 @@
>>     fixed(item->fixed),
>>     is_autogenerated_name(item->is_autogenerated_name),
>>     collation(item->collation),
>> -  with_subselect(item->with_subselect),
>> -  cmp_context(item->cmp_context)
>> +  cmp_context(item->cmp_context),
>> +  with_subselect(item->with_subselect)
>>   {
>>     next= thd->free_list;				// Put in free list
>>     thd->free_list= this;
>>
>> === modified file 'sql/item.h'
>> --- a/sql/item.h	2011-05-26 15:20:09 +0000
>> +++ b/sql/item.h	2011-06-01 08:03:48 +0000
>> @@ -567,10 +567,13 @@
>>     my_bool is_autogenerated_name;        /* indicate was name of this Item
>>                                              autogenerated or set by user */
>>     DTCollation collation;
>> +  Item_result cmp_context;              /* Comparison context */
>> + protected:
>>     my_bool with_subselect;               /* If this item is a subselect or some
>>                                              of its arguments is or contains a
>>                                              subselect. Computed by fix_fields.
> */
>> -  Item_result cmp_context;              /* Comparison context */
>> +
>> + public:
>>     // alloc&  destruct is done as start of select using sql_alloc
>>     Item();
>>     /*
>> @@ -1301,6 +1304,11 @@
>>        @retval FALSE Otherwise.
>>     */
>>     bool is_blob_field() const;
>> +
>> +  /**
>> +    Checks if this item or any of its arguments contains a subquery.
>> +  */
> arguements -> decendents ?
>> +  virtual bool has_subquery() const { return with_subselect; }
>>   };
>>
>>
>>
>> === modified file 'sql/item_cmpfunc.cc'
>> --- a/sql/item_cmpfunc.cc	2011-05-26 05:50:01 +0000
>> +++ b/sql/item_cmpfunc.cc	2011-06-01 08:03:48 +0000
>> @@ -4404,7 +4404,7 @@
>>         const_item_cache= FALSE;
>>       }
>>       with_sum_func=	    with_sum_func || item->with_sum_func;
>> -    with_subselect|=        item->with_subselect;
>> +    with_subselect|=        item->has_subquery();
>>       if (item->maybe_null)
>>         maybe_null=1;
>>     }
>>
>> === modified file 'sql/item_func.cc'
>> --- a/sql/item_func.cc	2011-05-26 10:13:07 +0000
>> +++ b/sql/item_func.cc	2011-06-01 08:03:48 +0000
>> @@ -218,7 +218,7 @@
>>         used_tables_cache|=     item->used_tables();
>>         not_null_tables_cache|= item->not_null_tables();
>>         const_item_cache&=      item->const_item();
>> -      with_subselect|=        item->with_subselect;
>> +      with_subselect|=        item->has_subquery();
>>       }
>>     }
>>     fix_length_and_dec();
>>
>> === modified file 'sql/sql_select.cc'
>> --- a/sql/sql_select.cc	2011-05-26 15:20:09 +0000
>> +++ b/sql/sql_select.cc	2011-06-01 08:03:48 +0000
>> @@ -10118,7 +10118,7 @@
>>         contains a subquery. If this is the case we do not include this
>>         part of the condition.
>>       */
>> -    return !item->with_subselect;
>> +    return !item->has_subquery();
>>     }
>>
>>     const Item::Type item_type= item->type();
>> @@ -11971,7 +11971,7 @@
>>         *simple_order=0;				// Must do a temp table to sort
>>       else if (!(order_tables&  not_const_tables))
>>       {
>> -      if (order->item[0]->with_subselect&&
>> +      if (order->item[0]->has_subquery()&&
>>             !(join->select_lex->options&  SELECT_DESCRIBE))
>>           order->item[0]->val_str(&order->item[0]->str_value);
>>         DBUG_PRINT("info",("removing: %s", order->item[0]->full_name()));
>>
>>
>>
>>
>


Thread
bzr commit into mysql-trunk branch (olav.sandstaa:3135) Bug#12355958Olav Sandstaa1 Jun
  • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3135) Bug#12355958Roy Lyseng1 Jun
    • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3135) Bug#12355958Olav Sandstaa1 Jun
      • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3135) Bug#12355958Roy Lyseng1 Jun