Hi Kristofer
kpettersson@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of thek. When thek does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-10-15 11:48:38+02:00, thek@adventure.(none) +1 -0
> Bug #31594 Query Cache fail to register tables because of trivial coding misstake.
>
> In function register_all_tables there is a check for success on the call to
> register_tables_from_list. This function has failed if it returns 0 (representing
> the
> number of the next table entry position).
>
> sql/sql_cache.cc@stripped, 2007-10-15 11:48:34+02:00, thek@adventure.(none) +1 -1
> If register_tables_from_list returns 0 it has failed, not the other way around.
>
> diff -Nrup a/sql/sql_cache.cc b/sql/sql_cache.cc
> --- a/sql/sql_cache.cc 2007-09-03 13:46:07 +02:00
> +++ b/sql/sql_cache.cc 2007-10-15 11:48:34 +02:00
> @@ -2501,7 +2501,7 @@ my_bool Query_cache::register_all_tables
>
> n= register_tables_from_list(tables_used, 0, block_table);
>
> - if (n)
> + if (n==0)
> {
> DBUG_PRINT("qcache", ("failed at table %d", (int) n));
> /* Unlink the tables we allocated above */
>
>
So, if I understand correctly the comments and the code:
In case of failures (n == 0), the code previously was :
- not performing some cleanup with unlink_tables(tmp)
- returning false from Query_cache::register_all_tables()
- not adding the query to the cache in Query_cache::store_query()
Now, in case of success (n != 0), the previous code was:
- corrupting (?) the internal structure by performing some unwanted
cleanup with unlink_tables(tmp)
- returning true from Query_cache::register_all_tables()
- adding a questionable query to the cache in Query_cache::store_query()
If that effectively did happen, this should have an effect so that the
query cache was
not behaving properly for some type of queries, or some type of table
invalidation ...
The bug report does not contain any script that can reproduce a
user-visible defect,
just the comment "Try to cache any query with basic tables!".
The existing test suite passes both with and without the fix, which
shows that it's not covering enough.
Could you add a test case with this patch, that detail what exact flaw
the bug caused ?
This will be needed anyway to properly document this change in the
release notes.
Thanks,
Marc.
