Browse Source

MDEV-35955 Wrong result for UPDATE ... ORDER BY LIMIT using tmp.table

Queries of the form
  UPDATE t1.*, t2.* ORDER BY t1.col1, t2.col2, ... LIMIT N;
now correctly sort first by t1.col, then by t2.col2.  Previously,
such queries would, when more than one ORDER BY column was specified,
incorrectly sort the columns from the input tables; as an example, when
two ORDER BY columns were specified, the sorting would first be by t2.col2
and then by t1.col1, updating more rows from the first table than the
second.
10.11-mdev-35955-update-wrong-result
Dave Gosselin 8 months ago
parent
commit
9109c16efd
  1. 80
      mysql-test/main/update.result
  2. 40
      mysql-test/main/update.test
  3. 130
      sql/sql_select.cc

80
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

40
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

130
sql/sql_select.cc

@ -70,6 +70,8 @@
#include "derived_handler.h"
#include "create_tmp_table.h"
#include <unordered_map>
/*
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<Item> &new_list2,
uint elements, List<Item> &items);
// Create list for using with tempory table
static bool change_refs_to_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array,
List<Item> &new_list1,
List<Item> &new_list2,
uint elements, List<Item> &items);
class OrderByColumnFixer;
static bool change_refs_to_tmp_fields(THD *thd,
Ref_ptr_array ref_pointer_array,
List<Item> &new_list1,
List<Item> &new_list2, uint elements,
List<Item> &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<Item> *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 *, Item *>;
ORDER_BY_to_OrigItem order_to_orig;
using OrigItem_to_TmpTableItem= std::unordered_map<Item*, Item*>;
OrigItem_to_TmpTableItem orig_to_tmp;
using Item_to_RefAddr= std::unordered_map<Item*, Item **>;
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<Item> &res_selected_fields,
List<Item> &res_all_fields, uint elements,
List<Item> &all_fields)
static bool change_refs_to_tmp_fields(THD *thd,
Ref_ptr_array ref_pointer_array,
List<Item> &res_selected_fields,
List<Item> &res_all_fields,
uint elements, List<Item> &all_fields,
OrderByColumnFixer &obcf)
{
List_iterator_fast<Item> 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)]=

Loading…
Cancel
Save