Bug#22343301: Error 1093 (HY000): You can't specify target table'.' for update

roylyseng · roylyseng · commit 3694eb1cfa0c · 2016-01-06T11:57:14.000+01:00
In the problem query, a derived table is contained in the SET clause of
the UPDATE statement. According to documentation, such derived tables
should be materialized, in order to avoid error about updating a table
that is also read in the same query.

These two mechanisms are used to ensure proper handling:

1. When resolving derived tables, exclude_from_table_unique_test is set
   so that materialized derived tables are excluded from later
   uniqueness test.
2. unique_table() later performs the uniqueness test.

After refactoring of derived tables in 5.7, exclude_from_table_unique_test
is set later, when setup_fields() is called for update_value_list.
But this is too late for unique_table() to notice it.

The solution is to delay the call to unique_table() to after the
derived tables are properly resolved. For single-table UPDATE, this
means after setup_fields() has been called for update_value_list.
For multi-table UPDATE, it means moving the test to
Query_result_update::prepare(), just after setup_fields() has been
called for *values.
Notice also that since Query_result_update::prepare() is not called
during query preparation, the uniqueness test has to be done twice.

Some light refactoring has been made to Query_result_update::prepare(),
to enhance locality of scope of local variables.
diff --git a/mysql-test/r/derived.result b/mysql-test/r/derived.result
@@ -3827,3 +3827,50 @@ NULL	NULL
 1	1
 1	5
 DROP TABLE t1, t2, t3;
+# Bug#22343301: Error 1093 (HY000): You can't specify target table'.'
+#               for update in FROM clause
+CREATE TABLE users (
+id int unsigned AUTO_INCREMENT,
+name varchar(255),
+position int DEFAULT NULL,
+PRIMARY KEY (id));
+INSERT INTO users (name, position) VALUES
+('user1','1'), ('user2','2'), ('user3','3'), ('user4','4'), ('user5','5');
+# Single-table update with non-mergeable derived table
+UPDATE users
+SET position = (SELECT COUNT(pos) + 1
+FROM (SELECT DISTINCT position AS pos FROM users) AS t2
+WHERE t2.pos < users.position)
+WHERE id = 3;
+# Single-table update with mergeable derived table
+# (but not merged due to being used in a subquery of an UPDATE statement)
+UPDATE users
+SET position = (SELECT COUNT(pos) + 1
+FROM (SELECT position AS pos FROM users) AS t2
+WHERE t2.pos < users.position)
+WHERE id = 3;
+# Multi-table update with non-mergeable derived table
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(pos) + 1
+FROM (SELECT DISTINCT position AS pos FROM users) AS t2
+WHERE t2.pos < users.position)
+WHERE id = 3;
+# Multi-table update with mergeable derived table
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(pos) + 1
+FROM (SELECT position AS pos FROM users) AS t2
+WHERE t2.pos < users.position)
+WHERE id = 3;
+UPDATE users
+SET position = (SELECT COUNT(t2.position) + 1
+FROM users AS t2
+WHERE t2.position < users.position)
+WHERE id = 3;
+ERROR HY000: You can't specify target table 'users' for update in FROM clause
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(t2.position) + 1
+FROM users AS t2
+WHERE t2.position < users.position)
+WHERE id = 3;
+ERROR HY000: You can't specify target table 'users' for update in FROM clause
+DROP TABLE users;
diff --git a/mysql-test/t/derived.test b/mysql-test/t/derived.test
@@ -2632,3 +2632,60 @@ eval explain $query;
 eval $query;
 
 DROP TABLE t1, t2, t3;
