summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Widenius <monty@askmonty.org>2010-08-23 12:46:25 +0300
committerMichael Widenius <monty@askmonty.org>2010-08-23 12:46:25 +0300
commitb6fe4713fe14c059c4bceca60ed1863a90b7e512 (patch)
treedde4fe858e152b2b411a5b8c1d750629cf228a35
parent096666fcbbf0329676cfd1d8623c1503613f1570 (diff)
downloadmariadb-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.result17
-rw-r--r--mysql-test/r/subselect4.result23
-rw-r--r--mysql-test/t/group_by.test17
-rw-r--r--mysql-test/t/subselect4.test26
-rw-r--r--sql/item.h24
-rw-r--r--sql/item_func.h15
-rw-r--r--sql/item_sum.cc18
-rw-r--r--sql/item_sum.h4
-rw-r--r--sql/sql_select.cc13
-rw-r--r--sql/sql_select.h29
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 */