From 185e24d592e812db2191ddb2a48a4c98f45f0337 Mon Sep 17 00:00:00 2001 From: Martin Hansson Date: Wed, 25 Nov 2009 11:02:25 +0100 Subject: [PATCH] Bug#48459: valgrind errors with query using 'Range checked for each record' There was an error in an internal structure in the range optimizer (SEL_ARG). Bad design causes parts of a data structure not to be initialized when it is in a certain state. All client code must check that this state is not present before trying to access the structure's data. Fixed by - Checking the state before trying to access data (in several places, most of which not covered by test case.) - Copying the keypart id when cloning SEL_ARGs --- mysql-test/r/range.result | 33 ++++++++++++++++++++++++ mysql-test/t/range.test | 34 ++++++++++++++++++++++++- sql/opt_range.cc | 53 +++++++++++++++++++-------------------- 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index ab40eefdc82..64e00521cd2 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -1604,6 +1604,39 @@ str_to_date('', '%Y-%m-%d') 0000-00-00 DROP TABLE t1, t2; # +# Bug#48459: valgrind errors with query using 'Range checked for each +# record' +# +CREATE TABLE t1 ( +a INT, +b CHAR(2), +c INT, +d INT, +KEY ( c ), +KEY ( d, a, b ( 2 ) ), +KEY ( b ( 1 ) ) +); +INSERT INTO t1 VALUES ( NULL, 'a', 1, 2 ), ( NULL, 'a', 1, 2 ), +( 1, 'a', 1, 2 ), ( 1, 'a', 1, 2 ); +CREATE TABLE t2 ( +a INT, +c INT, +e INT, +KEY ( e ) +); +INSERT INTO t2 VALUES ( 1, 1, NULL ), ( 1, 1, NULL ); +# Should not give Valgrind warnings +SELECT 1 +FROM t1, t2 +WHERE t1.d <> '1' AND t1.b > '1' +AND t1.a = t2.a AND t1.c = t2.c; +1 +1 +1 +1 +1 +DROP TABLE t1, t2; +# # Bug #48665: sql-bench's insert test fails due to wrong result # CREATE TABLE t1 (a INT, b INT, PRIMARY KEY (a)); diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 736d65792c5..5d5ad180f1a 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -1260,6 +1260,39 @@ SELECT str_to_date('', '%Y-%m-%d'); DROP TABLE t1, t2; +--echo # +--echo # Bug#48459: valgrind errors with query using 'Range checked for each +--echo # record' +--echo # +CREATE TABLE t1 ( + a INT, + b CHAR(2), + c INT, + d INT, + KEY ( c ), + KEY ( d, a, b ( 2 ) ), + KEY ( b ( 1 ) ) +); + +INSERT INTO t1 VALUES ( NULL, 'a', 1, 2 ), ( NULL, 'a', 1, 2 ), + ( 1, 'a', 1, 2 ), ( 1, 'a', 1, 2 ); + +CREATE TABLE t2 ( + a INT, + c INT, + e INT, + KEY ( e ) +); + +INSERT INTO t2 VALUES ( 1, 1, NULL ), ( 1, 1, NULL ); + +--echo # Should not give Valgrind warnings +SELECT 1 +FROM t1, t2 +WHERE t1.d <> '1' AND t1.b > '1' +AND t1.a = t2.a AND t1.c = t2.c; + +DROP TABLE t1, t2; --echo # --echo # Bug #48665: sql-bench's insert test fails due to wrong result @@ -1280,5 +1313,4 @@ SELECT * FROM t1 FORCE INDEX (PRIMARY) DROP TABLE t1; - --echo End of 5.1 tests diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 9d8d3bd43d9..94204962345 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -446,9 +446,9 @@ public: range_key, *range_key_flag); *range_key_flag|= key_tree->min_flag; if (key_tree->next_key_part && + key_tree->next_key_part->type == SEL_ARG::KEY_RANGE && key_tree->next_key_part->part == key_tree->part+1 && - !(*range_key_flag & (NO_MIN_RANGE | NEAR_MIN)) && - key_tree->next_key_part->type == SEL_ARG::KEY_RANGE) + !(*range_key_flag & (NO_MIN_RANGE | NEAR_MIN))) res+= key_tree->next_key_part->store_min_key(key, range_key, range_key_flag); return res; @@ -462,9 +462,9 @@ public: range_key, *range_key_flag); (*range_key_flag)|= key_tree->max_flag; if (key_tree->next_key_part && + key_tree->next_key_part->type == SEL_ARG::KEY_RANGE && key_tree->next_key_part->part == key_tree->part+1 && - !(*range_key_flag & (NO_MAX_RANGE | NEAR_MAX)) && - key_tree->next_key_part->type == SEL_ARG::KEY_RANGE) + !(*range_key_flag & (NO_MAX_RANGE | NEAR_MAX))) res+= key_tree->next_key_part->store_max_key(key, range_key, range_key_flag); return res; @@ -1700,6 +1700,7 @@ SEL_ARG *SEL_ARG::clone(RANGE_OPT_PARAM *param, SEL_ARG *new_parent, tmp->prev= *next_arg; // Link into next/prev chain (*next_arg)->next=tmp; (*next_arg)= tmp; + tmp->part= this->part; } else { @@ -7290,27 +7291,25 @@ int test_rb_tree(SEL_ARG *element,SEL_ARG *parent) } -/* - Count how many times SEL_ARG graph "root" refers to its part "key" +/** + Count how many times SEL_ARG graph "root" refers to its part "key" via + transitive closure. - SYNOPSIS - count_key_part_usage() - root An RB-Root node in a SEL_ARG graph. - key Another RB-Root node in that SEL_ARG graph. + @param root An RB-Root node in a SEL_ARG graph. + @param key Another RB-Root node in that SEL_ARG graph. - DESCRIPTION - The passed "root" node may refer to "key" node via root->next_key_part, - root->next->n + The passed "root" node may refer to "key" node via root->next_key_part, + root->next->n - This function counts how many times the node "key" is referred (via - SEL_ARG::next_key_part) by - - intervals of RB-tree pointed by "root", - - intervals of RB-trees that are pointed by SEL_ARG::next_key_part from - intervals of RB-tree pointed by "root", - - and so on. + This function counts how many times the node "key" is referred (via + SEL_ARG::next_key_part) by + - intervals of RB-tree pointed by "root", + - intervals of RB-trees that are pointed by SEL_ARG::next_key_part from + intervals of RB-tree pointed by "root", + - and so on. - Here is an example (horizontal links represent next_key_part pointers, - vertical links - next/prev prev pointers): + Here is an example (horizontal links represent next_key_part pointers, + vertical links - next/prev prev pointers): +----+ $ |root|-----------------+ @@ -7330,8 +7329,8 @@ int test_rb_tree(SEL_ARG *element,SEL_ARG *parent) ... +---+ $ | | |------------+ +---+ $ - RETURN - Number of links to "key" from nodes reachable from "root". + @return + Number of links to "key" from nodes reachable from "root". */ static ulong count_key_part_usage(SEL_ARG *root, SEL_ARG *key) @@ -7586,8 +7585,8 @@ check_quick_keys(PARAM *param, uint idx, SEL_ARG *key_tree, param->first_null_comp= key_tree->part+1; if (key_tree->next_key_part && - key_tree->next_key_part->part == key_tree->part+1 && - key_tree->next_key_part->type == SEL_ARG::KEY_RANGE) + key_tree->next_key_part->type == SEL_ARG::KEY_RANGE && + key_tree->next_key_part->part == key_tree->part+1) { // const key as prefix if (min_key_length == max_key_length && !memcmp(min_key, max_key, (uint) (tmp_max_key - max_key)) && @@ -7868,8 +7867,8 @@ get_quick_keys(PARAM *param,QUICK_RANGE_SELECT *quick,KEY_PART *key, &tmp_max_key,max_key_flag); if (key_tree->next_key_part && - key_tree->next_key_part->part == key_tree->part+1 && - key_tree->next_key_part->type == SEL_ARG::KEY_RANGE) + key_tree->next_key_part->type == SEL_ARG::KEY_RANGE && + key_tree->next_key_part->part == key_tree->part+1) { // const key as prefix if ((tmp_min_key - min_key) == (tmp_max_key - max_key) && memcmp(min_key, max_key, (uint)(tmp_max_key - max_key))==0 &&