diff --git a/mysql-test/r/group_min_max_innodb.result b/mysql-test/r/group_min_max_innodb.result index 320c4b2b750..ad8dde69548 100644 --- a/mysql-test/r/group_min_max_innodb.result +++ b/mysql-test/r/group_min_max_innodb.result @@ -118,3 +118,171 @@ COUNT(DISTINCT a) 1 DROP TABLE t1; End of 5.5 tests +# +# Bug#17909656 - WRONG RESULTS FOR A SIMPLE QUERY WITH GROUP BY +# +CREATE TABLE t0 ( +i1 INTEGER NOT NULL +); +INSERT INTO t0 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10), +(11),(12),(13),(14),(15),(16),(17),(18),(19),(20), +(21),(22),(23),(24),(25),(26),(27),(28),(29),(30); +CREATE TABLE t1 ( +c1 CHAR(1) NOT NULL, +i1 INTEGER NOT NULL, +i2 INTEGER NOT NULL, +UNIQUE KEY k1 (c1,i2) +) ENGINE=InnoDB; +INSERT INTO t1 SELECT 'A',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'B',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'C',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'D',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'E',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'F',i1,i1 FROM t0; +CREATE TABLE t2 ( +c1 CHAR(1) NOT NULL, +i1 INTEGER NOT NULL, +i2 INTEGER NOT NULL, +UNIQUE KEY k2 (c1,i1,i2) +) ENGINE=InnoDB; +INSERT INTO t2 SELECT 'A',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'B',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'C',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'D',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'E',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'F',i1,i1 FROM t0; +ANALYZE TABLE t1; +ANALYZE TABLE t2; +EXPLAIN SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' AND i2 = 17) OR ( c1 = 'F') +GROUP BY c1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range k1 k1 5 NULL 31 Using where; Using index +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' AND i2 = 17) OR ( c1 = 'F') +GROUP BY c1; +c1 max(i2) +C 17 +F 30 +EXPLAIN SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR ( c1 = 'F' AND i2 = 17)) +GROUP BY c1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range k1 k1 5 NULL 31 Using where; Using index +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR ( c1 = 'F' AND i2 = 17)) +GROUP BY c1; +c1 max(i2) +C 30 +F 17 +EXPLAIN SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR c1 = 'F' ) AND ( i2 = 17 ) +GROUP BY c1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range k1 k1 5 NULL 1 Using where; Using index for group-by +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR c1 = 'F' ) AND ( i2 = 17 ) +GROUP BY c1; +c1 max(i2) +C 17 +F 17 +EXPLAIN SELECT c1, max(i2) FROM t1 +WHERE ((c1 = 'C' AND (i2 = 40 OR i2 = 30)) OR ( c1 = 'F' AND (i2 = 40 ))) +GROUP BY c1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range k1 k1 5 NULL 3 Using where; Using index +SELECT c1, max(i2) FROM t1 +WHERE ((c1 = 'C' AND (i2 = 40 OR i2 = 30)) OR ( c1 = 'F' AND (i2 = 40 ))) +GROUP BY c1; +c1 max(i2) +C 30 +EXPLAIN SELECT c1, i1, max(i2) FROM t2 +WHERE (c1 = 'C' OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 ) +GROUP BY c1,i1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range k2 k2 9 NULL 59 Using where; Using index for group-by +SELECT c1, i1, max(i2) FROM t2 +WHERE (c1 = 'C' OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 ) +GROUP BY c1,i1; +c1 i1 max(i2) +C 17 17 +F 17 17 +EXPLAIN SELECT c1, i1, max(i2) FROM t2 +WHERE (((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 )) +GROUP BY c1,i1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range k2 k2 9 NULL 58 Using where; Using index for group-by +SELECT c1, i1, max(i2) FROM t2 +WHERE (((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 )) +GROUP BY c1,i1; +c1 i1 max(i2) +C 17 17 +F 17 17 +EXPLAIN SELECT c1, i1, max(i2) FROM t2 +WHERE ((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35) OR ( i2 = 17 )) +GROUP BY c1,i1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range k2 k2 5 NULL 181 Using where; Using index for group-by +SELECT c1, i1, max(i2) FROM t2 +WHERE ((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35) OR ( i2 = 17 )) +GROUP BY c1,i1; +c1 i1 max(i2) +A 17 17 +B 17 17 +C 1 1 +C 2 2 +C 3 3 +C 4 4 +C 5 5 +C 6 6 +C 7 7 +C 8 8 +C 9 9 +C 10 10 +C 11 11 +C 12 12 +C 13 13 +C 14 14 +C 15 15 +C 16 16 +C 17 17 +C 18 18 +C 19 19 +C 20 20 +C 21 21 +C 22 22 +C 23 23 +C 24 24 +C 25 25 +C 26 26 +C 27 27 +C 28 28 +C 29 29 +C 30 30 +D 17 17 +E 17 17 +F 1 1 +F 2 2 +F 3 3 +F 4 4 +F 5 5 +F 6 6 +F 7 7 +F 8 8 +F 9 9 +F 10 10 +F 11 11 +F 12 12 +F 13 13 +F 14 14 +F 15 15 +F 16 16 +F 17 17 +F 18 18 +F 19 19 +F 20 20 +F 21 21 +F 22 22 +F 23 23 +F 24 24 +F 25 25 +F 26 26 +F 27 27 +F 28 28 +F 29 29 +F 30 30 +DROP TABLE t0,t1,t2; diff --git a/mysql-test/t/group_min_max_innodb.test b/mysql-test/t/group_min_max_innodb.test index 7038eb2ff47..6967f847147 100644 --- a/mysql-test/t/group_min_max_innodb.test +++ b/mysql-test/t/group_min_max_innodb.test @@ -137,3 +137,96 @@ SELECT COUNT(DISTINCT a) FROM t1 WHERE b = 'b'; DROP TABLE t1; --echo End of 5.5 tests + +--echo # +--echo # Bug#17909656 - WRONG RESULTS FOR A SIMPLE QUERY WITH GROUP BY +--echo # + +CREATE TABLE t0 ( + i1 INTEGER NOT NULL +); + +INSERT INTO t0 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10), + (11),(12),(13),(14),(15),(16),(17),(18),(19),(20), + (21),(22),(23),(24),(25),(26),(27),(28),(29),(30); + +CREATE TABLE t1 ( + c1 CHAR(1) NOT NULL, + i1 INTEGER NOT NULL, + i2 INTEGER NOT NULL, + UNIQUE KEY k1 (c1,i2) +) ENGINE=InnoDB; + +INSERT INTO t1 SELECT 'A',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'B',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'C',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'D',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'E',i1,i1 FROM t0; +INSERT INTO t1 SELECT 'F',i1,i1 FROM t0; + +CREATE TABLE t2 ( + c1 CHAR(1) NOT NULL, + i1 INTEGER NOT NULL, + i2 INTEGER NOT NULL, + UNIQUE KEY k2 (c1,i1,i2) +) ENGINE=InnoDB; + +INSERT INTO t2 SELECT 'A',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'B',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'C',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'D',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'E',i1,i1 FROM t0; +INSERT INTO t2 SELECT 'F',i1,i1 FROM t0; + +-- disable_result_log +ANALYZE TABLE t1; +ANALYZE TABLE t2; +-- enable_result_log + +let query= +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' AND i2 = 17) OR ( c1 = 'F') +GROUP BY c1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR ( c1 = 'F' AND i2 = 17)) +GROUP BY c1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, max(i2) FROM t1 WHERE (c1 = 'C' OR c1 = 'F' ) AND ( i2 = 17 ) +GROUP BY c1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, max(i2) FROM t1 +WHERE ((c1 = 'C' AND (i2 = 40 OR i2 = 30)) OR ( c1 = 'F' AND (i2 = 40 ))) +GROUP BY c1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, i1, max(i2) FROM t2 +WHERE (c1 = 'C' OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 ) +GROUP BY c1,i1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, i1, max(i2) FROM t2 +WHERE (((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35)) AND ( i2 = 17 )) +GROUP BY c1,i1; +eval EXPLAIN $query; +eval $query; + +let query= +SELECT c1, i1, max(i2) FROM t2 +WHERE ((c1 = 'C' AND i1 < 40) OR ( c1 = 'F' AND i1 < 35) OR ( i2 = 17 )) +GROUP BY c1,i1; +eval EXPLAIN $query; +eval $query; + +DROP TABLE t0,t1,t2; diff --git a/sql/opt_range.cc b/sql/opt_range.cc index c7a7d2531af..05da107f0ea 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -1,4 +1,5 @@ -/* Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2000, 2014, 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 @@ -304,31 +305,54 @@ public: :min_flag(0),elements(1),use_count(1),left(0),right(0),next_key_part(0), color(BLACK), type(type_arg) {} - inline bool is_same(SEL_ARG *arg) + /** + returns true if a range predicate is equal. Use all_same() + to check for equality of all the predicates on this keypart. + */ + inline bool is_same(const SEL_ARG *arg) const { if (type != arg->type || part != arg->part) - return 0; + return false; if (type != KEY_RANGE) - return 1; + return true; return cmp_min_to_min(arg) == 0 && cmp_max_to_max(arg) == 0; } + /** + returns true if all the predicates in the keypart tree are equal + */ + bool all_same(const SEL_ARG *arg) const + { + if (type != arg->type || part != arg->part) + return false; + if (type != KEY_RANGE) + return true; + if (arg == this) + return true; + const SEL_ARG *cmp_arg= arg->first(); + const SEL_ARG *cur_arg= first(); + for (; cur_arg && cmp_arg && cur_arg->is_same(cmp_arg); + cur_arg= cur_arg->next, cmp_arg= cmp_arg->next); + if (cur_arg || cmp_arg) + return false; + return true; + } inline void merge_flags(SEL_ARG *arg) { maybe_flag|=arg->maybe_flag; } inline void maybe_smaller() { maybe_flag=1; } /* Return true iff it's a single-point null interval */ inline bool is_null_interval() { return maybe_null && max_value[0] == 1; } - inline int cmp_min_to_min(SEL_ARG* arg) + inline int cmp_min_to_min(const SEL_ARG* arg) const { return sel_cmp(field,min_value, arg->min_value, min_flag, arg->min_flag); } - inline int cmp_min_to_max(SEL_ARG* arg) + inline int cmp_min_to_max(const SEL_ARG* arg) const { return sel_cmp(field,min_value, arg->max_value, min_flag, arg->max_flag); } - inline int cmp_max_to_max(SEL_ARG* arg) + inline int cmp_max_to_max(const SEL_ARG* arg) const { return sel_cmp(field,max_value, arg->max_value, max_flag, arg->max_flag); } - inline int cmp_max_to_min(SEL_ARG* arg) + inline int cmp_max_to_min(const SEL_ARG* arg) const { return sel_cmp(field,max_value, arg->min_value, max_flag, arg->min_flag); } @@ -507,6 +531,7 @@ public: void test_use_count(SEL_ARG *root); #endif SEL_ARG *first(); + const SEL_ARG *first() const; SEL_ARG *last(); void make_root(); inline bool simple_key() @@ -583,6 +608,18 @@ public: SEL_ARG *clone_tree(RANGE_OPT_PARAM *param); }; +/** + Helper function to compare two SEL_ARG's. +*/ +static bool all_same(const SEL_ARG *sa1, const SEL_ARG *sa2) +{ + if (sa1 == NULL && sa2 == NULL) + return true; + if ((sa1 != NULL && sa2 == NULL) || (sa1 == NULL && sa2 != NULL)) + return false; + return sa1->all_same(sa2); +} + class SEL_IMERGE; @@ -1770,6 +1807,13 @@ SEL_ARG *SEL_ARG::clone(RANGE_OPT_PARAM *param, SEL_ARG *new_parent, return tmp; } +/** + This gives the first SEL_ARG in the interval list, and the minimal element + in the red-black tree + + @return + SEL_ARG first SEL_ARG in the interval list +*/ SEL_ARG *SEL_ARG::first() { SEL_ARG *next_arg=this; @@ -1780,6 +1824,11 @@ SEL_ARG *SEL_ARG::first() return next_arg; } +const SEL_ARG *SEL_ARG::first() const +{ + return const_cast(this)->first(); +} + SEL_ARG *SEL_ARG::last() { SEL_ARG *next_arg=this; @@ -9356,6 +9405,8 @@ void QUICK_ROR_UNION_SELECT::add_keys_and_lengths(String *key_names, static inline uint get_field_keypart(KEY *index, Field *field); static inline SEL_ARG * get_index_range_tree(uint index, SEL_TREE* range_tree, PARAM *param, uint *param_idx); +static bool get_sel_arg_for_keypart(Field *field, SEL_ARG *index_range_tree, + SEL_ARG **cur_range); static bool get_constant_key_infix(KEY *index_info, SEL_ARG *index_range_tree, KEY_PART_INFO *first_non_group_part, KEY_PART_INFO *min_max_arg_part, @@ -9451,6 +9502,8 @@ cost_group_min_max(TABLE* table, KEY *index_info, uint used_key_parts, above tests. By transitivity then it also follows that each WA_i participates in the index I (if this was already tested for GA, NGA and C). + WA2. If there is a predicate on C, then it must be in conjunction + to all predicates on all earlier keyparts in I. C) Overall query form: SELECT EXPR([A_1,...,A_k], [B_1,...,B_m], [MIN(C)], [MAX(C)]) @@ -9856,6 +9909,25 @@ get_best_group_min_max(PARAM *param, SEL_TREE *tree, double read_time) } } + /** + Test WA2:If there are conditions on a column C participating in + MIN/MAX, those conditions must be conjunctions to all earlier + keyparts. Otherwise, Loose Index Scan cannot be used. + */ + if (tree && min_max_arg_item) + { + uint dummy; + SEL_ARG *index_range_tree= get_index_range_tree(cur_index, tree, param, + &dummy); + SEL_ARG *cur_range= NULL; + if (get_sel_arg_for_keypart(min_max_arg_part->field, + index_range_tree, &cur_range) || + (cur_range && cur_range->type != SEL_ARG::KEY_RANGE)) + { + goto next_index; + } + } + /* If we got to this point, cur_index_info passes the test. */ key_infix_parts= cur_key_infix_len ? (uint) (first_non_infix_part - first_non_group_part) : 0; @@ -10099,73 +10171,75 @@ check_group_min_max_predicates(COND *cond, Item_field *min_max_arg_item, /* - Get SEL_ARG tree, if any, for the keypart covering non grouping - attribute (NGA) field 'nga_field'. + Get the SEL_ARG tree 'tree' for the keypart covering 'field', if + any. 'tree' must be a unique conjunction to ALL predicates in earlier + keyparts of 'keypart_tree'. - This function enforces the NGA3 test: If 'keypart_tree' contains a - condition for 'nga_field', there can only be one range. In the - opposite case, this function returns with error and 'cur_range' - should not be used. + E.g., if 'keypart_tree' is for a composite index (kp1,kp2) and kp2 + covers 'field', all these conditions satisfies the requirement: - Note that the NGA1 and NGA2 requirements, like whether or not the - range predicate for 'nga_field' is equality, is not tested by this - function. + 1. "(kp1=2 OR kp1=3) AND kp2=10" => returns "kp2=10" + 2. "(kp1=2 AND kp2=10) OR (kp1=3 AND kp2=10)" => returns "kp2=10" + 3. "(kp1=2 AND (kp2=10 OR kp2=11)) OR (kp1=3 AND (kp2=10 OR kp2=11))" + => returns "kp2=10 OR kp2=11" - @param[in] nga_field The NGA field we want the SEL_ARG tree for + whereas these do not + 1. "(kp1=2 AND kp2=10) OR kp1=3" + 2. "(kp1=2 AND kp2=10) OR (kp1=3 AND kp2=11)" + 3. "(kp1=2 AND kp2=10) OR (kp1=3 AND (kp2=10 OR kp2=11))" + + This function effectively tests requirement WA2. In combination with + a test that the returned tree has no more than one range it is also + a test of NGA3. + + @param[in] field The field we want the SEL_ARG tree for @param[in] keypart_tree Root node of the SEL_ARG* tree for the index @param[out] cur_range The SEL_ARG tree, if any, for the keypart covering field 'keypart_field' - @retval true 'keypart_tree' contained a predicate for 'nga_field' but - multiple ranges exists. 'cur_range' should not be used. + @retval true 'keypart_tree' contained a predicate for 'field' that + is not conjunction to all predicates on earlier keyparts @retval false otherwise */ static bool -get_sel_arg_for_keypart(Field *nga_field, +get_sel_arg_for_keypart(Field *field, SEL_ARG *keypart_tree, SEL_ARG **cur_range) { - if(keypart_tree == NULL) + if (keypart_tree == NULL) return false; - if(keypart_tree->field->eq(nga_field)) + if (keypart_tree->field->eq(field)) { - /* - Enforce NGA3: If a condition for nga_field has been found, only - a single range is allowed. - */ - if (keypart_tree->prev || keypart_tree->next) - return true; // There are multiple ranges - *cur_range= keypart_tree; return false; } - SEL_ARG *found_tree= NULL; + SEL_ARG *tree_first_range= NULL; SEL_ARG *first_kp= keypart_tree->first(); - for (SEL_ARG *cur_kp= first_kp; cur_kp && !found_tree; - cur_kp= cur_kp->next) + for (SEL_ARG *cur_kp= first_kp; cur_kp; cur_kp= cur_kp->next) { + SEL_ARG *curr_tree= NULL; if (cur_kp->next_key_part) { - if (get_sel_arg_for_keypart(nga_field, + if (get_sel_arg_for_keypart(field, cur_kp->next_key_part, - &found_tree)) + &curr_tree)) return true; - } /* - Enforce NGA3: If a condition for nga_field has been found,only - a single range is allowed. - */ - if (found_tree && first_kp->next) - return true; // There are multiple ranges + Check if the SEL_ARG tree for 'field' is identical for all ranges in + 'keypart_tree + */ + if (cur_kp == first_kp) + tree_first_range= curr_tree; + else if (!all_same(tree_first_range, curr_tree)) + return true; } - *cur_range= found_tree; + *cur_range= tree_first_range; return false; } - /* Extract a sequence of constants from a conjunction of equality predicates. @@ -10188,7 +10262,8 @@ get_sel_arg_for_keypart(Field *nga_field, (const_ci = NG_i).. In addition, there can only be one range when there is such a gap. Thus all the NGF_i attributes must fill the 'gap' between the last group-by - attribute and the MIN/MAX attribute in the index (if present). If these + attribute and the MIN/MAX attribute in the index (if present). Also ensure + that there is only a single range on NGF_i (NGA3). If these conditions hold, copy each constant from its corresponding predicate into key_infix, in the order its NG_i attribute appears in the index, and update key_infix_len with the total length of the key parts in key_infix. @@ -10197,7 +10272,6 @@ get_sel_arg_for_keypart(Field *nga_field, TRUE if the index passes the test FALSE o/w */ - static bool get_constant_key_infix(KEY *index_info, SEL_ARG *index_range_tree, KEY_PART_INFO *first_non_group_part, @@ -10217,32 +10291,42 @@ get_constant_key_infix(KEY *index_info, SEL_ARG *index_range_tree, { cur_range= NULL; /* - Find the range tree for the current keypart. We assume that - index_range_tree points to the first keypart in the index. + Check NGA3: + 1. get_sel_arg_for_keypart gets the range tree for the 'field' and also + checks for a unique conjunction of this tree with all the predicates + on the earlier keyparts in the index. + 2. Check for multiple ranges on the found keypart tree. + + We assume that index_range_tree points to the leftmost keypart in + the index. */ - if(get_sel_arg_for_keypart(cur_part->field, index_range_tree, &cur_range)) + if (get_sel_arg_for_keypart(cur_part->field, index_range_tree, + &cur_range)) + return false; + + if (cur_range && cur_range->elements > 1) return false; if (!cur_range) { if (min_max_arg_part) - return FALSE; /* The current keypart has no range predicates at all. */ + return false; /* The current keypart has no range predicates at all. */ else { *first_non_infix_part= cur_part; - return TRUE; + return true; } } if ((cur_range->min_flag & NO_MIN_RANGE) || (cur_range->max_flag & NO_MAX_RANGE) || (cur_range->min_flag & NEAR_MIN) || (cur_range->max_flag & NEAR_MAX)) - return FALSE; + return false; uint field_length= cur_part->store_length; if (cur_range->maybe_null && cur_range->min_value[0] && cur_range->max_value[0]) - { + { /* cur_range specifies 'IS NULL'. In this case the argument points to a "null value" (is_null_string) that may not always be long @@ -10261,7 +10345,7 @@ get_constant_key_infix(KEY *index_info, SEL_ARG *index_range_tree, *key_infix_len+= field_length; } else - return FALSE; + return false; } if (!min_max_arg_part && (cur_part == last_part))