+
+--echo # Bug#22343301: Error 1093 (HY000): You can't specify target table'.'
+--echo #               for update in FROM clause
+
+CREATE TABLE users (
+ id int unsigned AUTO_INCREMENT,
+ name varchar(255),
+ position int DEFAULT NULL,
+ PRIMARY KEY (id));
+
+INSERT INTO users (name, position) VALUES
+ ('user1','1'), ('user2','2'), ('user3','3'), ('user4','4'), ('user5','5');
+
+--echo # Single-table update with non-mergeable derived table
+UPDATE users
+SET position = (SELECT COUNT(pos) + 1
+                FROM (SELECT DISTINCT position AS pos FROM users) AS t2
+                WHERE t2.pos < users.position)
+WHERE id = 3;
+
+--echo # Single-table update with mergeable derived table
+--echo # (but not merged due to being used in a subquery of an UPDATE statement)
+UPDATE users
+SET position = (SELECT COUNT(pos) + 1
+                FROM (SELECT position AS pos FROM users) AS t2
+                WHERE t2.pos < users.position)
+WHERE id = 3;
+
+--echo # Multi-table update with non-mergeable derived table
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(pos) + 1
+                FROM (SELECT DISTINCT position AS pos FROM users) AS t2
+                WHERE t2.pos < users.position)
+WHERE id = 3;
+
+--echo # Multi-table update with mergeable derived table
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(pos) + 1
+                FROM (SELECT position AS pos FROM users) AS t2
+                WHERE t2.pos < users.position)
+WHERE id = 3;
+
+--error ER_UPDATE_TABLE_USED
+UPDATE users
+SET position = (SELECT COUNT(t2.position) + 1
+                FROM users AS t2
+                WHERE t2.position < users.position)
+WHERE id = 3;
+
+--error ER_UPDATE_TABLE_USED
+UPDATE users, (SELECT 1) AS dummy
+SET position = (SELECT COUNT(t2.position) + 1
+                FROM users AS t2
+                WHERE t2.position < users.position)
+WHERE id = 3;
+
+DROP TABLE users;
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -1143,15 +1143,6 @@ bool mysql_prepare_update(THD *thd, const TABLE_LIST *update_table_ref,
   // Return covering keys derived from conditions and ORDER BY clause:
   *covering_keys_for_cond= update_table_ref->table->covering_keys;
 
-  // Check that table to be updated is not used in a subquery
-  TABLE_LIST *const duplicate= unique_table(thd, update_table_ref,
-                                            table_list->next_global, 0);
-  if (duplicate)
-  {
-    update_non_unique_table_error(table_list, "UPDATE", duplicate);
-    DBUG_RETURN(true);
-  }
-
   // Check the fields we are going to modify
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   table_list->set_want_privilege(UPDATE_ACL);
@@ -1179,6 +1170,15 @@ bool mysql_prepare_update(THD *thd, const TABLE_LIST *update_table_ref,
 
   thd->mark_used_columns= mark_used_columns_saved;
 
+  // Check that table to be updated is not used in a subquery
+  TABLE_LIST *const duplicate= unique_table(thd, update_table_ref,
+                                            table_list->next_global, 0);
+  if (duplicate)
+  {
+    update_non_unique_table_error(table_list, "UPDATE", duplicate);
+    DBUG_RETURN(true);
+  }
+
   if (setup_ftfuncs(select))
     DBUG_RETURN(true);                          /* purecov: inspected */
 
@@ -1569,12 +1569,33 @@ int Sql_cmd_update::mysql_multi_update_prepare(THD *thd)
     2) This validation is done by Query_result_update::prepare() but it is
     not called by PREPARE.
     3) So we do it below.
+    @todo Remove this code duplication as part of WL#6570
   */
   if (thd->stmt_arena->is_stmt_prepare())
   {
     if (setup_fields(thd, Ref_ptr_array(), update_value_list, SELECT_ACL,
                      NULL, false, false))
       DBUG_RETURN(true);
+
+    /*
+      Check that table being updated is not being used in a subquery, but
+      skip all tables of the UPDATE query block itself
+    */
+    select->exclude_from_table_unique_test= true;
+    for (TABLE_LIST *tr= select->leaf_tables; tr; tr= tr->next_leaf)
+    {
+      if (tr->lock_type != TL_READ &&
+          tr->lock_type != TL_READ_NO_INSERT)
+      {
+        TABLE_LIST *duplicate= unique_table(thd, tr, select->leaf_tables, 0);
+        if (duplicate != NULL)
+        {
+          update_non_unique_table_error(select->leaf_tables, "UPDATE",
+                                        duplicate);
+          DBUG_RETURN(true);
+        }
+      }
+    }
   }
 
   /* check single table update for view compound from several tables */
