From 812cba1dc45f737a91afc96855c8371eac107089 Mon Sep 17 00:00:00 2001 From: Rucha Deodhar Date: Mon, 6 Jul 2026 16:12:37 -0400 Subject: [PATCH] MDEV-40125: OLD_VALUE crashes on a view with an expression Analysis: When OLD_VALUE() is used on a view, field resolution creates an Item_direct_view_ref instead of an Item_ref where real_item() points to the expression. But Item_old_field::fix_fields() continues assuming that a Field object was available, but view references do not have a field pointer, which resulted in a crash. Fix: Store the real referenced item in Item_old_field::expr and use it when the OLD_VALUE() reference does not have a Field object. This allows OLD_VALUE() to work with view fields and avoids dereferencing a NULL field pointer. Fix relevant methods accordingly. --- mysql-test/main/update.result | 27 ++++++++++++ mysql-test/main/update.test | 21 ++++++++++ sql/item.cc | 77 ++++++++++++++++------------------- sql/item.h | 43 ++++++++++++------- sql/sql_base.cc | 32 ++++++++++----- sql/table.cc | 1 - 6 files changed, 134 insertions(+), 67 deletions(-) diff --git a/mysql-test/main/update.result b/mysql-test/main/update.result index c8ef0707ea2fa..80f6edbc66eff 100644 --- a/mysql-test/main/update.result +++ b/mysql-test/main/update.result @@ -1148,4 +1148,31 @@ UPDATE t1 SET a=NULL RETURNING OLD_VALUE(a) AS old, a AS new; old new NULL DROP TABLE t1; +# +# MDEV-40125: OLD_VALUE crashes on a view with an expression +# +CREATE TABLE t ( id INT PRIMARY KEY, a INT ); +INSERT INTO t VALUES (1, 1), (3, 3), (5, 5); +CREATE VIEW v AS SELECT id, a, a*2 AS a2 FROM t; +SELECT * FROM v; +id a a2 +1 1 2 +3 3 6 +5 5 10 +UPDATE v SET a = a+1 WHERE id = 1 RETURNING OLD_VALUE(id) as old_id, id, +OLD_VALUE(a) as old_a, a, +OLD_VALUE(a2)as old_a2, a2; +old_id id old_a a old_a2 a2 +1 1 1 2 2 4 +SELECT * FROM v; +id a a2 +1 2 4 +3 3 6 +5 5 10 +UPDATE v SET a = a+1 WHERE id = 20 RETURNING OLD_VALUE(id) as old_id, id, +OLD_VALUE(a) as old_a, a, +OLD_VALUE(a2)as old_a2, a2; +old_id id old_a a old_a2 a2 +DROP TABLE t; +DROP VIEW v; # End of 13.0 test diff --git a/mysql-test/main/update.test b/mysql-test/main/update.test index 0a386449f586d..ed68193223a7c 100644 --- a/mysql-test/main/update.test +++ b/mysql-test/main/update.test @@ -1005,4 +1005,25 @@ INSERT INTO t1 VALUES('') RETURNING *; UPDATE t1 SET a=NULL RETURNING OLD_VALUE(a) AS old, a AS new; DROP TABLE t1; +--echo # +--echo # MDEV-40125: OLD_VALUE crashes on a view with an expression +--echo # + +CREATE TABLE t ( id INT PRIMARY KEY, a INT ); +INSERT INTO t VALUES (1, 1), (3, 3), (5, 5); +CREATE VIEW v AS SELECT id, a, a*2 AS a2 FROM t; + +SELECT * FROM v; +UPDATE v SET a = a+1 WHERE id = 1 RETURNING OLD_VALUE(id) as old_id, id, + OLD_VALUE(a) as old_a, a, + OLD_VALUE(a2)as old_a2, a2; +SELECT * FROM v; + +UPDATE v SET a = a+1 WHERE id = 20 RETURNING OLD_VALUE(id) as old_id, id, + OLD_VALUE(a) as old_a, a, + OLD_VALUE(a2)as old_a2, a2; + +DROP TABLE t; +DROP VIEW v; + --echo # End of 13.0 test diff --git a/sql/item.cc b/sql/item.cc index 3bdd7f82da327..9e4f766660da9 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3383,6 +3383,19 @@ void Item_field::set_field(Field *field_par) if (field->table->s->tmp_table == SYSTEM_TMP_TABLE || field->table->s->tmp_table == INTERNAL_TMP_TABLE) set_refers_to_temp_table(); + + field->ptr_old= field_par->table->record[1] + + field_par->offset(field->table->record[0]); + if (field_par->null_ptr) + { + field->null_ptr_old= field_par->table->record[1] + + (field_par->null_ptr - + field_par->table->record[0]); + } + else + { + field->null_ptr_old = nullptr; + } } @@ -7230,6 +7243,14 @@ void Item_field::make_send_field(THD *thd, Send_field *tmp_field) tmp_field->db_name= db_name; } +void Item_old_field::make_send_field(THD *thd, Send_field *tmp_field) +{ + if (field) + Item_field::make_send_field(thd, tmp_field); + else + expr->make_send_field(thd, tmp_field); +} + /** Save a field value in another field @@ -8055,55 +8076,27 @@ bool Item_field::send(Protocol *protocol, st_value *buffer) bool Item_old_field::send(Protocol *protocol, st_value *buffer) { - bool result; - - change_field_ptr(); - result= Item_field::send(protocol, buffer); - change_field_ptr(); - - return result; -} - -void Item_old_field::change_field_ptr() -{ - std::swap(field->ptr_old, field->ptr); - std::swap(field->null_ptr, field->null_ptr_old); -} - - -bool Item_old_field::fix_fields(THD *thd, Item **reference) -{ - if (Item_field::fix_fields(thd, reference)) - return true; - /* - Store the pointer to where old values are store before update. - */ + bool result= false; if (field) { - field->ptr_old= field->table->record[1] + - field->offset(field->table->record[0]); - if (field->null_ptr) - { - field->null_ptr_old = - field->table->record[1] + - (field->null_ptr - field->table->record[0]); - } - else - { - field->null_ptr_old = nullptr; - } + (void) change_field_ptr(0); + result= Item_field::send(protocol, buffer); + (void) change_field_ptr(0); } else { - /* - Field is NULL in case of view. But we need the information to field - to return its old value. Hence get the field from the reference item - and proceed. - */ - field= ((Item_old_field*)((*reference)->real_item()))->field; - field->ptr_old= field->table->record[1] + field->offset(field->table->record[0]); + expr->walk(&Item::change_field_ptr, 0, 0); + result= expr->send(protocol, buffer); + expr->walk(&Item::change_field_ptr, 0, 0); } + return result; +} + +bool Item_field::change_field_ptr(void *arg) +{ + std::swap(field->ptr_old, field->ptr); + std::swap(field->null_ptr, field->null_ptr_old); return false; } diff --git a/sql/item.h b/sql/item.h index e0c398daa4225..65261672646af 100644 --- a/sql/item.h +++ b/sql/item.h @@ -988,7 +988,8 @@ class Item :public Value_source, return rc; } public: - bool is_old_value_reference; + bool is_old_value_reference; + virtual Item *get_correct_item() { return this; } /* Cache val_str() into the own buffer, e.g. to evaluate constant expressions with subqueries in the ORDER/GROUP clauses. @@ -2289,6 +2290,7 @@ class Item :public Value_source, // FIXME reduce the number of "add field to bitmap" processors virtual bool add_field_to_set_processor(void *arg) { return 0; } virtual bool register_field_in_read_map(void *arg) { return 0; } + virtual bool change_field_ptr(void *arg) { return 0; } virtual bool register_field_in_write_map(void *arg) { return 0; } virtual bool register_field_in_bitmap(void *arg) { return 0; } virtual bool update_table_bitmaps_processor(void *arg) { return 0; } @@ -3768,6 +3770,7 @@ class Item_ident :public Item_result_field const LEX_CSTRING &db_name_arg, const LEX_CSTRING &table_name_arg, const LEX_CSTRING &field_name_arg); Item_ident(THD *thd, Item_ident *item); + Item_ident(THD *thd):Item_result_field(thd) {} Item_ident(THD *thd, TABLE_LIST *view_arg, const LEX_CSTRING &field_name_arg); LEX_CSTRING full_name_cstring() const override; void cleanup() override; @@ -3796,7 +3799,7 @@ class Item_field :public Item_ident, public Load_data_outvar { protected: - void set_field(Field *field); + virtual void set_field(Field *field); public: Field *field; Item_equal *item_equal; @@ -3831,6 +3834,7 @@ class Item_field :public Item_ident, const LEX_CSTRING &field_name_arg) :Item_field(thd, context_arg, null_clex_str, null_clex_str, field_name_arg) {} + Item_field(THD *thd):Item_ident(thd){field= nullptr;} Item_field(THD *thd, Name_resolution_context *context_arg) :Item_field(thd, context_arg, null_clex_str, null_clex_str, null_clex_str) {} @@ -4054,6 +4058,7 @@ class Item_field :public Item_ident, { return field->max_display_length(); } Item_field *field_for_view_update() override { return this; } int fix_outer_field(THD *thd, Field **field, Item **reference); + bool change_field_ptr(void *arg) override; Item *update_value_transformer(THD *thd, uchar *select_arg) override; Item *derived_field_transformer_for_having(THD *thd, uchar *arg) override; Item *derived_field_transformer_for_where(THD *thd, uchar *arg) override; @@ -4095,34 +4100,44 @@ class Item_old_field: public Item_field Name_resolution_context *context_arg, const LEX_CSTRING &db_arg, const LEX_CSTRING &table_name_arg, - const LEX_CSTRING &field_name_arg) + const LEX_CSTRING &field_name_arg, Item *item=nullptr) : Item_field(thd, context_arg, db_arg, table_name_arg, field_name_arg) - { is_old_value_reference= true; } + { is_old_value_reference= true; expr= item;} Item_old_field(THD *thd, Name_resolution_context *context_arg, - const LEX_CSTRING &field_name_arg) + const LEX_CSTRING &field_name_arg, + Item *item=nullptr) : Item_field(thd, context_arg, field_name_arg) - { is_old_value_reference= true; } + { is_old_value_reference= true; expr= item;} - Item_old_field(THD *thd, Item_field *item) + Item_old_field(THD *thd, Item* item= nullptr):Item_field(thd) + { expr= item; } + + Item_old_field(THD *thd, Item_field *item, Item *item_expr=nullptr) : Item_field(thd, item) - { is_old_value_reference= true; } + { is_old_value_reference= true; expr= item_expr;} Item_old_field(THD *thd, Name_resolution_context *context_arg, - Field *field) + Field *field=nullptr, Item *item=nullptr) : Item_field(thd, context_arg, field) - { is_old_value_reference= true; } + { is_old_value_reference= true; expr= item; } - Item_old_field(THD *thd, Field *field) + Item_old_field(THD *thd, Field *field= nullptr, Item *item=nullptr) : Item_field(thd, field) - { is_old_value_reference= true; } + { is_old_value_reference= true; expr= item; } bool send(Protocol *protocol, st_value *buffer) override; Type type() const override { return FIELD_OLD_ITEM; } - bool fix_fields(THD *, Item **) override; - void change_field_ptr(); + Item *get_correct_item() override { return expr ? expr : this; } + void make_send_field(THD *thd, Send_field *tmp_field) override; + const Type_handler *type_handler() const override + { + const Type_handler *handler= field ? field->type_handler() : expr->type_handler(); + return handler->type_handler_for_item_field(); + } + Item *expr; private: uchar *saved_row_ref; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 54b409381974c..3b279b382862f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -6399,12 +6399,14 @@ find_field_in_view(THD *thd, TABLE_LIST *table_list, (*ref)->is_old_value_reference) { Item *real = item->real_item(); + Item_old_field *old= nullptr; + if (real->type() == Item::FIELD_ITEM) - { - Item_old_field *old = - new (thd->mem_root) Item_old_field(thd, (Item_field*) real); - item = old; - } + old= new (thd->mem_root) Item_old_field(thd, (Item_field*) real); + else + old= new (thd->mem_root) Item_old_field(thd, real); + + item = old; } /* @@ -6825,15 +6827,25 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list, { if (!ref) DBUG_RETURN(fld); + Item *it= (*ref)->real_item(); + if (it->type() == Item::FIELD_ITEM) field_to_set= ((Item_field*)it)->field; else { + char buffer[128]; + size_t len= (*ref)->name.length * sizeof((*ref)->name.str[0]); + strncpy(buffer, (*ref)->name.str, len); + if (it->type() == Item::FIELD_OLD_ITEM && + ((Item_old_field*)(it))->expr) + { + it->get_correct_item()->set_name(thd, (*ref)->name); + } if (thd->column_usage == MARK_COLUMNS_READ) - it->walk(&Item::register_field_in_read_map, 0, 0); + it->get_correct_item()->walk(&Item::register_field_in_read_map, 0, 0); else - it->walk(&Item::register_field_in_write_map, 0, 0); + it->get_correct_item()->walk(&Item::register_field_in_write_map, 0, 0); } } else @@ -8334,10 +8346,10 @@ bool setup_fields(THD *thd, Ref_ptr_array ref_pointer_array, item->split_sum_func(thd, ref_pointer_array, *sum_func_list, SPLIT_SUM_SELECT); } - lex->current_select->select_list_tables|= item->used_tables(); - lex->used_tables|= item->used_tables(); + lex->current_select->select_list_tables|= item->get_correct_item()->used_tables(); + lex->used_tables|= item->get_correct_item()->used_tables(); lex->current_select->cur_pos_in_select_list++; - lex->current_select->rownum_in_field_list |= item->with_rownum_func(); + lex->current_select->rownum_in_field_list |= item->get_correct_item()->with_rownum_func(); } lex->current_select->is_item_list_lookup= save_is_item_list_lookup; lex->current_select->cur_pos_in_select_list= UNDEF_POS; diff --git a/sql/table.cc b/sql/table.cc index fb25f4ddaf1bf..0bf10dd5f9bb9 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -7378,7 +7378,6 @@ const Lex_ident_column Field_iterator_view::name() return ptr->name; } - Item *Field_iterator_view::create_item(THD *thd) { return create_view_field(thd, view, &ptr->item, &ptr->name);