From: Alexander Nozdrin
Date: June 8 2011 10:34am
Subject: Re: bzr commit into mysql-trunk branch (alexander.nozdrin:3167) Bug#11763162
List-Archive: http://lists.mysql.com/commits/138829
Message-Id: <4DEF504B.5030304@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hi,

Ok, I see.

What if we rename 'wi' to 'wi_tmp',
and 'wi_saved' to 'wi'?


On 06/08/11 12:09, Jon Olav Hauglid wrote:
>> === modified file 'sql/sql_show.cc'
>> --- a/sql/sql_show.cc	2011-06-07 13:09:47 +0000
>> +++ b/sql/sql_show.cc	2011-06-07 13:29:37 +0000
>> @@ -6803,25 +6803,26 @@ static bool do_fill_table(THD *thd,
>>      // Warning_info, so "useful warnings" get rejected. In order to avoid
>>      // that problem we create a Warning_info instance, which is capable of
>>      // storing "unlimited" number of warnings.
>> +  Diagnostics_area *da= thd->get_stmt_da();
>>      Warning_info wi(thd->query_id, true);
>>      Warning_info *wi_saved= thd->get_warning_info();
>>
>> -  thd->set_warning_info(&wi);
>> +  da->set_warning_info(&wi);
>>
>>      bool res= table_list->schema_table->fill_table(
>>        thd, table_list, join_table->condition());
>>
>> -  thd->set_warning_info(wi_saved);
>> +  da->set_warning_info(wi_saved);
>>
>>      // Pass an error if any.
>>
>> -  if (thd->get_stmt_da()->is_error())
>> +  if (da->is_error())
>>      {
>> -    thd->get_warning_info()->push_warning(thd,
>> -                                          thd->get_stmt_da()->sql_errno(),
>> -                                          thd->get_stmt_da()->get_sqlstate(),
>> -                                          MYSQL_ERROR::WARN_LEVEL_ERROR,
>> -                                          thd->get_stmt_da()->message());
>> +    wi_saved->push_warning(thd,
>> +                           da->sql_errno(),
>> +                           da->get_sqlstate(),
>> +                           MYSQL_ERROR::WARN_LEVEL_ERROR,
>> +                           da->message());
>>      }
>>
>>      // Pass warnings (if any).
>> @@ -6836,7 +6837,7 @@ static bool do_fill_table(THD *thd,
>>      while ((err= it++))
>>      {
>>        if (err->get_level() != MYSQL_ERROR::WARN_LEVEL_ERROR)
>> -      thd->get_warning_info()->push_warning(thd, err);
>> +      wi_saved->push_warning(thd, err);
>>      }
>>
>>      return res;
> 
> Using wi_saved after it has been set back to be the current
> warning info for THD, is a bit confusing. The name implies
> that it's a inactive warning info just saved for a short time,
> not something you'd push warnings into.
> 
> Also, IMO calling da->get_warning_info() to get wi_saved makes
> the code clearer since you later do da->set_warning_info(wi_saved).
> 
> --- Jon Olav
>