Commit d7d0f639 authored by Alexey Kopytov's avatar Alexey Kopytov

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.
parent 27d5b6e5
...@@ -1713,4 +1713,15 @@ f1 f2 f3 f4 f1 = f2 ...@@ -1713,4 +1713,15 @@ f1 f2 f3 f4 f1 = f2
NULL NULL NULL NULL NULL NULL NULL NULL NULL NULL
drop table t1; 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 End of 5.1 tests
...@@ -1082,6 +1082,20 @@ select a.f1 as a, b.f4 as b, a.f1 > b.f4 as gt, ...@@ -1082,6 +1082,20 @@ select a.f1 as a, b.f4 as b, a.f1 > b.f4 as gt,
from t1 a, t1 b; from t1 a, t1 b;
select *, f1 = f2 from t1; select *, f1 = f2 from t1;
drop table 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 #
--echo End of 5.1 tests --echo End of 5.1 tests
...@@ -417,26 +417,6 @@ void Item_sum::mark_as_sum_func() ...@@ -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) void Item_sum::print(String *str, enum_query_type query_type)
{ {
/* orig_args is not filled with valid values until fix_fields() */ /* orig_args is not filled with valid values until fix_fields() */
......
...@@ -339,7 +339,6 @@ public: ...@@ -339,7 +339,6 @@ public:
forced_const= TRUE; forced_const= TRUE;
} }
virtual bool const_item() const { return forced_const; } virtual bool const_item() const { return forced_const; }
void make_field(Send_field *field);
virtual void print(String *str, enum_query_type query_type); virtual void print(String *str, enum_query_type query_type);
void fix_num_length_and_dec(); void fix_num_length_and_dec();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment