List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:March 24 2011 2:42pm
Subject:Re: bzr commit into mysql-trunk branch (guilhem.bichot:3348) Bug#11766522
View as plain text  
Hi Guilhem,

thank you for fixing this problem.

Based on the commit description and our IRC discussion, I am fairly convinced 
that the fix addresses the problem at hand. However, please add more 
documentation to the commit record and also consider the suggestions given below.

On 16.03.11 16.16, Guilhem Bichot wrote:
> #At file:///home/mysql_src/bzrrepos_new/59651/ based on
> revid:roy.lyseng@stripped
>
>   3348 Guilhem Bichot	2011-03-16
>        Fix for BUG#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED WITH
> JOIN_CACHE_LEVEL=3"
>        see commit comment of sql_join_cache.cc.

I think that the commit header would be more readable if you gave the general 
comment here, and then added specific documentation of what is done to 
sql_join_cache.cc in the description of that file. But that may be just a 
personal preference thing...

>       @ mysql-test/include/join_cache.inc
>          test for bug. Without the code fix, join_cache_jcl3 would hit an assertion
> failure in join_read_key2.
>       @ mysql-test/r/join_cache_jcl1.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl2.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl3.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl4.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl5.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl6.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl7.result
>          correct result (32 rows)
>       @ mysql-test/r/join_cache_jcl8.result
>          correct result (32 rows)
>       @ sql/handler.h
>          Removing the assignments to table->status
>          from ha_innodb.cc, ha_myisam.cc, ha_heap.cc causes test failures,
>          which shows that engines must set table->status.
>          So tell this brutal truth, in comments: engines must set table->status.
>          All those which I could check, do so (including PBXT).
>          Question for reviewers: most of the affected handler functions
>          have ha_* non-virtual wrappers (ha_index_next_same(), etc);
>          we could, in a follow-up patch:
>          - remove the requirement from the engine (don't set table->status
>          in their functions anymore)
>          - set table->status in the ha_* wrappers.
>          - make STATUS_ * flags invisible to engines
>          - change them to an enum, and replace the magic value "0"
>          with an enum value.
I have not evaluated this, as I am not that familar with impact of changes in 
engine interface.
>       @ sql/sql_join_cache.cc
>          Deleting broken code which caused the bug. It was broken for two reasons.
>          First, it affected JOIN_TABs located _before_ 'join_tab' in the plan;
>          there is no reason why reading rows from 'join_tab' should affect
>          TABLE::status of previous JOIN_TABs.
>          Second, the save/restore logic was not recursion-safe; consider:
>          int save_x, x; // global variables
>          f()
>          {
>            save_x= x; // save
>            f();
>            x= save_x; // restore
>          }
>          the inner call to f() will overwrite the value of save_x,
>          in the end x will not get its original value back.
>          That is what caused the assertion failure. In the bug's testcase,
>          the last tables of the plan are t7, t8, t6. t7 is doing ref access,
>          t8 and t6 are doing join buffering. During execution,
>          at some point, no row is found in t7 so t7's table->status is set to
>          STATUS_NOT_FOUND // because no record found
>          | STATUS_NULL_ROW // because NULL-complemented (LEFT JOIN)
>          Then we continue into t8: at some point the join buffer of t8 is full,
>          so t8 cannot accumulate more partial records sent by t7, so
>          join_records() is called for t8, to generate records (of t8 joined
>          with its buffer), in order to be able to later empty t8's buffer.
>          Those records are generated, which backs up t7's table->status
>          into t7's tab->status, and changes table->status to 0.
>          Generated records are passed to t6. t6's buffer gets full. So
>          join_records() is called for t6. Which backs up t7's table->status
>          (0) into t7's tab->status. From then on, things are wrong, as when
>          the two join_records() finally end, t7's table->status is not
>          restored to its original value (STATUS_NOT_FOUND|STATUS_NULL_ROW)
>          but to 0 ("save_x" issue above). After that, more rows come from tables
>          before t7, so we have another ref access to t7 i.e. a call to
> join_read_key2(),
>          which has this code:
>            if (cmp_buffer_with_ref(tab->join->thd, table, table_ref) ||
>                (table->status&  (STATUS_GARBAGE | STATUS_NULL_ROW)))
>            {
>          ...
>            }
>            else if (table->status == 0)
>            {
>              DBUG_ASSERT(table_ref->has_record);
>              table_ref->use_count++;
>            }
>          The "else" branch is expected to be for when:
>          - the lookup value is the same as the previous lookup value (see
> cmp_buffer_with_ref())
>          - the previous lookup did find a record (i.e. table->status==0)
>          - in which case there should be a record ready for reusal
>          (which is asserted with table_ref->has_record) and a new lookup can be
> avoided.
>          But in our case, though the lookup value is the same, there
>          is no record; if t7's table->status was correct it would still
>          have STATUS_NULL_ROW and we wouldn't enter the "else" block.
>          Because t7's table->status is wrongly 0, we enter the "else"
>          and the assertion fires, because there is no record actually.
>
>          This code dates from 2000 at the latest (when code was imported into
> bitkeeper).
>
>          It is unlikely that this causes a real bug in 5.1 or 5.5 because:
>          - those older versions do join buffering only for inner join
>          - in an inner join, if the "ref" access finds no rows, we don't
>          go into the "next" tables, so have no chance to corrupt table->status
>          of the table doing "ref" access.
>       @ sql/sql_select.cc
>          unused flag (was never set)
>       @ sql/sql_select.h
>          JOIN_TAB::status not needed anymore
>       @ sql/structs.h
>          Best-effort documentation of STATUS_ flags, to the extent of what I
> understood.
>          Deleting STATUS_PARENT and STATUS_NOT_READ: they were never set, and this is
> sure:
>          I temporarily changed TABLE::status to an enum so that any
>          table->status= 8; /* using 8 instead of STATUS_NOT_READ */
>          would be caught by the compiler.
>          Question for reviewers:
>          the most used values are STATUS_NOT_FOUND and 0 (only those
>          two are used by all storage engines; see ha_innobase::index_read() for an
> example);
>          should we replace #define-s by an enum and give a name to this 0?
>          Note that the enum will then be part of the storage engine API.

