From 4869e7f4a8d1e997936de775536bf3708cf99529 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Mon, 18 May 2020 16:39:38 +0200 Subject: MDEV-22615 system_time_zone may be incorrectly reported as "unknown". TIME_ZONE_ID_UNKNOWN return code from GetDynamicTimeZoneInformation() does not mean failure. It only means, daylight saving dates in the returned strct are not valid. TIME_ZONE_ID_INVALID means failure, in this case "unknown" should be returned --- sql/mysqld.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index b9908d8c5f3..57eee46a21a 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4176,7 +4176,7 @@ static void get_win_tzname(char* buf, size_t size) {0,0} }; DYNAMIC_TIME_ZONE_INFORMATION tzinfo; - if (GetDynamicTimeZoneInformation(&tzinfo) == TIME_ZONE_ID_UNKNOWN) + if (GetDynamicTimeZoneInformation(&tzinfo) == TIME_ZONE_ID_INVALID) { strncpy(buf, "unknown", size); return; -- cgit v1.2.1 From 06fb78c6acf0cf22ab756e5b89652aa6a5e8485e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 18 May 2020 11:29:43 +0400 Subject: MDEV-21995 Server crashes in Item_field::real_type_handler with table value constructor 1. Code simplification: Item_default_value handled all these values: a. DEFAULT(field) b. DEFAULT c. IGNORE and had various conditions to distinguish (a) from (b) and from (c). Introducing a new abstract class Item_contextually_typed_value_specification, to handle (b) and (c), so the hierarchy now looks as follows: Item Item_result_field Item_ident Item_field Item_default_value - DEFAULT(field) Item_contextually_typed_value_specification Item_default_specification - DEFAULT Item_ignore_specification - IGNORE 2. Introducing a new virtual method is_evaluable_expression() to determine if an Item is: - a normal expression, so its val_xxx()/get_date() methods can be called - or a just an expression substitute, whose value methods cannot be called. 3. Disallowing Items that are not evalualble expressions in table value constructors. --- mysql-test/main/table_value_constr.result | 11 +++ mysql-test/main/table_value_constr.test | 14 +++ sql/item.cc | 109 +++++++--------------- sql/item.h | 150 +++++++++++++++++++++++++----- sql/sql_select.cc | 1 + sql/sql_tvc.cc | 3 +- sql/sql_yacc.yy | 4 +- sql/sql_yacc_ora.yy | 4 +- 8 files changed, 190 insertions(+), 106 deletions(-) diff --git a/mysql-test/main/table_value_constr.result b/mysql-test/main/table_value_constr.result index b00cfd28dcc..b95f9289360 100644 --- a/mysql-test/main/table_value_constr.result +++ b/mysql-test/main/table_value_constr.result @@ -2610,3 +2610,14 @@ $$ a 0 1 +# +# MDEV-21995 Server crashes in Item_field::real_type_handler with table value constructor +# +VALUES (IGNORE); +ERROR HY000: 'ignore' is not allowed in this context +VALUES (DEFAULT); +ERROR HY000: 'default' is not allowed in this context +EXECUTE IMMEDIATE 'VALUES (?)' USING IGNORE; +ERROR HY000: 'ignore' is not allowed in this context +EXECUTE IMMEDIATE 'VALUES (?)' USING DEFAULT; +ERROR HY000: 'default' is not allowed in this context diff --git a/mysql-test/main/table_value_constr.test b/mysql-test/main/table_value_constr.test index e7843c604dd..11d553f0b85 100644 --- a/mysql-test/main/table_value_constr.test +++ b/mysql-test/main/table_value_constr.test @@ -1339,3 +1339,17 @@ BEGIN NOT ATOMIC END; $$ DELIMITER ;$$ + + +--echo # +--echo # MDEV-21995 Server crashes in Item_field::real_type_handler with table value constructor +--echo # + +--error ER_UNKNOWN_ERROR +VALUES (IGNORE); +--error ER_UNKNOWN_ERROR +VALUES (DEFAULT); +--error ER_UNKNOWN_ERROR +EXECUTE IMMEDIATE 'VALUES (?)' USING IGNORE; +--error ER_UNKNOWN_ERROR +EXECUTE IMMEDIATE 'VALUES (?)' USING DEFAULT; diff --git a/sql/item.cc b/sql/item.cc index bf0a8e0e754..9a4325c73bc 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -99,6 +99,15 @@ void item_init(void) } +void Item::raise_error_not_evaluable() +{ + Item::Print tmp(this, QT_ORDINARY); + // TODO-10.5: add an error message to errmsg-utf8.txt + my_printf_error(ER_UNKNOWN_ERROR, + "'%s' is not allowed in this context", MYF(0), tmp.ptr()); +} + + void Item::push_note_converted_to_negative_complement(THD *thd) { push_warning(thd, Sql_condition::WARN_LEVEL_NOTE, ER_UNKNOWN_ERROR, @@ -4547,6 +4556,23 @@ int Item_param::save_in_field(Field *field, bool no_conversions) } +bool Item_param::is_evaluable_expression() const +{ + switch (state) { + case SHORT_DATA_VALUE: + case LONG_DATA_VALUE: + case NULL_VALUE: + return true; + case NO_VALUE: + return true; // Not assigned yet, so we don't know + case IGNORE_VALUE: + case DEFAULT_VALUE: + break; + } + return false; +} + + bool Item_param::can_return_value() const { // There's no "default". See comments in Item_param::save_in_field(). @@ -9308,12 +9334,7 @@ bool Item_default_value::fix_fields(THD *thd, Item **items) Item_field *field_arg; Field *def_field; DBUG_ASSERT(fixed == 0); - - if (!arg) - { - fixed= 1; - return FALSE; - } + DBUG_ASSERT(arg); /* DEFAULT() do not need table field so should not ask handler to bring @@ -9389,11 +9410,7 @@ void Item_default_value::cleanup() void Item_default_value::print(String *str, enum_query_type query_type) { - if (!arg) - { - str->append(STRING_WITH_LEN("default")); - return; - } + DBUG_ASSERT(arg); str->append(STRING_WITH_LEN("default(")); /* We take DEFAULT from a field so do not need it value in case of const @@ -9407,6 +9424,7 @@ void Item_default_value::print(String *str, enum_query_type query_type) void Item_default_value::calculate() { + DBUG_ASSERT(arg); if (field->default_value) field->set_default(); DEBUG_SYNC(field->table->in_use, "after_Item_default_value_calculate"); @@ -9450,14 +9468,8 @@ bool Item_default_value::send(Protocol *protocol, st_value *buffer) int Item_default_value::save_in_field(Field *field_arg, bool no_conversions) { - if (arg) - { - calculate(); - return Item_field::save_in_field(field_arg, no_conversions); - } - - return field_arg->save_in_field_default_value(context->error_processor == - &view_error_processor); + calculate(); + return Item_field::save_in_field(field_arg, no_conversions); } table_map Item_default_value::used_tables() const @@ -9478,13 +9490,7 @@ Item *Item_default_value::transform(THD *thd, Item_transformer transformer, uchar *args) { DBUG_ASSERT(!thd->stmt_arena->is_stmt_prepare()); - - /* - If the value of arg is NULL, then this object represents a constant, - so further transformation is unnecessary (and impossible). - */ - if (!arg) - return 0; + DBUG_ASSERT(arg); Item *new_item= arg->transform(thd, transformer, args); if (!new_item) @@ -9501,57 +9507,6 @@ Item *Item_default_value::transform(THD *thd, Item_transformer transformer, return (this->*transformer)(thd, args); } -void Item_ignore_value::print(String *str, enum_query_type query_type) -{ - str->append(STRING_WITH_LEN("ignore")); -} - -int Item_ignore_value::save_in_field(Field *field_arg, bool no_conversions) -{ - return field_arg->save_in_field_ignore_value(context->error_processor == - &view_error_processor); -} - -String *Item_ignore_value::val_str(String *str) -{ - DBUG_ASSERT(0); // never should be called - null_value= 1; - return 0; -} - -double Item_ignore_value::val_real() -{ - DBUG_ASSERT(0); // never should be called - null_value= 1; - return 0.0; -} - -longlong Item_ignore_value::val_int() -{ - DBUG_ASSERT(0); // never should be called - null_value= 1; - return 0; -} - -my_decimal *Item_ignore_value::val_decimal(my_decimal *decimal_value) -{ - DBUG_ASSERT(0); // never should be called - null_value= 1; - return 0; -} - -bool Item_ignore_value::get_date(MYSQL_TIME *ltime, ulonglong fuzzydate) -{ - DBUG_ASSERT(0); // never should be called - null_value= 1; - return TRUE; -} - -bool Item_ignore_value::send(Protocol *protocol, st_value *buffer) -{ - DBUG_ASSERT(0); // never should be called - return TRUE; -} bool Item_insert_value::eq(const Item *item, bool binary_cmp) const { diff --git a/sql/item.h b/sql/item.h index a40f0ab082e..e4f0c5974bd 100644 --- a/sql/item.h +++ b/sql/item.h @@ -655,6 +655,7 @@ public: WINDOW_FUNC_ITEM, STRING_ITEM, INT_ITEM, REAL_ITEM, NULL_ITEM, VARBIN_ITEM, COPY_STR_ITEM, FIELD_AVG_ITEM, DEFAULT_VALUE_ITEM, + CONTEXTUALLY_TYPED_VALUE_ITEM, PROC_ITEM,COND_ITEM, REF_ITEM, FIELD_STD_ITEM, FIELD_VARIANCE_ITEM, INSERT_VALUE_ITEM, SUBSELECT_ITEM, ROW_ITEM, CACHE_ITEM, TYPE_HOLDER, @@ -709,6 +710,7 @@ protected: } Field *create_tmp_field_int(TABLE *table, uint convert_int_length); + void raise_error_not_evaluable(); void push_note_converted_to_negative_complement(THD *thd); void push_note_converted_to_positive_complement(THD *thd); @@ -1349,6 +1351,24 @@ public: INSERT INTO t1 (vcol) VALUES (NULL) -> ok */ virtual bool vcol_assignment_allowed_value() const { return false; } + /* + Determines if the Item is an evaluable expression, that is + it can return a value, so we can call methods val_xxx(), get_date(), etc. + Most items are evaluable expressions. + Examples of non-evaluable expressions: + - Item_contextually_typed_value_specification (handling DEFAULT and IGNORE) + - Item_type_param bound to DEFAULT and IGNORE + We cannot call the mentioned methods for these Items, + their method implementations typically have DBUG_ASSERT(0). + */ + virtual bool is_evaluable_expression() const { return true; } + bool check_is_evaluable_expression_or_error() + { + if (is_evaluable_expression()) + return false; // Ok + raise_error_not_evaluable(); + return true; // Error + } /* cloning of constant items (0 if it is not const) */ virtual Item *clone_item(THD *thd) { return 0; } virtual Item* build_clone(THD *thd) { return get_copy(thd); } @@ -3513,6 +3533,7 @@ class Item_param :public Item_basic_value, const String *value_query_val_str(THD *thd, String* str) const; bool value_eq(const Item *item, bool binary_cmp) const; Item *value_clone_item(THD *thd); + bool is_evaluable_expression() const; bool can_return_value() const; public: @@ -5797,20 +5818,11 @@ class Item_default_value : public Item_field public: Item *arg; Field *cached_field; - Item_default_value(THD *thd, Name_resolution_context *context_arg) - :Item_field(thd, context_arg, (const char *)NULL, (const char *)NULL, - &null_clex_str), - arg(NULL), cached_field(NULL) {} Item_default_value(THD *thd, Name_resolution_context *context_arg, Item *a) :Item_field(thd, context_arg, (const char *)NULL, (const char *)NULL, &null_clex_str), arg(a), cached_field(NULL) {} - Item_default_value(THD *thd, Name_resolution_context *context_arg, Field *a) - :Item_field(thd, context_arg, (const char *)NULL, (const char *)NULL, - &null_clex_str), - arg(NULL), cached_field(NULL) {} enum Type type() const { return DEFAULT_VALUE_ITEM; } - bool vcol_assignment_allowed_value() const { return arg == NULL; } bool eq(const Item *item, bool binary_cmp) const; bool fix_fields(THD *, Item **); void cleanup(); @@ -5825,7 +5837,7 @@ public: bool save_in_param(THD *thd, Item_param *param) { // It should not be possible to have "EXECUTE .. USING DEFAULT(a)" - DBUG_ASSERT(arg == NULL); + DBUG_ASSERT(0); param->set_default(); return false; } @@ -5850,34 +5862,124 @@ public: Item *transform(THD *thd, Item_transformer transformer, uchar *args); }; + +class Item_contextually_typed_value_specification: public Item +{ +public: + Item_contextually_typed_value_specification(THD *thd) :Item(thd) + { } + enum Type type() const { return CONTEXTUALLY_TYPED_VALUE_ITEM; } + bool vcol_assignment_allowed_value() const { return true; } + bool eq(const Item *item, bool binary_cmp) const + { + return false; + } + bool is_evaluable_expression() const { return false; } + bool fix_fields(THD *thd, Item **items) + { + fixed= true; + return false; + } + String *val_str(String *str) + { + DBUG_ASSERT(0); // never should be called + null_value= true; + return 0; + } + double val_real() + { + DBUG_ASSERT(0); // never should be called + null_value= true; + return 0.0; + } + longlong val_int() + { + DBUG_ASSERT(0); // never should be called + null_value= true; + return 0; + } + my_decimal *val_decimal(my_decimal *decimal_value) + { + DBUG_ASSERT(0); // never should be called + null_value= true; + return 0; + } + bool get_date(MYSQL_TIME *ltime,ulonglong fuzzydate) + { + DBUG_ASSERT(0); // never should be called + return null_value= true; + } + bool send(Protocol *protocol, st_value *buffer) + { + DBUG_ASSERT(0); + return true; + } + const Type_handler *type_handler() const + { + DBUG_ASSERT(0); + return &type_handler_null; + } +}; + + +/* + ::= DEFAULT +*/ +class Item_default_specification: + public Item_contextually_typed_value_specification +{ +public: + Item_default_specification(THD *thd) + :Item_contextually_typed_value_specification(thd) + { } + void print(String *str, enum_query_type query_type) + { + str->append(STRING_WITH_LEN("default")); + } + int save_in_field(Field *field_arg, bool no_conversions) + { + return field_arg->save_in_field_default_value(false); + } + bool save_in_param(THD *thd, Item_param *param) + { + param->set_default(); + return false; + } + Item *get_copy(THD *thd) + { return get_item_copy(thd, this); } +}; + + /** This class is used as bulk parameter INGNORE representation. It just do nothing when assigned to a field + This is a non-standard MariaDB extension. */ -class Item_ignore_value : public Item_default_value +class Item_ignore_specification: + public Item_contextually_typed_value_specification { public: - Item_ignore_value(THD *thd, Name_resolution_context *context_arg) - :Item_default_value(thd, context_arg) - {}; - - void print(String *str, enum_query_type query_type); - int save_in_field(Field *field_arg, bool no_conversions); + Item_ignore_specification(THD *thd) + :Item_contextually_typed_value_specification(thd) + { } + void print(String *str, enum_query_type query_type) + { + str->append(STRING_WITH_LEN("ignore")); + } + int save_in_field(Field *field_arg, bool no_conversions) + { + return field_arg->save_in_field_ignore_value(false); + } bool save_in_param(THD *thd, Item_param *param) { param->set_ignore(); return false; } - - String *val_str(String *str); - double val_real(); - longlong val_int(); - my_decimal *val_decimal(my_decimal *decimal_value); - bool get_date(MYSQL_TIME *ltime,ulonglong fuzzydate); - bool send(Protocol *protocol, st_value *buffer); + Item *get_copy(THD *thd) + { return get_item_copy(thd, this); } }; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index dd12ef0aa17..567e3047a1d 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -17202,6 +17202,7 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type, } case Item::FIELD_ITEM: case Item::DEFAULT_VALUE_ITEM: + case Item::CONTEXTUALLY_TYPED_VALUE_ITEM: case Item::INSERT_VALUE_ITEM: case Item::TRIGGER_FIELD_ITEM: { diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index 92cd229eebb..def519035d9 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -59,7 +59,8 @@ bool fix_fields_for_tvc(THD *thd, List_iterator_fast &li) while replacing their values to NAME_CONST()s. So fix only those that have not been. */ - if (item->fix_fields_if_needed(thd, 0)) + if (item->fix_fields_if_needed(thd, 0) || + item->check_is_evaluable_expression_or_error()) DBUG_RETURN(true); } } diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 14d71139dad..edcd2171721 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -13673,13 +13673,13 @@ expr_or_default: expr { $$= $1;} | DEFAULT { - $$= new (thd->mem_root) Item_default_value(thd, Lex->current_context()); + $$= new (thd->mem_root) Item_default_specification(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; } | IGNORE_SYM { - $$= new (thd->mem_root) Item_ignore_value(thd, Lex->current_context()); + $$= new (thd->mem_root) Item_ignore_specification(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; } diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index d174e02fc24..60ff19e06b2 100644 --- a/sql/sql_yacc_ora.yy +++ b/sql/sql_yacc_ora.yy @@ -13634,13 +13634,13 @@ expr_or_default: expr { $$= $1;} | DEFAULT { - $$= new (thd->mem_root) Item_default_value(thd, Lex->current_context()); + $$= new (thd->mem_root) Item_default_specification(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; } | IGNORE_SYM { - $$= new (thd->mem_root) Item_ignore_value(thd, Lex->current_context()); + $$= new (thd->mem_root) Item_ignore_specification(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; } -- cgit v1.2.1