From d7d0f6390b8d95b34ffb79f1a77ab11657e064a8 Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Fri, 27 Aug 2010 13:44:35 +0400 Subject: [PATCH] Bug #54465: assert: field_types == 0 || field_types[field_pos] == MYSQL_TYPE_LONGLONG A MIN/MAX() function with a subquery as its argument could lead to a debug assertion on debug builds or wrong data on release ones. The problem was a combination of the following factors: - Item_sum_hybrid::fix_fields() might use the argument (args[0]) to calculate 'hybrid_field_type' which was later used to decide how the data should be sent to the client. - Item_sum::make_field() might use the argument again to calculate the field's type when sending result set metadata to the client. - The argument could be changed in between these two calls via Item::set_arg() leading to inconsistent metadata being reported. Here is what was happening for the bug's test case: 1. Item_sum_hybrid::fix_fields() calculates hybrid_field_type as MYSQL_TYPE_LONGLONG based on args[0] which is an Item::SUBSELECT_ITEM at that time. 2. A temporary table is created to execute the query. create_tmp_field_from_item() creates a Field_long object according to the subselect's max_length. 3. The subselect item in Item_sum_hybrid is replaced by the Item_field object referencing the newly created Field_long. 4. Item_sum::make_field() rightfully returns the MYSQL_TYPE_LONG type when calculating the result set metadata. 5. When sending the actual data, Item::send() relies on the virtual field_type() function which in our case returns previously calculated hybrid_field_type == MYSQL_TYPE_LONGLONG. It looks like the only solution is to never refer to the argument's metadata after the result metadata has been calculated in fix_fields(), since the argument itself may be different by then. In this sense, Item_sum::make_field() should never be used, because it may rely on the argument's metadata and is only called after fix_fields(). The "default" implementation in Item::make_field() should be used instead as it relies only on field_type(), but not on the argument's type. Fixed by removing Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. mysql-test/r/func_group.result: Added a test case for bug #54465. mysql-test/t/func_group.test: Added a test case for bug #54465. sql/item_sum.cc: Removed Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. sql/item_sum.h: Removed Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. --- mysql-test/r/func_group.result | 11 +++++++++++ mysql-test/t/func_group.test | 14 ++++++++++++++ sql/item_sum.cc | 20 -------------------- sql/item_sum.h | 1 - 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/mysql-test/r/func_group.result b/mysql-test/r/func_group.result index b36f561578b..606f879b47f 100644 --- a/mysql-test/r/func_group.result +++ b/mysql-test/r/func_group.result @@ -1713,4 +1713,15 @@ f1 f2 f3 f4 f1 = f2 NULL NULL NULL NULL NULL drop table t1; # +# Bug #54465: assert: field_types == 0 || field_types[field_pos] == +# MYSQL_TYPE_LONGLONG +# +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); +SELECT MAX((SELECT 1 FROM t1 ORDER BY @var LIMIT 1)) m FROM t1 t2, t1 +ORDER BY t1.a; +m +1 +DROP TABLE t1; +# End of 5.1 tests diff --git a/mysql-test/t/func_group.test b/mysql-test/t/func_group.test index 6dbc8a05789..72a78f612a2 100644 --- a/mysql-test/t/func_group.test +++ b/mysql-test/t/func_group.test @@ -1082,6 +1082,20 @@ select a.f1 as a, b.f4 as b, a.f1 > b.f4 as gt, from t1 a, t1 b; select *, f1 = f2 from t1; drop table t1; + +--echo # +--echo # Bug #54465: assert: field_types == 0 || field_types[field_pos] == +--echo # MYSQL_TYPE_LONGLONG +--echo # + +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); + +SELECT MAX((SELECT 1 FROM t1 ORDER BY @var LIMIT 1)) m FROM t1 t2, t1 + ORDER BY t1.a; + +DROP TABLE t1; + --echo # --echo End of 5.1 tests diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 25b3bd5d91d..ae9e46e2abf 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -417,26 +417,6 @@ void Item_sum::mark_as_sum_func() } -void Item_sum::make_field(Send_field *tmp_field) -{ - if (args[0]->type() == Item::FIELD_ITEM && keep_field_type()) - { - ((Item_field*) args[0])->field->make_field(tmp_field); - /* For expressions only col_name should be non-empty string. */ - char *empty_string= (char*)""; - tmp_field->db_name= empty_string; - tmp_field->org_table_name= empty_string; - tmp_field->table_name= empty_string; - tmp_field->org_col_name= empty_string; - tmp_field->col_name= name; - if (maybe_null) - tmp_field->flags&= ~NOT_NULL_FLAG; - } - else - init_make_field(tmp_field, field_type()); -} - - void Item_sum::print(String *str, enum_query_type query_type) { /* orig_args is not filled with valid values until fix_fields() */ diff --git a/sql/item_sum.h b/sql/item_sum.h index fe05858ab1d..26290a812f4 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -339,7 +339,6 @@ public: forced_const= TRUE; } virtual bool const_item() const { return forced_const; } - void make_field(Send_field *field); virtual void print(String *str, enum_query_type query_type); void fix_num_length_and_dec();