revision-id: 470b99d4cbc93daad8fed568bea6b6eed1920d58 (mariadb-10.3.6-84-g470b99d4cbc) parent(s): 522cd3c7aa9b466d5e0a4d010d4acb13c76a014c author: Varun Gupta committer: Varun Gupta timestamp: 2018-08-10 14:48:51 +0530 message: MDEV-16722: Assertion `type() != NULL_ITEM' failed We hit this assert during the create of a temporary table field because the current code does not handle the case when the value of the NAME_CONST function is NULL. Fixed this by allowing creation of temporary table fields even for the case when NAME_CONST returns NULL value. Introduced tmp_table_field_from_field_type_maybe_null() function in Item class so both Item_basic_value and Item_name_const can use it. Introduced a virtual method get_func_item() in the Item class. --- mysql-test/main/win.result | 11 +++++++++++ mysql-test/main/win.test | 9 +++++++++ sql/item.cc | 22 ++++++---------------- sql/item.h | 24 +++++++++++++++++++++--- sql/item_cmpfunc.h | 14 ++++---------- sql/item_func.h | 1 + sql/sql_select.cc | 41 ++++++++++++++++------------------------- 7 files changed, 68 insertions(+), 54 deletions(-) diff --git a/mysql-test/main/win.result b/mysql-test/main/win.result index 7607cebc3a5..fd0fbefdcc5 100644 --- a/mysql-test/main/win.result +++ b/mysql-test/main/win.result @@ -3334,3 +3334,14 @@ d x 00:00:01 00:00:02 00:00:02 NULL DROP TABLE t1; +# +# MDEV-16722: Assertion `type() != NULL_ITEM' failed +# +create table t1 (a int); +insert into t1 values (1),(2),(3); +SELECT row_number() OVER (order by a) FROM t1 order by NAME_CONST('myname',NULL); +row_number() OVER (order by a) +1 +2 +3 +drop table t1; diff --git a/mysql-test/main/win.test b/mysql-test/main/win.test index 4b73f70d737..cc16595fcd4 100644 --- a/mysql-test/main/win.test +++ b/mysql-test/main/win.test @@ -2100,3 +2100,12 @@ CREATE TABLE t1 (d time); INSERT INTO t1 VALUES ('00:00:01'),('00:00:02'); SELECT *, LEAD(d) OVER (ORDER BY d) AS x FROM t1; DROP TABLE t1; + +--echo # +--echo # MDEV-16722: Assertion `type() != NULL_ITEM' failed +--echo # + +create table t1 (a int); +insert into t1 values (1),(2),(3); +SELECT row_number() OVER (order by a) FROM t1 order by NAME_CONST('myname',NULL); +drop table t1; diff --git a/sql/item.cc b/sql/item.cc index 8b962d1706d..d9c66d3cfc5 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -2004,7 +2004,6 @@ Item_name_const::Item_name_const(THD *thd, Item *name_arg, Item *val): Item_fixed_hybrid(thd), value_item(val), name_item(name_arg) { Item::maybe_null= TRUE; - valid_args= true; if (!name_item->basic_const_item()) goto err; @@ -2023,7 +2022,6 @@ Item_name_const::Item_name_const(THD *thd, Item *name_arg, Item *val): } err: - valid_args= false; my_error(ER_WRONG_ARGUMENTS, MYF(0), "NAME_CONST"); } @@ -2031,24 +2029,16 @@ Item_name_const::Item_name_const(THD *thd, Item *name_arg, Item *val): Item::Type Item_name_const::type() const { /* - As - 1. one can try to create the Item_name_const passing non-constant - arguments, although it's incorrect and - 2. the type() method can be called before the fix_fields() to get - type information for a further type cast, e.g. - if (item->type() == FIELD_ITEM) - ((Item_field *) item)->... - we return NULL_ITEM in the case to avoid wrong casting. - - valid_args guarantees value_item->basic_const_item(); if type is - FUNC_ITEM, then we have a fudged item_func_neg() on our hands - and return the underlying type. + + We are guarenteed that value_item->basic_const_item(), if not + an error is thrown that WRONG ARGUMENTS are supplied to + NAME_CONST function. + If type is FUNC_ITEM, then we have a fudged item_func_neg() + on our hands and return the underlying type. For Item_func_set_collation() e.g. NAME_CONST('name', 'value' COLLATE collation) we return its 'value' argument type. */ - if (!valid_args) - return NULL_ITEM; Item::Type value_type= value_item->type(); if (value_type == FUNC_ITEM) { diff --git a/sql/item.h b/sql/item.h index a96406fa0cd..d6ad088e60a 100644 --- a/sql/item.h +++ b/sql/item.h @@ -820,6 +820,10 @@ class Item: public Value_source, return tmp_table_field_from_field_type(table); } Field *create_tmp_field_int(TABLE *table, uint convert_int_length); + Field *tmp_table_field_from_field_type_maybe_null(TABLE *table, + Tmp_field_src *src, + const Tmp_field_param *param, + bool is_explicit_null); void push_note_converted_to_negative_complement(THD *thd); void push_note_converted_to_positive_complement(THD *thd); @@ -876,6 +880,7 @@ class Item: public Value_source, expressions with subqueries in the ORDER/GROUP clauses. */ String *val_str() { return val_str(&str_value); } + virtual Item_func *get_item_func() { return NULL; } const MY_LOCALE *locale_from_val_str(); @@ -2608,7 +2613,20 @@ class Item_basic_value :public Item, Item_basic_value(THD *thd): Item(thd) {} public: Field *create_tmp_field_ex(TABLE *table, Tmp_field_src *src, - const Tmp_field_param *param); + const Tmp_field_param *param) + { + + /* + create_tmp_field_ex() for this type of Items is called for: + - CREATE TABLE ... SELECT + - In ORDER BY: SELECT max(a) FROM t1 GROUP BY a ORDER BY 'const'; + - In CURSORS: + DECLARE c CURSOR FOR SELECT 'test'; + OPEN c; + */ + return tmp_table_field_from_field_type_maybe_null(table, src, param, + type() == Item::NULL_ITEM); + } bool eq(const Item *item, bool binary_cmp) const; const Type_all_attributes *get_type_all_attributes_from_const() const { return this; } @@ -2949,7 +2967,6 @@ class Item_name_const : public Item_fixed_hybrid { Item *value_item; Item *name_item; - bool valid_args; public: Item_name_const(THD *thd, Item *name_arg, Item *val); @@ -2982,7 +2999,8 @@ class Item_name_const : public Item_fixed_hybrid DECLARE c CURSOR FOR SELECT NAME_CONST('x','y') FROM t1; OPEN c; */ - return create_tmp_field_ex_simple(table, src, param); + return tmp_table_field_from_field_type_maybe_null(table, src, param, + type() == Item::NULL_ITEM); } int save_in_field(Field *field, bool no_conversions) { diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 3336885c036..2d917389e32 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3313,11 +3313,8 @@ class Item_cond_and :public Item_cond inline bool is_cond_and(Item *item) { - if (item->type() != Item::COND_ITEM) - return FALSE; - - Item_cond *cond_item= (Item_cond*) item; - return (cond_item->functype() == Item_func::COND_AND_FUNC); + Item_func *func_item= item->get_item_func(); + return func_item && func_item->type() == Item::COND_ITEM && func_item->functype() == Item_func::COND_AND_FUNC; } class Item_cond_or :public Item_cond @@ -3418,11 +3415,8 @@ class Item_func_cursor_notfound: public Item_func_cursor_bool_attr inline bool is_cond_or(Item *item) { - if (item->type() != Item::COND_ITEM) - return FALSE; - - Item_cond *cond_item= (Item_cond*) item; - return (cond_item->functype() == Item_func::COND_OR_FUNC); + Item_func *func_item= item->get_item_func(); + return func_item && func_item->type() == Item::COND_ITEM && func_item->functype() == Item_func::COND_OR_FUNC; } Item *and_expressions(Item *a, Item *b, Item **org_item); diff --git a/sql/item_func.h b/sql/item_func.h index f44986a9111..48b2ad2afeb 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -393,6 +393,7 @@ class Item_func :public Item_func_or_sum, bool with_sum_func() const { return m_with_sum_func; } With_sum_func_cache* get_with_sum_func_cache() { return this; } + Item_func *get_item_func() { return this; } }; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 80ca1d7da62..bfd1c7580fc 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -16616,6 +16616,22 @@ Field *Item::create_tmp_field_int(TABLE *table, uint convert_int_length) *this, table); } +Field *Item::tmp_table_field_from_field_type_maybe_null(TABLE *table, + Tmp_field_src *src, + const Tmp_field_param *param, + bool is_explicit_null) +{ + DBUG_ASSERT(!param->make_copy_field()); + DBUG_ASSERT(!is_result_field()); + Field *result; + if ((result= tmp_table_field_from_field_type(table))) + { + if (result && is_explicit_null) + result->is_created_from_null_item= true; + } + return result; +} + Field *Item_sum::create_tmp_field(bool group, TABLE *table) { @@ -16847,31 +16863,6 @@ Field *Item_func_sp::create_tmp_field_ex(TABLE *table, return result; } - -Field *Item_basic_value::create_tmp_field_ex(TABLE *table, - Tmp_field_src *src, - const Tmp_field_param *param) -{ - /* - create_tmp_field_ex() for this type of Items is called for: - - CREATE TABLE ... SELECT - - In ORDER BY: SELECT max(a) FROM t1 GROUP BY a ORDER BY 'const'; - - In CURSORS: - DECLARE c CURSOR FOR SELECT 'test'; - OPEN c; - */ - DBUG_ASSERT(!param->make_copy_field()); - DBUG_ASSERT(!is_result_field()); - Field *result; - if ((result= tmp_table_field_from_field_type(table))) - { - if (type() == Item::NULL_ITEM) // Item_null or Item_param - result->is_created_from_null_item= true; - } - return result; -} - - /** Create field for temporary table.