maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Sanja!

On May 22, sanja@xxxxxxxxxxxxxxxx wrote:
> ------------------------------------------------------------
> revno: 3769
> revision-id: sanja@xxxxxxxxxxxxxxxx-20130522125526-aw778iepj1yceskd
> parent: holyfoot@xxxxxxxxxxxx-20130514213637-3uvcda98gemgiquf
> committer: sanja@xxxxxxxxxxxxxxxx
> branch nick: work-maria-5.5-MDEV-4520
> timestamp: Wed 2013-05-22 15:55:26 +0300
> message:
>   MDEV-4520 fix.

First, please, fix the changeset comment. The first line should be
MDEV-<number><space><complete issue summary>

For example:

MDEV-4520 Assertion `0' fails in Query_cache::end_of_result on concurrent drop event and event execution

Then, after an empty line, you add whatever explanations you think are
necessary. Like the one below:

>   If there is no net.vio then query cache cant't get data via
>   net_real_write() so it is better just do not try to cache such
>   query.

Also, it'd be good to record the fixed bug in the changeset metadata.
This can be done either with "bzr gcommit" and my patches to it (see the
article in KB), or from the command line with "bzr commit --fixes".

> === modified file 'sql/sql_cache.cc'
> --- a/sql/sql_cache.cc	2013-05-07 11:05:09 +0000
> +++ b/sql/sql_cache.cc	2013-05-22 12:55:26 +0000
> @@ -3977,7 +3977,8 @@ Query_cache::is_cacheable(THD *thd, LEX
>    if (thd->lex->safe_to_cache_query &&
>        (thd->variables.query_cache_type == 1 ||
>         (thd->variables.query_cache_type == 2 && (lex->select_lex.options &
> -						 OPTION_TO_QUERY_CACHE))))
> +						 OPTION_TO_QUERY_CACHE))) &&
> +      thd->net.vio)
>    {
>      DBUG_PRINT("qcache", ("options: %lx  %lx  type: %u",
>                            (long) OPTION_TO_QUERY_CACHE,

The fix itself looks ok. But please add a test case.

And think if there can be more cases (besides events) where SELECT is
executed with thd->net.vio == 0. E.g. init-file? or init-connect?
Or INSERT (in the replication thread) that invokes a stored function or
a trigger? Anything you can think of.

Regards,
Sergei