From: Jon Olav Hauglid
Date: June 8 2011 8:09am
Subject: Re: bzr commit into mysql-trunk branch (alexander.nozdrin:3167) Bug#11763162
List-Archive: http://lists.mysql.com/commits/138823
Message-Id: <4DEF2E29.3010506@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit


Hello!

On 06/07/2011 03:29 PM, Alexander Nozdrin wrote:
>   3167 Alexander Nozdrin	2011-06-07
>        Pre-requisite patch for Bug#11763162 (55843 - Handled condition
>        appears as not handled).
>
>        The patch changes the relationship between THD, Diagnostics_area
>        and Warning_info classes:
>
>          - before the patch, THD owned both Diagnostics_area and
>            Warning_info instances.
>
>          - after the patch THD owns Diagnostics_area instance,
>            and Diagnostics_area owns Warning_info instance.
>
>        The patch changes THD::get_warning_info() so that it still
>        works (to save code changes) and eliminates THD::set_warning_info().
>        Users should use Diagnostics_area::set_warning_info() instead.
>

> === 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