diff --git a/mysql-test/main/update.result b/mysql-test/main/update.result index da1964d70c46f..5f91415f32ef1 100644 --- a/mysql-test/main/update.result +++ b/mysql-test/main/update.result @@ -765,3 +765,83 @@ ccc yyy u xxb drop table t1; # End of MariaDB 10.4 tests +# +# MDEV-35955 Wrong result for UPDATE ... ORDER BY LIMIT which uses tmp.table +# +create table t1 (id int primary key, v int); +create table t2 (id int primary key, v int); +insert into t1 (id, v) values (2,3),(1,4); +insert into t2 (id, v) values (5,5),(6,6); +select t1.*, t2.* from t1, t2 order by t1.id, t2.id limit 2; +id v id v +1 4 5 5 +1 4 6 6 +UPDATE t1, t2 SET t1.v=-1, t2.v=-1 ORDER BY t1.id, t2.id LIMIT 2; +select * from t1; +id v +2 3 +1 -1 +select * from t2; +id v +5 -1 +6 -1 +drop table t1, t2; +create table t1 (id int primary key, v text) engine=myisam; +create table t2 (id int primary key, v text) engine=myisam; +insert into t1 (id, v) values (1,'b'),(2,'fo'),(3,'bar'),(4,'barr'),(5,'bazzz'); +insert into t2 (id, v) values (6,'quxqux'),(7,'foofoof'),(8,'barbarba'),(9,'quxquxqux'),(10,'bazbazbazb'); +select t1.*, t2.* from t1, t2 order by t1.id, t2.id limit 2; +id v id v +1 b 6 quxqux +1 b 7 foofoof +update t1, t2 set t1.v='DELETED', t2.v='DELETED' order by t1.id, t2.id limit 2; +select * from t1; +id v +1 DELETED +2 fo +3 bar +4 barr +5 bazzz +select * from t2; +id v +6 DELETED +7 DELETED +8 barbarba +9 quxquxqux +10 bazbazbazb +drop table t1, t2; +create table t1 (id int primary key, v int); +create table t2 (id int primary key, v int); +create table t3 (id int primary key, v int); +insert into t1 (id, v) values (1, 1000), (2, 2000), (3, 3000), (4, 4000), (5, 5000); +insert into t2 (id, v) values (10, 100), (20, 200), (30, 300), (40, 400), (50, 500); +insert into t3 (id, v) values (11, 111), (22, 222), (33, 333), (44, 444), (55, 555); +select t1.*, t2.*, t3.* from t1, t2, t3 order by t1.id, t2.id, t3.id limit 3; +id v id v id v +1 1000 10 100 11 111 +1 1000 10 100 22 222 +1 1000 10 100 33 333 +UPDATE t1, t2, t3 SET t1.v=-1, t2.v=-2, t3.v=-3 ORDER BY t1.id, t2.id, t3.id LIMIT 3; +select * from t1; +id v +1 -1 +2 2000 +3 3000 +4 4000 +5 5000 +select * from t2; +id v +10 -2 +20 200 +30 300 +40 400 +50 500 +select * from t3; +id v +11 -3 +22 -3 +33 -3 +44 444 +55 555 +drop table t1, t2, t3; +# End of MariaDB 10.11 tests diff --git a/mysql-test/main/update.test b/mysql-test/main/update.test index 9b329cce3c9f6..fe938dd35f0ef 100644 --- a/mysql-test/main/update.test +++ b/mysql-test/main/update.test @@ -707,3 +707,43 @@ select * from t1; drop table t1; --echo # End of MariaDB 10.4 tests + +--echo # +--echo # MDEV-35955 Wrong result for UPDATE ... ORDER BY LIMIT which uses tmp.table +--echo # + +create table t1 (id int primary key, v int); +create table t2 (id int primary key, v int); +insert into t1 (id, v) values (2,3),(1,4); +insert into t2 (id, v) values (5,5),(6,6); +select t1.*, t2.* from t1, t2 order by t1.id, t2.id limit 2; +UPDATE t1, t2 SET t1.v=-1, t2.v=-1 ORDER BY t1.id, t2.id LIMIT 2; +select * from t1; +select * from t2; + +drop table t1, t2; +create table t1 (id int primary key, v text) engine=myisam; +create table t2 (id int primary key, v text) engine=myisam; +insert into t1 (id, v) values (1,'b'),(2,'fo'),(3,'bar'),(4,'barr'),(5,'bazzz'); +insert into t2 (id, v) values (6,'quxqux'),(7,'foofoof'),(8,'barbarba'),(9,'quxquxqux'),(10,'bazbazbazb'); +select t1.*, t2.* from t1, t2 order by t1.id, t2.id limit 2; +update t1, t2 set t1.v='DELETED', t2.v='DELETED' order by t1.id, t2.id limit 2; +select * from t1; +select * from t2; + +drop table t1, t2; +create table t1 (id int primary key, v int); +create table t2 (id int primary key, v int); +create table t3 (id int primary key, v int); +insert into t1 (id, v) values (1, 1000), (2, 2000), (3, 3000), (4, 4000), (5, 5000); +insert into t2 (id, v) values (10, 100), (20, 200), (30, 300), (40, 400), (50, 500); +insert into t3 (id, v) values (11, 111), (22, 222), (33, 333), (44, 444), (55, 555); +select t1.*, t2.*, t3.* from t1, t2, t3 order by t1.id, t2.id, t3.id limit 3; +UPDATE t1, t2, t3 SET t1.v=-1, t2.v=-2, t3.v=-3 ORDER BY t1.id, t2.id, t3.id LIMIT 3; +select * from t1; +select * from t2; +select * from t3; + +drop table t1, t2, t3; + +--echo # End of MariaDB 10.11 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index b88e8b4c5d571..c6b9b5f8ccb84 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -70,6 +70,8 @@ #include "derived_handler.h" #include "create_tmp_table.h" +#include + /* A key part number that means we're using a fulltext scan. @@ -282,10 +284,13 @@ static bool change_to_use_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, List &new_list2, uint elements, List &items); // Create list for using with tempory table -static bool change_refs_to_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, - List &new_list1, - List &new_list2, - uint elements, List &items); +class OrderByColumnFixer; +static bool change_refs_to_tmp_fields(THD *thd, + Ref_ptr_array ref_pointer_array, + List &new_list1, + List &new_list2, uint elements, + List &items, + OrderByColumnFixer &obcf); static void init_tmptable_sum_functions(Item_sum **func); static void update_tmptable_sum_func(Item_sum **func,TABLE *tmp_table); static void copy_sum_funcs(Item_sum **func_ptr, Item_sum **end); @@ -3588,6 +3593,101 @@ bool JOIN::add_fields_for_current_rowid(JOIN_TAB *cur, List *table_fields) } +/* + ORDER BY structures have an item member which points somewhere into the + JOIN::ref_ptrs array. In fact, it is a pointer to a pointer to an item + and the entries in the JOIN::ref_ptrs structure can shift around during + make_aggr_tables_info. This will happen when a temporary table is needed + and existing item references must point into the new temporary table. In + that case, the entries in the ref_ptrs array are replaced with new Item + instances which point to columns from the temporary table, but are done so + based on the boundary (or border) between the two lists that were glued + together to make all_fields (see the setup_fields function). Any object + holding a pointer to somewhere in the ref_ptrs array will now be pointing + at an item referencing the temporary table. This is the desired effect, + but when multiple ORDER BY fields are specified in the query and the ref_ptrs + updated, the ORDER BY structures may not be pointing at the correct new + temporary item field because of the way that ref_ptrs is built during + change_refs_to_tmp_fields. + + This class works by remembering what the old ORDER BY item pointer values + were before the new temporary table was created. When new temporary field + items are created from existing items, this class remembers them as a mapping + from existing items to the new temporary items. Then, after the new + temporary table is created and the new ref_ptrs array installed, this class + fixes the ORDER BY item fields to point to the correct new temporary item + fields created for the new temporary table. +*/ +class OrderByColumnFixer +{ + // Unordered maps are constant time for insertion and lookups on average. + using ORDER_BY_to_OrigItem= std::unordered_map; + ORDER_BY_to_OrigItem order_to_orig; + + using OrigItem_to_TmpTableItem= std::unordered_map; + OrigItem_to_TmpTableItem orig_to_tmp; + + using Item_to_RefAddr= std::unordered_map; + Item_to_RefAddr items_to_ref_addrs; + +public: + OrderByColumnFixer(ORDER *order_by_list) + { + /* Remember the current ORDER BY item pointer values as a mapping + from ORDER BY to the item pointer. Later, we'll use the item + pointer to look up the new temporary item created to replace it. + */ + for (ORDER *next= order_by_list; next; next= next->next) + order_to_orig.insert(std::make_pair(next, *next->item)); + } + ~OrderByColumnFixer() = default; + + void map_orig_to_tmp(Item *orig, Item *tmp) + { + /* Remember which new temporary item field pointers correspond to + their original counterparts. We'll use this to find the new + temporary items and their respective positions in JOIN::ref_ptrs. + */ + if (orig && tmp) + orig_to_tmp.insert(std::make_pair(orig, tmp)); + } + + void repair_order_by_columns(Ref_ptr_array refs) + { + // This method runs in linear time on average. + /* refs[i] are (potentially) new temporary item fields for the new + temp table and &refs[i] are the addresses to those pointers which + we need to update ORDER BY's item pointer-to-a-pointer. + */ + for (uint i= 0; i < refs.size(); ++i) + items_to_ref_addrs.insert(std::make_pair(refs[i], &refs[i])); + + /* For each ORDER BY, look at its old prior item that we remembered + when the constructor was called, and look up the new temporary table + item for it. Then, using that, look up the new ref_ptr offset and + fix up the ORDER BY field accordingly. + */ + for (ORDER_BY_to_OrigItem::iterator iter= order_to_orig.begin(); + iter != order_to_orig.end(); + ++iter) + { + ORDER *order_by= iter->first; + Item *orig_item= iter->second; + + OrigItem_to_TmpTableItem::iterator ott= orig_to_tmp.find(orig_item); + if (ott == orig_to_tmp.end()) + continue; + Item_to_RefAddr::iterator itra= items_to_ref_addrs.find(ott->second); + if (itra == items_to_ref_addrs.end()) + continue; + + // Make ORDER BY point to the correct entry in the ref_ptrs array. + order_by->item= itra->second; + } + } +}; + + /** Set info for aggregation tables @@ -3825,6 +3925,7 @@ bool JOIN::make_aggr_tables_info() optimize_distinct(); /* Change sum_fields reference to calculated fields in tmp_table */ + OrderByColumnFixer obcf(order); items1= ref_ptr_array_slice(2); if ((sort_and_group || curr_tab->table->group || tmp_table_param.precomputed_group_by) && @@ -3837,15 +3938,16 @@ bool JOIN::make_aggr_tables_info() } else { - if (change_refs_to_tmp_fields(thd, items1, - tmp_fields_list1, tmp_all_fields1, - fields_list.elements, all_fields)) + if (change_refs_to_tmp_fields(thd, items1, tmp_fields_list1, + tmp_all_fields1, fields_list.elements, + all_fields, obcf)) DBUG_RETURN(true); } curr_all_fields= &tmp_all_fields1; curr_fields_list= &tmp_fields_list1; // Need to set them now for correct group_fields setup, reset at the end. set_items_ref_array(items1); + obcf.repair_order_by_columns(ref_ptrs); curr_tab->ref_array= &items1; curr_tab->all_fields= &tmp_all_fields1; curr_tab->fields= &tmp_fields_list1; @@ -28225,6 +28327,8 @@ change_to_use_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, @param res_all_fields new list of all items @param elements number of elements in select item list @param all_fields all fields list + @param obcf object that maps original item fields to their + temporary table item field counterparts @retval 0 ok @@ -28232,11 +28336,12 @@ change_to_use_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, 1 error */ -static bool -change_refs_to_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, - List &res_selected_fields, - List &res_all_fields, uint elements, - List &all_fields) +static bool change_refs_to_tmp_fields(THD *thd, + Ref_ptr_array ref_pointer_array, + List &res_selected_fields, + List &res_all_fields, + uint elements, List &all_fields, + OrderByColumnFixer &obcf) { List_iterator_fast it(all_fields); Item *item, *new_item; @@ -28254,6 +28359,7 @@ change_refs_to_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, return 1; } + obcf.map_orig_to_tmp(item, new_item); if (res_all_fields.push_back(new_item, thd->mem_root)) return 1; ref_pointer_array[((i < border)? all_fields.elements-i-1 : i-border)]=