MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:March 16 2010 9:25am
Subject:Re: bzr commit into mysql-6.0-codebase-bugfixing branch
(jorgen.loland:3805) Bug#51092
View as plain text  
Hi Jørgen!

Functionality is fine, but below comments must be addressed before approval.

Thanks,
Roy

Jorgen Loland wrote:
> #At file:///localhome/jl208045/mysql/mysql-6.0-codebase-bugfixing-51092/ based on
> revid:roy.lyseng@stripped
> 
>  3805 Jorgen Loland	2010-03-11
>       BUG#51092 "Linked join buffer gives wrong result for 3-way 
>                   cross join"
>       
>       The function JOIN_CACHE::read_all_record_fields could return 0
>       for an incremental join cache in two cases:
>       1. there were no more records in the associated join buffer
>       2. there was no table fields stored in the join buffer.
>       As a result the function JOIN_CACHE::get_record() could
>       return prematurely and did not read all needed fields from
>       join buffers into the record buffer.
>       
>       Now the function JOIN_CACHE::read_all_record_fields returns
>       -1 if there are no more records in the associated join buffer.
>       
>       Patch based on contribution by Igor.
>      @ mysql-test/r/join_cache.result
>         Added test for BUG#51092
>      @ mysql-test/t/join_cache.test
>         Added test for BUG#51092
>      @ sql/sql_join_cache.cc
>         Make JOIN_CACHE::get_record() and friends discriminate between the "no more
> records in buffer" case and the "no fields for this table in the buffer" for incremental
> join cache
> 
>     modified:
>       mysql-test/r/join_cache.result
>       mysql-test/t/join_cache.test
>       sql/sql_join_cache.cc
> === modified file 'mysql-test/r/join_cache.result'
> --- a/mysql-test/r/join_cache.result	2010-01-20 10:11:29 +0000
> +++ b/mysql-test/r/join_cache.result	2010-03-11 09:21:34 +0000
> @@ -4081,3 +4081,46 @@ c1	c2	c1	c2	LENGTH(t2.c1)	LENGTH(t2.c2)
>  2	2	tt	uu	2	2
>  set optimizer_join_cache_level=default;
>  DROP TABLE t1,t2;
> +#
> +# Bug#51092: Linked join buffer gives wrong result 
> +#            for 3-way cross join
> +#
> +CREATE TABLE t1 (a INT, b INT);
> +INSERT INTO t1 VALUES (1,1),(2,2);
> +CREATE TABLE t2 (a INT, b INT);
> +INSERT INTO t2 VALUES (1,1),(2,2);
> +CREATE TABLE t3 (a INT, b INT);
> +INSERT INTO t3 VALUES (1,1),(2,2);
> +EXPLAIN SELECT t1.* FROM t1,t2,t3;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	
> +1	SIMPLE	t2	ALL	NULL	NULL	NULL	NULL	2	Using join buffer
> +1	SIMPLE	t3	ALL	NULL	NULL	NULL	NULL	2	Using join buffer
> +SELECT t1.* FROM t1,t2,t3;
> +a	b
> +1	1
> +2	2
> +1	1
> +2	2
> +1	1
> +2	2
> +1	1
> +2	2
> +SET optimizer_join_cache_level=2;
> +EXPLAIN SELECT t1.* FROM t1,t2,t3;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	
> +1	SIMPLE	t2	ALL	NULL	NULL	NULL	NULL	2	Using join buffer
> +1	SIMPLE	t3	ALL	NULL	NULL	NULL	NULL	2	Using join buffer
> +SELECT t1.* FROM t1,t2,t3;
> +a	b
> +1	1
> +2	2
> +1	1
> +2	2
> +1	1
> +2	2
> +1	1
> +2	2
> +SET optimizer_join_cache_level=default;
> +DROP TABLE t1,t2,t3;
> 
> === modified file 'mysql-test/t/join_cache.test'
> --- a/mysql-test/t/join_cache.test	2010-01-20 10:11:29 +0000
> +++ b/mysql-test/t/join_cache.test	2010-03-11 09:21:34 +0000
> @@ -1751,3 +1751,29 @@ SELECT t1.*, t2.*, LENGTH(t2.c1), LENGTH
>  set optimizer_join_cache_level=default;
>  
>  DROP TABLE t1,t2;
> +
> +--echo #
> +--echo # Bug#51092: Linked join buffer gives wrong result 
> +--echo #            for 3-way cross join
> +--echo #
> +
> +CREATE TABLE t1 (a INT, b INT);
> +INSERT INTO t1 VALUES (1,1),(2,2);
> +
> +CREATE TABLE t2 (a INT, b INT);
> +INSERT INTO t2 VALUES (1,1),(2,2);
> +
> +CREATE TABLE t3 (a INT, b INT);
> +INSERT INTO t3 VALUES (1,1),(2,2);
> +
> +EXPLAIN SELECT t1.* FROM t1,t2,t3;
> +SELECT t1.* FROM t1,t2,t3;
> +
> +SET optimizer_join_cache_level=2;
> +
> +EXPLAIN SELECT t1.* FROM t1,t2,t3;
> +SELECT t1.* FROM t1,t2,t3;
> +
> +SET optimizer_join_cache_level=default;
> +
> +DROP TABLE t1,t2,t3;
> 
> === modified file 'sql/sql_join_cache.cc'
> --- a/sql/sql_join_cache.cc	2010-02-05 13:00:33 +0000
> +++ b/sql/sql_join_cache.cc	2010-03-11 09:21:34 +0000
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2000-2006 MySQL AB
> +/* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
>  
>     This program is free software; you can redistribute it and/or modify
>     it under the terms of the GNU General Public License as published by
> @@ -30,6 +30,8 @@
>  #include "mysql_priv.h"
>  #include "sql_select.h"
>  
> +#define NO_MORE_RECORDS_IN_BUFFER  (uint)(-1)

NO_MORE_RECORDS_IN_BUFFER is not defined in the interface, and should not
be defined here.

> +
>  
>  /*****************************************************************************
>   *  Join cache module
> @@ -1234,7 +1236,8 @@ bool JOIN_CACHE::get_record()
>      prev_rec_ptr= prev_cache->get_rec_ref(pos);
>    }
>    curr_rec_pos= pos;
> -  if (!(res= read_all_record_fields() == 0))
> +  res= read_all_record_fields();
> +  if (res != NO_MORE_RECORDS_IN_BUFFER)

"res" is boolean, but return value of read_all_record_fields() is uint.
>    {
>      pos+= referenced_fields*size_of_fld_ofs;
>      if (prev_cache)
> @@ -1323,7 +1326,8 @@ bool JOIN_CACHE::get_match_flag_by_pos(u
>      read data. 
>  
>    RETURN
> -    length of the data read from the join buffer
> +    (-1) - if there are no more records in the join buffer
> +    length of the data read from the join buffer - otherwise
>  */

-1 is outside the value range of the return value (uint). You must either change 
the interface so that return value is int, or change the value to UINT_MAX.
>  
>  uint JOIN_CACHE::read_all_record_fields()
> @@ -1331,7 +1335,7 @@ uint JOIN_CACHE::read_all_record_fields(
>    uchar *init_pos= pos;
>    
>    if (pos > last_rec_pos || !records)
> -    return 0;
> +    return NO_MORE_RECORDS_IN_BUFFER;

Please only use values defined in function's interface.
>  
>    /* First match flag, read null bitmaps and null_row flag for each table */
>    read_flag_fields();
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 
Thread
bzr commit into mysql-6.0-codebase-bugfixing branch (jorgen.loland:3805)Bug#51092Jorgen Loland11 Mar
  • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(jorgen.loland:3805) Bug#51092Roy Lyseng16 Mar