diff options
author | Michael Widenius <monty@askmonty.org> | 2010-08-23 12:46:25 +0300 |
---|---|---|
committer | Michael Widenius <monty@askmonty.org> | 2010-08-23 12:46:25 +0300 |
commit | b6fe4713fe14c059c4bceca60ed1863a90b7e512 (patch) | |
tree | dde4fe858e152b2b411a5b8c1d750629cf228a35 | |
parent | 096666fcbbf0329676cfd1d8623c1503613f1570 (diff) | |
download | mariadb-git-b6fe4713fe14c059c4bceca60ed1863a90b7e512.tar.gz |
Fix for LP#612894 Some aggregate functions (such as MIN MAX) work incorrectly in subqueries after getting NULL value
mysql-test/r/group_by.result:
Added test that showed problems that no_rows_in_results() didn't work for expressions
mysql-test/r/subselect4.result:
Test case for LP#612894
mysql-test/t/group_by.test:
Added test that showed problems that no_rows_in_results() didn't work for expressions
mysql-test/t/subselect4.test:
Test case for LP#612894
sql/item.h:
Added restore_to_before_no_rows_in_result()
Added function processor for no_rows_in_results() and restore_to_before_no_rows_in_results() to ensure it works with functions
Fix that above functions are handled by Item_ref()
sql/item_func.h:
Ensure that no_rows_in_results() and restore_to_before_no_rows_in_result() are called for all function arguments
sql/item_sum.cc:
Added restore_to_before_no_rows_in_result() to restore settings after Item_sum_hybrid::no_rows_in_result() was called.
This is needed to handle the case where we have made 'make_const()' on the item in opt_sum(), but the item will be reused again in a sub query.
Ignore multiple calls to no_rows_in_result() as Item_ref is calling it twice.
sql/item_sum.h:
Added restore_to_before_no_rows_in_result();
sql/sql_select.cc:
Added reset of no_rows_in_result() for JOIN::reinit()
sql/sql_select.h:
Added marker if no_rows_in_result() is called.
-rw-r--r-- | mysql-test/r/group_by.result | 17 | ||||
-rw-r--r-- | mysql-test/r/subselect4.result | 23 | ||||
-rw-r--r-- | mysql-test/t/group_by.test | 17 | ||||
-rw-r--r-- | mysql-test/t/subselect4.test | 26 | ||||
-rw-r--r-- | sql/item.h | 24 | ||||
-rw-r--r-- | sql/item_func.h | 15 | ||||
-rw-r--r-- | sql/item_sum.cc | 18 | ||||
-rw-r--r-- | sql/item_sum.h | 4 | ||||
-rw-r--r-- | sql/sql_select.cc | 13 | ||||
-rw-r--r-- | sql/sql_select.h | 29 |
10 files changed, 170 insertions, 16 deletions
diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result index 645dd460735..deda00364aa 100644 --- a/mysql-test/r/group_by.result +++ b/mysql-test/r/group_by.result @@ -1809,5 +1809,22 @@ SELECT MAX(t2.a) FROM t2 LEFT JOIN t1 ON t2.a = t1.a; MAX(t2.a) 2 DROP TABLE t1, t2; +CREATE TABLE t1 (a int(11) NOT NULL); +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 ( +key_col int(11) NOT NULL, +KEY (key_col) +); +INSERT INTO t2 VALUES (1),(2); +select min(t2.key_col) from t1,t2 where t1.a=1; +min(t2.key_col) +1 +select min(t2.key_col) from t1,t2 where t1.a > 1000; +min(t2.key_col) +NULL +select min(t2.key_col)+1 from t1,t2 where t1.a> 1000; +min(t2.key_col)+1 +NULL +drop table t1,t2; # # End of 5.1 tests diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result index 482e0045840..153fbed9622 100644 --- a/mysql-test/r/subselect4.result +++ b/mysql-test/r/subselect4.result @@ -59,3 +59,26 @@ FROM t3 WHERE 1 = 0 GROUP BY 1; (SELECT 1 FROM t1,t2 WHERE t2.b > t3.b) DROP TABLE t1,t2,t3; End of 5.0 tests. +CREATE TABLE t1 (col_int_nokey int(11) NOT NULL, col_varchar_nokey varchar(1) NOT NULL) engine=myisam; +INSERT INTO t1 VALUES (2,'s'),(0,'v'),(2,'s'); +CREATE TABLE t2 ( +pk int(11) NOT NULL AUTO_INCREMENT, +`col_int_key` int(11) NOT NULL, +col_varchar_key varchar(1) NOT NULL, +PRIMARY KEY (pk), +KEY `col_int_key` (`col_int_key`), +KEY `col_varchar_key` (`col_varchar_key`) +) ENGINE=MyISAM; +INSERT INTO t2 VALUES (4,10,'g'), (5,20,'v'); +SELECT t1.col_int_nokey,(SELECT MIN( t2_a.col_int_key ) FROM t2 t2_a, t2 t2_b, t1 t1_a WHERE t1_a.col_varchar_nokey = t2_b.col_varchar_key and t1.col_int_nokey ) as sub FROM t1; +col_int_nokey sub +2 10 +0 NULL +2 10 +SELECT t1.col_int_nokey,(SELECT MIN( t2_a.col_int_key ) +1 FROM t2 t2_a, t2 t2_b, t1 t1_a WHERE t1_a.col_varchar_nokey = t2_b.col_varchar_key and t1.col_int_nokey ) as sub FROM t1; +col_int_nokey sub +2 11 +0 NULL +2 11 +DROP TABLE t1,t2; +End of 5.1 tests. diff --git a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test index c5b27ee1a62..c640a58d597 100644 --- a/mysql-test/t/group_by.test +++ b/mysql-test/t/group_by.test @@ -1219,6 +1219,23 @@ EXPLAIN SELECT MAX(t2.a) FROM t2 LEFT JOIN t1 ON t2.a = t1.a; SELECT MAX(t2.a) FROM t2 LEFT JOIN t1 ON t2.a = t1.a; DROP TABLE t1, t2; +# +# min() returns wrong value when used in expression when there is no matching +# rows +# + +CREATE TABLE t1 (a int(11) NOT NULL); +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 ( + key_col int(11) NOT NULL, + KEY (key_col) +); +INSERT INTO t2 VALUES (1),(2); + +select min(t2.key_col) from t1,t2 where t1.a=1; +select min(t2.key_col) from t1,t2 where t1.a > 1000; +select min(t2.key_col)+1 from t1,t2 where t1.a> 1000; +drop table t1,t2; --echo # diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test index 440eca22828..c287091ab54 100644 --- a/mysql-test/t/subselect4.test +++ b/mysql-test/t/subselect4.test @@ -62,3 +62,29 @@ FROM t3 WHERE 1 = 0 GROUP BY 1; DROP TABLE t1,t2,t3; --echo End of 5.0 tests. + +# +# Fix for LP#612894 +# Some aggregate functions (such as MIN MAX) work incorrectly in subqueries +# after getting NULL value +# + +CREATE TABLE t1 (col_int_nokey int(11) NOT NULL, col_varchar_nokey varchar(1) NOT NULL) engine=myisam; +INSERT INTO t1 VALUES (2,'s'),(0,'v'),(2,'s'); +CREATE TABLE t2 ( + pk int(11) NOT NULL AUTO_INCREMENT, + `col_int_key` int(11) NOT NULL, + col_varchar_key varchar(1) NOT NULL, + PRIMARY KEY (pk), + KEY `col_int_key` (`col_int_key`), + KEY `col_varchar_key` (`col_varchar_key`) +) ENGINE=MyISAM; +INSERT INTO t2 VALUES (4,10,'g'), (5,20,'v'); + +SELECT t1.col_int_nokey,(SELECT MIN( t2_a.col_int_key ) FROM t2 t2_a, t2 t2_b, t1 t1_a WHERE t1_a.col_varchar_nokey = t2_b.col_varchar_key and t1.col_int_nokey ) as sub FROM t1; + +SELECT t1.col_int_nokey,(SELECT MIN( t2_a.col_int_key ) +1 FROM t2 t2_a, t2 t2_b, t1 t1_a WHERE t1_a.col_varchar_nokey = t2_b.col_varchar_key and t1.col_int_nokey ) as sub FROM t1; + +DROP TABLE t1,t2; + +--echo End of 5.1 tests. diff --git a/sql/item.h b/sql/item.h index 45cfb4881dd..af0d7648689 100644 --- a/sql/item.h +++ b/sql/item.h @@ -852,6 +852,7 @@ public: set value of aggregate function in case of no rows for grouping were found */ virtual void no_rows_in_result() {} + virtual void restore_to_before_no_rows_in_result() {} virtual Item *copy_or_same(THD *thd) { return this; } virtual Item *copy_andor_structure(THD *thd) { return this; } virtual Item *real_item() { return this; } @@ -908,6 +909,21 @@ public: virtual bool register_field_in_read_map(uchar *arg) { return 0; } virtual bool enumerate_field_refs_processor(uchar *arg) { return 0; } virtual bool mark_as_eliminated_processor(uchar *arg) { return 0; } + + /* To call bool function for all arguments */ + struct bool_func_call_args + { + Item *original_func_item; + void (Item::*bool_function)(); + }; + bool call_bool_func_processor(uchar *org_item) + { + bool_func_call_args *info= (bool_func_call_args*) org_item; + /* Avoid recursion, as walk also calls for original item */ + if (info->original_func_item != this) + (this->*(info->bool_function))(); + return FALSE; + } /* Check if a partition function is allowed SYNOPSIS @@ -2321,6 +2337,14 @@ public: return (*ref)->walk(processor, walk_subquery, arg) || (this->*processor)(arg); } + void no_rows_in_result() + { + (*ref)->no_rows_in_result(); + } + void restore_to_before_no_rows_in_result() + { + (*ref)->restore_to_before_no_rows_in_result(); + } virtual void print(String *str, enum_query_type query_type); bool result_as_longlong() { diff --git a/sql/item_func.h b/sql/item_func.h index b7994bd941e..2b466960af2 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -217,6 +217,21 @@ public: { return functype() == *(Functype *) arg; } + + void no_rows_in_result() + { + bool_func_call_args info; + info.original_func_item= this; + info.bool_function= &Item::no_rows_in_result; + walk(&Item::call_bool_func_processor, FALSE, (uchar*) &info); + } + void restore_to_before_no_rows_in_result() + { + bool_func_call_args info; + info.original_func_item= this; + info.bool_function= &Item::restore_to_before_no_rows_in_result; + walk(&Item::call_bool_func_processor, FALSE, (uchar*) &info); + } }; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 2f79fd65ff3..b3b07119066 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -1638,8 +1638,22 @@ void Item_sum_hybrid::cleanup() void Item_sum_hybrid::no_rows_in_result() { - was_values= FALSE; - clear(); + /* We may be called here twice in case of ref field in function */ + if (was_values) + { + was_values= FALSE; + was_null_value= value->null_value; + clear(); + } +} + +void Item_sum_hybrid::restore_to_before_no_rows_in_result() +{ + if (!was_values) + { + was_values= TRUE; + null_value= value->null_value= was_null_value; + } } diff --git a/sql/item_sum.h b/sql/item_sum.h index b82f2d961f1..ac6a56400a4 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -496,7 +496,7 @@ public: enum Sumfunctype sum_func () const { return SUM_DISTINCT_FUNC; } void reset_field() {} // not used void update_field() {} // not used - virtual void no_rows_in_result() {} + void no_rows_in_result() {} void fix_length_and_dec(); enum Item_result result_type () const { return val.traits->type(); } virtual void calculate_val_and_count(); @@ -845,6 +845,7 @@ protected: enum_field_types hybrid_field_type; int cmp_sign; bool was_values; // Set if we have found at least one row (for max/min only) + bool was_null_value; public: Item_sum_hybrid(Item *item_par,int sign) @@ -876,6 +877,7 @@ protected: void cleanup(); bool any_value() { return was_values; } void no_rows_in_result(); + void restore_to_before_no_rows_in_result(); Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length); }; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 60da06fadf1..50fe78492c0 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1688,6 +1688,16 @@ JOIN::reinit() func->clear(); } + if (no_rows_in_result_called) + { + /* Reset effect of possible no_rows_in_result() */ + List_iterator_fast<Item> it(fields_list); + Item *item; + + no_rows_in_result_called= 0; + while ((item= it++)) + item->restore_to_before_no_rows_in_result(); + } DBUG_RETURN(0); } @@ -12681,8 +12691,11 @@ end_send_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), { List_iterator_fast<Item> it(*join->fields); Item *item; + DBUG_PRINT("info", ("no matching rows")); + /* No matching rows for group function */ join->clear(); + join->no_rows_in_result_called= 1; while ((item= it++)) item->no_rows_in_result(); diff --git a/sql/sql_select.h b/sql/sql_select.h index 285019b4a5c..83aa1ec1661 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -365,24 +365,26 @@ public: the number of rows in it may vary from one subquery execution to another. */ bool no_const_tables; + bool no_rows_in_result_called; /** Copy of this JOIN to be used with temporary tables. - tmp_join is used when the JOIN needs to be "reusable" (e.g. in a subquery - that gets re-executed several times) and we know will use temporary tables - for materialization. The materialization to a temporary table overwrites the - JOIN structure to point to the temporary table after the materialization is - done. This is where tmp_join is used : it's a copy of the JOIN before the - materialization and is used in restoring before re-execution by overwriting - the current JOIN structure with the saved copy. - Because of this we should pay extra care of not freeing up helper structures - that are referenced by the original contents of the JOIN. We can check for - this by making sure the "current" join is not the temporary copy, e.g. - !tmp_join || tmp_join != join + tmp_join is used when the JOIN needs to be "reusable" (e.g. in a + subquery that gets re-executed several times) and we know will use + temporary tables for materialization. The materialization to a + temporary table overwrites the JOIN structure to point to the + temporary table after the materialization is done. This is where + tmp_join is used : it's a copy of the JOIN before the + materialization and is used in restoring before re-execution by + overwriting the current JOIN structure with the saved copy. + Because of this we should pay extra care of not freeing up helper + structures that are referenced by the original contents of the + JOIN. We can check for this by making sure the "current" join is + not the temporary copy, e.g. !tmp_join || tmp_join != join - We should free these sub-structures at JOIN::destroy() if the "current" join - has a copy is not that copy. + We should free these sub-structures at JOIN::destroy() if the + "current" join has a copy is not that copy. */ JOIN *tmp_join; ROLLUP rollup; ///< Used with rollup @@ -512,6 +514,7 @@ public: optimized= 0; cond_equal= 0; group_optimized_away= 0; + no_rows_in_result_called= 0; all_fields= fields_arg; if (&fields_list != &fields_arg) /* Avoid valgrind-warning */ |