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();
>
>
>
> ------------------------------------------------------------------------
>
>