@@ -1602,29 +1623,6 @@ int Sql_cmd_update::mysql_multi_update_prepare(THD *thd)
 
   /* @todo: downgrade the metadata locks here. */
 
-  /*
-    Check that we are not using table that we are updating, but we should
-    skip all tables of UPDATE SELECT itself
-  */
-  select->exclude_from_table_unique_test= true;
-  for (TABLE_LIST *tl= select->leaf_tables; tl; tl= tl->next_leaf)
-  {
-    if (tl->lock_type != TL_READ &&
-        tl->lock_type != TL_READ_NO_INSERT)
-    {
-      TABLE_LIST *duplicate;
-      if ((duplicate= unique_table(thd, tl, table_list, 0)))
-      {
-        update_non_unique_table_error(table_list, "UPDATE", duplicate);
-        DBUG_RETURN(true);
-      }
-    }
-  }
-  /*
-    Set exclude_from_table_unique_test value back to FALSE. It is needed for
-    further check in Query_result_update::prepare whether to use record cache.
-  */
-  select->exclude_from_table_unique_test= false;
   /*
     Syntax rule for multi-table update prevents these constructs.
     But they are possible for single-table UPDATE against multi-table view.
@@ -1705,21 +1703,18 @@ Query_result_update::Query_result_update(TABLE_LIST *table_list,
 int Query_result_update::prepare(List<Item> &not_used_values,
                                  SELECT_LEX_UNIT *lex_unit)
 {
-  TABLE_LIST *table_ref;
   SQL_I_List<TABLE_LIST> update;
-  table_map tables_to_update;
-  Item_field *item;
   List_iterator_fast<Item> field_it(*fields);
   List_iterator_fast<Item> value_it(*values);
-  uint i, max_fields;
-  uint leaf_table_count= 0;
   DBUG_ENTER("Query_result_update::prepare");
 
+  SELECT_LEX *const select= lex_unit->first_select();
+
   thd->count_cuted_fields= CHECK_FIELD_WARN;
   thd->cuted_fields=0L;
   THD_STAGE_INFO(thd, stage_updating_main_table);
 
-  tables_to_update= get_table_map(fields);
+  const table_map tables_to_update= get_table_map(fields);
 
   if (!tables_to_update)
   {
@@ -1732,18 +1727,18 @@ int Query_result_update::prepare(List<Item> &not_used_values,
     TABLE::tmp_set by pointing TABLE::read_set to it and then restore it after
     setup_fields().
   */
-  for (table_ref= leaves; table_ref; table_ref= table_ref->next_leaf)
+  for (TABLE_LIST *tr= leaves; tr; tr= tr->next_leaf)
   {
-    if (tables_to_update & table_ref->map())
+    if (tables_to_update & tr->map())
     {
-      TABLE *const table= table_ref->table;
+      TABLE *const table= tr->table;
       DBUG_ASSERT(table->read_set == &table->def_read_set);
       table->read_set= &table->tmp_set;
       bitmap_clear_all(table->read_set);
     }
     // Resolving may be needed for subsequent executions
-    if (table_ref->check_option && !table_ref->check_option->fixed &&
-        table_ref->check_option->fix_fields(thd, NULL))
+    if (tr->check_option && !tr->check_option->fixed &&
+        tr->check_option->fix_fields(thd, NULL))
       DBUG_RETURN(1);        /* purecov: inspected */
   }
 