My advice is that these changes should be part of a separate bug fix, but that 
is up to you.

Use of enum is generally good, but I am not familiar with what changes can be 
done in the engine interface.

I propose that you do the following additional changes:

- Add a #define for value 0 (e.g. STATUS_ROW_FOUND)

- Rename STATUS_GARBAGE to e.g STATUS_END_OF_SCAN (you may have to keep 
STATUS_GARBAGE as a synonym).

- Delete the STATUS_NO_RECORD (it's a combined status, or a bit map, or 
something...).

- Clarify the semantics of the two "no row" constant values. It seems that 
STATUS_GARBAGE is set before scanning for rows, or when a scan has reached the 
end (hence I suggest STATUS_END_OF_SCAN), and STATUS_NOT_FOUND is set when we 
have done a key lookup, or are in the middle of a scan and is positioned on a 
row that should not be considered.


>      modified:
>        mysql-test/include/join_cache.inc
>        mysql-test/r/join_cache_jcl1.result
>        mysql-test/r/join_cache_jcl2.result
>        mysql-test/r/join_cache_jcl3.result
>        mysql-test/r/join_cache_jcl4.result
>        mysql-test/r/join_cache_jcl5.result
>        mysql-test/r/join_cache_jcl6.result
>        mysql-test/r/join_cache_jcl7.result
>        mysql-test/r/join_cache_jcl8.result
>        sql/handler.h
>        sql/sql_join_cache.cc
>        sql/sql_select.cc
>        sql/sql_select.h
>        sql/structs.h
> === modified file 'mysql-test/include/join_cache.inc'
> --- a/mysql-test/include/join_cache.inc	2010-08-11 13:28:19 +0000
> +++ b/mysql-test/include/join_cache.inc	2011-03-16 15:16:54 +0000
> @@ -1577,3 +1577,72 @@ eval explain $query;
>   eval $query;
>
>   drop table t1,t2;
> +
> +--echo
> +--echo # Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +--echo # WITH JOIN_CACHE_LEVEL=3"
> +--echo
> +
> +CREATE TABLE t1 (
> +  b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +
> +CREATE TABLE t4 (
> +  col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +
> +CREATE TABLE t6 (
> +  col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +
> +CREATE TABLE t7 (
> +  col319 timestamp NOT NULL,
> +  UNIQUE KEY idx263 (col319)
> +)  ;
> +# zero rows would do, if there was no const-table optimization
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +
> +CREATE TABLE t3 (
> +  col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +# one single row would do, if there was no const-table optimization
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +
> +CREATE TABLE t5 (
> +  col712 time
> +)  ;
> +# zero rows would do, if there was no const-table optimization
> +insert into t5 values(0),(0);
> +
> +CREATE TABLE t8 (
> +  col804 char(169),
> +  col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +
> +CREATE TABLE t2 (
> +  col841 varchar(10)
> +)  ;
> +# one single row would do, if there was no const-table optimization
> +INSERT INTO t2 VALUES (''),('');
> +
> +# small buffer, to trigger "full buffer" in both caches of t8 and t6
> +set join_buffer_size=1;
> +select @@join_buffer_size;
> +
> +--disable_warnings
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +--enable_warnings
> +
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +
> +set @@join_buffer_size=default;
>
> === modified file 'mysql-test/r/join_cache_jcl1.result'
> --- a/mysql-test/r/join_cache_jcl1.result	2011-02-17 14:42:21 +0000
> +++ b/mysql-test/r/join_cache_jcl1.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl2.result'
> --- a/mysql-test/r/join_cache_jcl2.result	2011-02-17 14:42:21 +0000
> +++ b/mysql-test/r/join_cache_jcl2.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl3.result'
> --- a/mysql-test/r/join_cache_jcl3.result	2011-02-17 14:42:21 +0000
> +++ b/mysql-test/r/join_cache_jcl3.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl4.result'
> --- a/mysql-test/r/join_cache_jcl4.result	2011-02-17 14:42:21 +0000
> +++ b/mysql-test/r/join_cache_jcl4.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl5.result'
> --- a/mysql-test/r/join_cache_jcl5.result	2010-11-30 13:55:22 +0000
> +++ b/mysql-test/r/join_cache_jcl5.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl6.result'
> --- a/mysql-test/r/join_cache_jcl6.result	2010-11-30 13:55:22 +0000
> +++ b/mysql-test/r/join_cache_jcl6.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl7.result'
> --- a/mysql-test/r/join_cache_jcl7.result	2010-11-30 13:55:22 +0000
> +++ b/mysql-test/r/join_cache_jcl7.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'mysql-test/r/join_cache_jcl8.result'
> --- a/mysql-test/r/join_cache_jcl8.result	2010-11-30 13:55:22 +0000
> +++ b/mysql-test/r/join_cache_jcl8.result	2011-03-16 15:16:54 +0000
> @@ -2250,5 +2250,91 @@ col_int_key	col_datetime
>   0	2000-09-26 07:45:57
>   2	2003-02-11 21:19:41
>   drop table t1,t2;
> +
> +# Bug#11766522 "59651: ASSERTION `TABLE_REF->HAS_RECORD' FAILED
> +# WITH JOIN_CACHE_LEVEL=3"
> +
> +CREATE TABLE t1 (
> +b varchar(20)
> +)  ;
> +INSERT INTO t1 VALUES ('1'),('1');
> +CREATE TABLE t4 (
> +col253 text
> +)  ;
> +INSERT INTO t4 VALUES (''),('pf');
> +CREATE TABLE t6 (
> +col282 timestamp
> +)  ;
> +INSERT INTO t6 VALUES ('2010-11-07 01:04:45'),('2010-12-13 01:36:32');
> +CREATE TABLE t7 (
> +col319 timestamp NOT NULL,
> +UNIQUE KEY idx263 (col319)
> +)  ;
> +insert into t7 values("2000-01-01"),("2000-01-02");
> +CREATE TABLE t3 (
> +col582 char(230) CHARACTER SET utf8 DEFAULT NULL
> +)  ;
> +INSERT INTO t3 VALUES ('cymej'),('spb');
> +CREATE TABLE t5 (
> +col712 time
> +)  ;
> +insert into t5 values(0),(0);
> +CREATE TABLE t8 (
> +col804 char(169),
> +col805 varchar(51)
> +)  ;
> +INSERT INTO t8 VALUES ('tmqcb','pwk');
> +CREATE TABLE t2 (
> +col841 varchar(10)
> +)  ;
> +INSERT INTO t2 VALUES (''),('');
> +set join_buffer_size=1;
> +Warnings:
> +Warning	1292	Truncated incorrect join_buffer_size value: '1'
> +select @@join_buffer_size;
> +@@join_buffer_size
> +128
> +select 1 from
> +(t1 join t2 join t3)
> +left join t4 on 1
> +left join t5 on 1 like t4.col253
> +left join t6 on t5.col712 is null
> +left join t7 on t1.b<=>t7.col319
> +left join t8 on t3.col582<=  1;
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +1
> +drop table t1,t2,t3,t4,t5,t6,t7,t8;
> +set @@join_buffer_size=default;
>   set optimizer_join_cache_level = default;
>   set optimizer_switch = default;
>
> === modified file 'sql/handler.h'
> --- a/sql/handler.h	2011-01-26 21:12:56 +0000
> +++ b/sql/handler.h	2011-03-16 15:16:54 +0000
> @@ -1799,9 +1799,12 @@ public:
>     }
>     /**
>        @brief
> -     Positions an index cursor to the index specified in the handle. Fetches the
> -     row if available. If the key value is null, begin at the first key of the
> -     index.
> +     Positions an index cursor to the index specified in the handle
> +     ('active_index'). Fetches the row if available. If the key value is null,
> +     begin at the first key of the index.
> +     To indicate success (found a record), function returns 0 _and_
> +     sets table->status to 0. Otherwise, returns non-zero and sets
> +     table->status to STATUS_NOT_FOUND.
>     */
>     virtual int index_read_map(uchar * buf, const uchar * key,
>                                key_part_map keypart_map,
> @@ -1813,27 +1816,34 @@ public:
>   protected:
>     /**
>        @brief
> -     Positions an index cursor to the index specified in the handle. Fetches the
> -     row if available. If the key value is null, begin at the first key of the
> -     index.
> +     Positions an index cursor to the index specified in parameter. Fetches
> +     the row if available. If the key value is null, begin at the first key of
> +     the index.
> +     @returns as index_read_map().
>     */
>     virtual int index_read_idx_map(uchar * buf, uint index, const uchar * key,
>                                    key_part_map keypart_map,
>                                    enum ha_rkey_function find_flag);
For the functions below, I'd appreciate if you added full explanations of what 
they are supposed to do.
> +  /// @returns as index_read_map().
>     virtual int index_next(uchar * buf)
>      { return  HA_ERR_WRONG_COMMAND; }
> +  /// @returns as index_read_map().
>     virtual int index_prev(uchar * buf)
>      { return  HA_ERR_WRONG_COMMAND; }
> +  /// @returns as index_read_map().
>     virtual int index_first(uchar * buf)
>      { return  HA_ERR_WRONG_COMMAND; }
> +  /// @returns as index_read_map().
>     virtual int index_last(uchar * buf)
>      { return  HA_ERR_WRONG_COMMAND; }
>   public:
> +  /// @returns as index_read_map().
>     virtual int index_next_same(uchar *buf, const uchar *key, uint keylen);
>     /**
>        @brief
>        The following functions works like index_read, but it find the last
>        row with the current key value or prefix.
> +     @returns as index_read_map().
>     */
>     virtual int index_read_last_map(uchar * buf, const uchar * key,
>                                     key_part_map keypart_map)
> @@ -1853,7 +1863,9 @@ public:
>       { return NULL; }
>     virtual int ft_read(uchar *buf) { return HA_ERR_WRONG_COMMAND; }
>   protected:
> +  /// @returns as index_read_map().
>     virtual int rnd_next(uchar *buf)=0;
> +  /// @returns as index_read_map().
>     virtual int rnd_pos(uchar * buf, uchar *pos)=0;
>   public:
>     /**
>
> === modified file 'sql/sql_join_cache.cc'
> --- a/sql/sql_join_cache.cc	2010-09-02 07:34:10 +0000
> +++ b/sql/sql_join_cache.cc	2011-03-16 15:16:54 +0000
> @@ -1761,7 +1761,6 @@ enum_nested_loop_state JOIN_CACHE_BNL::j
>   {
>     uint cnt;
>     int error;
> -  JOIN_TAB *tab;
>     READ_RECORD *info;
>     enum_nested_loop_state rc= NESTED_LOOP_OK;
>     SQL_SELECT *select= join_tab->cache_select;
> @@ -1788,12 +1787,6 @@ enum_nested_loop_state JOIN_CACHE_BNL::j
>       join_tab->select->quick= 0;
>     }
>
> -  for (tab= join->join_tab; tab != join_tab ; tab++)
> -  {
> -    tab->status= tab->table->status;
> -    tab->table->status= 0;
> -  }
> -
>     /* Start retrieving all records of the joined table */
>     if ((error= join_init_read_record(join_tab)))
>     {
> @@ -1857,8 +1850,6 @@ enum_nested_loop_state JOIN_CACHE_BNL::j
>     if (error>  0)				// Fatal error
>       rc= NESTED_LOOP_ERROR;
>   finish:
> -  for (tab= join->join_tab; tab != join_tab ; tab++)
> -    tab->table->status= tab->status;
>     return rc;
>   }

It would also be nice to see the "finish" labels go. Some places it is easy to 
do a break, some places not...
>
> @@ -2375,12 +2366,6 @@ JOIN_CACHE_BKA::init_join_matching_recor
>     /* Dynamic range access is never used with BKA */
>     DBUG_ASSERT(join_tab->use_quick != 2);
>
> -  for (JOIN_TAB *tab =join->join_tab; tab != join_tab ; tab++)
> -  {
> -    tab->status= tab->table->status;
> -    tab->table->status= 0;
> -  }
> -
>     init_mrr_buff();
>
>     /*
> @@ -2416,9 +2401,7 @@ JOIN_CACHE_BKA::init_join_matching_recor
>   enum_nested_loop_state
>   JOIN_CACHE_BKA::end_join_matching_records(enum_nested_loop_state rc)
>   {
> -  for (JOIN_TAB *tab=join->join_tab; tab != join_tab ; tab++)
> -    tab->table->status= tab->status;
> -  return rc;
> +  return rc;
>   }
>
>
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2011-03-03 09:43:14 +0000
> +++ b/sql/sql_select.cc	2011-03-16 15:16:54 +0000
> @@ -18154,7 +18154,7 @@ join_read_key2(JOIN_TAB *tab, TABLE *tab
>       indices on NOT NULL columns (see create_ref_for_key()).
>     */
>     if (cmp_buffer_with_ref(tab->join->thd, table, table_ref) ||
> -      (table->status&  (STATUS_GARBAGE | STATUS_NO_PARENT |
> STATUS_NULL_ROW)))
> +      (table->status&  (STATUS_GARBAGE | STATUS_NULL_ROW)))
>     {
>       if (table_ref->key_err)
>       {
>
> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h	2011-03-01 14:57:53 +0000
> +++ b/sql/sql_select.h	2011-03-16 15:16:54 +0000
> @@ -296,7 +296,6 @@ typedef struct st_join_table : public Sq
>
>     table_map	dependent,key_dependent;
>     uint		index;
> -  uint		status;				///<  Save status for cache
>     uint		used_fields,used_fieldlength,used_blobs;
>     uint          used_null_fields;
>     uint          used_rowid_fields;
> @@ -470,7 +469,6 @@ st_join_table::st_join_table()
>       dependent(0),
>       key_dependent(0),
>       index(0),
> -    status(0),
>       used_fields(0),
>       used_fieldlength(0),
>       used_blobs(0),
>
> === modified file 'sql/structs.h'
> --- a/sql/structs.h	2010-10-21 09:49:16 +0000
> +++ b/sql/structs.h	2011-03-16 15:16:54 +0000
> @@ -233,11 +233,19 @@ typedef struct  user_conn {
>   	/* Bits in form->status */
>   #define STATUS_NO_RECORD	(1+2)	/* Record isn't usably */
>   #define STATUS_GARBAGE		1
> -#define STATUS_NOT_FOUND	2	/* No record in database when needed */
> -#define STATUS_NO_PARENT	4	/* Parent record wasn't found */
> -#define STATUS_NOT_READ		8	/* Record isn't read */
> -#define STATUS_UPDATED		16	/* Record is updated by formula */
> -#define STATUS_NULL_ROW		32	/* table->null_row is set */
> +/**
> +   Means we were searching for a record and didn't find it. This is used by
> +   storage engines (@see handler::index_read_map()) and the Server layer.
> +*/
> +#define STATUS_NOT_FOUND	2
> +/// Reserved for use by multi-table update. Means the record has been updated.
> +#define STATUS_UPDATED		16
> +/**
> +   Means that table->null_row is set. This is an artificial NULL-filled record
> +   (one example: in outer join, if no match has been found in inner table).
> +*/
> +#define STATUS_NULL_ROW		32
> +/// Reserved for use by multi-table delete. Means the record has been deleted.
>   #define STATUS_DELETED		64
>
>   /*
I suggest that you write "row" instead of "record" everywhere for consistency.

Thanks,
Roy
Thread
bzr commit into mysql-trunk branch (guilhem.bichot:3348) Bug#11766522Guilhem Bichot16 Mar
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3348) Bug#11766522Roy Lyseng24 Mar