@@ -1755,11 +1750,11 @@ int Query_result_update::prepare(List<Item> &not_used_values,
   int error= setup_fields(thd, Ref_ptr_array(), *values, SELECT_ACL, NULL,
                           false, false);
 
-  for (table_ref= leaves; table_ref; table_ref= table_ref->next_leaf)
+  for (TABLE_LIST *tr= leaves; tr; tr= tr->next_leaf)
   {
-    if (tables_to_update & table_ref->map())
+    if (tables_to_update & tr->map())
     {
-      TABLE *const table= table_ref->table;
+      TABLE *const table= tr->table;
       table->read_set= &table->def_read_set;
       bitmap_union(table->read_set, &table->tmp_set);
       bitmap_clear_all(&table->tmp_set);
@@ -1769,31 +1764,54 @@ int Query_result_update::prepare(List<Item> &not_used_values,
   if (error)
     DBUG_RETURN(1);    
 
+  /*
+    Check that table being updated is not being used in a subquery, but
+    skip all tables of the UPDATE query block itself
+  */
+  select->exclude_from_table_unique_test= true;
+  for (TABLE_LIST *tr= select->leaf_tables; tr; tr= tr->next_leaf)
+  {
+    if (tr->lock_type != TL_READ &&
+        tr->lock_type != TL_READ_NO_INSERT)
+    {
+      TABLE_LIST *duplicate= unique_table(thd, tr, all_tables, 0);
+      if (duplicate != NULL)
+      {
+        update_non_unique_table_error(all_tables, "UPDATE", duplicate);
+        DBUG_RETURN(true);
+      }
+    }
+  }
+  /*
+    Set exclude_from_table_unique_test value back to FALSE. It is needed for
+    further check whether to use record cache.
+  */
+  select->exclude_from_table_unique_test= false;
   /*
     Save tables beeing updated in update_tables
     update_table->shared is position for table
     Don't use key read on tables that are updated
   */
 
   update.empty();
-  for (table_ref= leaves; table_ref; table_ref= table_ref->next_leaf)
+  uint leaf_table_count= 0;
+  for (TABLE_LIST *tr= leaves; tr; tr= tr->next_leaf)
   {
     /* TODO: add support of view of join support */
     leaf_table_count++;
-    if (tables_to_update & table_ref->map())
+    if (tables_to_update & tr->map())
     {
-      TABLE_LIST *tl= (TABLE_LIST*) thd->memdup(table_ref,
-						sizeof(*tl));
-      if (!tl)
+      TABLE_LIST *dup= (TABLE_LIST*) thd->memdup(tr, sizeof(*dup));
+      if (dup == NULL)
 	DBUG_RETURN(1);
 
-      TABLE *const table= table_ref->table;
+      TABLE *const table= tr->table;
 
-      update.link_in_list(tl, &tl->next_local);
-      table_ref->shared= tl->shared= table_count++;
+      update.link_in_list(dup, &dup->next_local);
+      tr->shared= dup->shared= table_count++;
       table->no_keyread=1;
       table->covering_keys.clear_all();
-      table->pos_in_table_list= tl;
+      table->pos_in_table_list= dup;
       if (table->triggers &&
           table->triggers->has_triggers(TRG_EVENT_UPDATE,
                                         TRG_ACTION_AFTER))
@@ -1823,31 +1841,33 @@ int Query_result_update::prepare(List<Item> &not_used_values,
   update_operations= (COPY_INFO**) thd->mem_calloc(sizeof(COPY_INFO*) *
                                                table_count);
 
-  if (thd->is_fatal_error)
+  if (thd->is_error())
     DBUG_RETURN(1);
-  for (i=0 ; i < table_count ; i++)
+  for (uint i= 0; i < table_count; i++)
   {
     fields_for_table[i]= new List_item;
     values_for_table[i]= new List_item;
   }
-  if (thd->is_fatal_error)
+  if (thd->is_error())
     DBUG_RETURN(1);
 
   /* Split fields into fields_for_table[] and values_by_table[] */
 
-  while ((item= (Item_field *) field_it++))
+  Item *item;
+  while ((item= field_it++))
   {
-    Item *value= value_it++;
-    uint offset= item->table_ref->shared;
-    fields_for_table[offset]->push_back(item);
+    Item_field *const field= down_cast<Item_field *>(item);
+    Item *const value= value_it++;
+    uint offset= field->table_ref->shared;
+    fields_for_table[offset]->push_back(field);
     values_for_table[offset]->push_back(value);
   }
   if (thd->is_fatal_error)
     DBUG_RETURN(1);
 
   /* Allocate copy fields */
-  max_fields=0;
-  for (i=0 ; i < table_count ; i++)
+  uint max_fields= 0;
+  for (uint i= 0; i < table_count; i++)
     set_if_bigger(max_fields, fields_for_table[i]->elements + leaf_table_count);
   copy_field= new Copy_field[max_fields];
 
