Better bug fix for:

#9728  'Decreased functionality in "on duplicate key update
#8147  'a column proclaimed ambigous in INSERT ... SELECT .. ON DUPLICATE'

This ensures fields are uniquely qualified and also that one can't update other tables in the ON DUPLICATE KEY UPDATE part


mysql-test/r/insert_select.result:
  More tests for bug #9728 and #8147
mysql-test/r/insert_update.result:
  Updated tests after changing how INSERT ... SELECT .. ON DUPLICATE KEY works
mysql-test/t/insert_select.test:
  More tests for bug #9728 and #8147
mysql-test/t/insert_update.test:
  Updated tests after changing how INSERT ... SELECT .. ON DUPLICATE KEY works
mysys/my_access.c:
  Cleanup (shorter loop variable names)
sql/ha_ndbcluster.cc:
  Indentation fixes
sql/item.cc:
  Remove item_flags
sql/item.h:
  Remove item_flags
sql/mysql_priv.h:
  New arguments to mysql_prepare_insert
sql/sql_base.cc:
  Remove old fix for bug #8147
sql/sql_insert.cc:
  Extend mysql_prepare_insert() with new field list for tables that can be used in the values port of ON DUPLICATE KEY UPDATE
sql/sql_parse.cc:
  Revert fix for #9728
  Allow one to use other tables in ON DUPLICATE_KEY for INSERT ... SELECT if there is no GROUP BY clause
sql/sql_prepare.cc:
  New arguments to mysql_prepare_insert
sql/sql_yacc.yy:
  Revert bug fix for #9728

14 files changed, 123 insertions, 78 deletions

diff --git a/mysql-test/r/insert_select.result b/mysql-test/r/insert_select.result
index 69eb64f08ea..2ac73fe7662 100644
--- a/mysql-test/r/insert_select.result
+++ b/mysql-test/r/insert_select.result
@@ -1,4 +1,4 @@
-drop table if exists t1,t2;
+drop table if exists t1,t2,t3;
 create table t1 (bandID MEDIUMINT UNSIGNED NOT NULL PRIMARY KEY, payoutID SMALLINT UNSIGNED NOT NULL);
 insert into t1 (bandID,payoutID) VALUES (1,6),(2,6),(3,4),(4,9),(5,10),(6,1),(7,12),(8,12);
 create table t2 (payoutID SMALLINT UNSIGNED NOT NULL PRIMARY KEY);
@@ -636,16 +636,35 @@ ff1	ff2
 drop table t1, t2;
 create table t1 (a int unique);
 create table t2 (a int, b int);
+create table t3 (c int, d int);
 insert into t1 values (1),(2);
 insert into t2 values (1,2);
+insert into t3 values (1,6),(3,7);
 select * from t1;
 a
 1
 2
-insert into t1 select t2.a from t2 on duplicate key update a= a + t2.b;
+insert into t1 select a from t2 on duplicate key update a= t1.a + t2.b;
 select * from t1;
 a
 2
 3
-drop table t1;
-drop table t2;
+insert into t1 select a+1 from t2 on duplicate key update t1.a= t1.a + t2.b+1;
+select * from t1;
+a
+3
+5
+insert into t1 select t3.c from t3 on duplicate key update a= a + t3.d;
+select * from t1;
+a
+1
+5
+10
+insert into t1 select t2.a from t2 group by t2.a on duplicate key update a= a + 10;
+insert into t1 select t2.a from t2 on duplicate key update a= a + t2.b;
+ERROR 23000: Column 'a' in field list is ambiguous
+insert into t1 select t2.a from t2 on duplicate key update t2.a= a + t2.b;
+ERROR 42S02: Unknown table 't2' in field list
+insert into t1 select t2.a from t2 group by t2.a on duplicate key update a= t1.a + t2.b;
+ERROR 42S02: Unknown table 't2' in field list
+drop table t1,t2,t3;
diff --git a/mysql-test/r/insert_update.result b/mysql-test/r/insert_update.result
index 2143538469b..150f4ef26c7 100644
--- a/mysql-test/r/insert_update.result
+++ b/mysql-test/r/insert_update.result
@@ -143,7 +143,7 @@ INSERT t1 VALUES (1,2,10), (3,4,20);
 CREATE TABLE t2 (a INT, b INT, c INT, d INT);
 INSERT t2 VALUES (5,6,30,1), (7,4,40,1), (8,9,60,1);
 INSERT t2 VALUES (2,1,11,2), (7,4,40,2);
-INSERT t1 SELECT a,b,c FROM t2 WHERE d=1 ON DUPLICATE KEY UPDATE c=c+100;
+INSERT t1 SELECT a,b,c FROM t2 WHERE d=1 ON DUPLICATE KEY UPDATE c=t1.c+100;
 SELECT * FROM t1;
 a	b	c
 1	2	10
@@ -158,6 +158,8 @@ a	b	c
 5	0	30
 8	9	60
 INSERT t1 SELECT a,b,c FROM t2 WHERE d=2 ON DUPLICATE KEY UPDATE c=c+VALUES(a);
+ERROR 23000: Column 'c' in field list is ambiguous
+INSERT t1 SELECT a,b,c FROM t2 WHERE d=2 ON DUPLICATE KEY UPDATE c=t1.c+VALUES(t1.a);
 SELECT *, VALUES(a) FROM t1;
 a	b	c	VALUES(a)
 1	2	10	NULL
@@ -174,7 +176,7 @@ select * from t1;
 a
 1
 2
-insert ignore into t1 select a from t1 on duplicate key update a=a+1 ;
+insert ignore into t1 select a from t1 as t2 on duplicate key update a=t1.a+1 ;
 select * from t1;
 a
 1
@@ -185,5 +187,7 @@ a
 2
 3
 insert into t1 select a from t1 on duplicate key update a=a+1 ;
-ERROR 23000: Duplicate entry '3' for key 1
+ERROR 23000: Column 'a' in field list is ambiguous
+insert ignore into t1 select a from t1 on duplicate key update a=t1.a+1 ;
+ERROR 23000: Column 't1.a' in field list is ambiguous
 drop table t1;
diff --git a/mysql-test/t/insert_select.test b/mysql-test/t/insert_select.test
index 7402940fa52..67799873b73 100644
--- a/mysql-test/t/insert_select.test
+++ b/mysql-test/t/insert_select.test
@@ -3,7 +3,7 @@
 #
 
 --disable_warnings
-drop table if exists t1,t2;
+drop table if exists t1,t2,t3;
 --enable_warnings
 
 create table t1 (bandID MEDIUMINT UNSIGNED NOT NULL PRIMARY KEY, payoutID SMALLINT UNSIGNED NOT NULL);
@@ -182,10 +182,24 @@ drop table t1, t2;
 #
 create table t1 (a int unique);
 create table t2 (a int, b int);
+create table t3 (c int, d int);
 insert into t1 values (1),(2);
 insert into t2 values (1,2);
+insert into t3 values (1,6),(3,7);
 select * from t1;
-insert into t1 select t2.a from t2 on duplicate key update a= a + t2.b;
+insert into t1 select a from t2 on duplicate key update a= t1.a + t2.b;
 select * from t1;
-drop table t1;
-drop table t2; 
+insert into t1 select a+1 from t2 on duplicate key update t1.a= t1.a + t2.b+1;
+select * from t1;
+insert into t1 select t3.c from t3 on duplicate key update a= a + t3.d;
+select * from t1;
+insert into t1 select t2.a from t2 group by t2.a on duplicate key update a= a + 10;
+
+#Some error cases
+--error 1052
+insert into t1 select t2.a from t2 on duplicate key update a= a + t2.b;
+--error 1109
+insert into t1 select t2.a from t2 on duplicate key update t2.a= a + t2.b;
+--error 1109
+insert into t1 select t2.a from t2 group by t2.a on duplicate key update a= t1.a + t2.b;
+drop table t1,t2,t3;
diff --git a/mysql-test/t/insert_update.test b/mysql-test/t/insert_update.test
index f5857840588..3d6297d8d7a 100644
--- a/mysql-test/t/insert_update.test
+++ b/mysql-test/t/insert_update.test
@@ -72,11 +72,13 @@ CREATE TABLE t2 (a INT, b INT, c INT, d INT);
 # column names deliberately clash with columns in t1 (Bug#8147)
 INSERT t2 VALUES (5,6,30,1), (7,4,40,1), (8,9,60,1);
 INSERT t2 VALUES (2,1,11,2), (7,4,40,2);
-INSERT t1 SELECT a,b,c FROM t2 WHERE d=1 ON DUPLICATE KEY UPDATE c=c+100;
+INSERT t1 SELECT a,b,c FROM t2 WHERE d=1 ON DUPLICATE KEY UPDATE c=t1.c+100;
 SELECT * FROM t1;
 INSERT t1 SET a=5 ON DUPLICATE KEY UPDATE b=0;
 SELECT * FROM t1;
+--error 1052
 INSERT t1 SELECT a,b,c FROM t2 WHERE d=2 ON DUPLICATE KEY UPDATE c=c+VALUES(a);
+INSERT t1 SELECT a,b,c FROM t2 WHERE d=2 ON DUPLICATE KEY UPDATE c=t1.c+VALUES(t1.a);
 SELECT *, VALUES(a) FROM t1;
 DROP TABLE t1;
 DROP TABLE t2;
@@ -89,10 +91,12 @@ create table t1 (a int not null unique) engine=myisam;
 insert into t1 values (1),(2);
 insert ignore into t1 select 1 on duplicate key update a=2;
 select * from t1;
-insert ignore into t1 select a from t1 on duplicate key update a=a+1 ;
+insert ignore into t1 select a from t1 as t2 on duplicate key update a=t1.a+1 ;
 select * from t1;
 insert into t1 select 1 on duplicate key update a=2;
 select * from t1;
---error 1062
+--error 1052
 insert into t1 select a from t1 on duplicate key update a=a+1 ;
+--error 1052
+insert ignore into t1 select a from t1 on duplicate key update a=t1.a+1 ;
 drop table t1;
diff --git a/mysys/my_access.c b/mysys/my_access.c
index 1b9ad6ff380..8fc83a020cf 100644
--- a/mysys/my_access.c
+++ b/mysys/my_access.c
@@ -98,17 +98,16 @@ int check_if_legal_filename(const char *path)
 
   for (reserved_name= reserved_names; *reserved_name; reserved_name++)
   {
+    const char *reserved= *reserved_name;       /* never empty */
     const char *name= path;
-    const char *current_reserved_name= *reserved_name;
     
-    while (name != end && *current_reserved_name)
+    do
     {
-      if (*current_reserved_name != my_toupper(&my_charset_latin1, *name))
+      if (*reserved != my_toupper(&my_charset_latin1, *name))
         break;
-      current_reserved_name++;
       if (++name == end)
         DBUG_RETURN(1);                         /* Found wrong path */
-    }
+    } while (*++reserved);
   }
   DBUG_RETURN(0);
 }
diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
index 88b7ff4dcb8..d5153269843 100644
--- a/sql/ha_ndbcluster.cc
+++ b/sql/ha_ndbcluster.cc
@@ -439,8 +439,10 @@ int ha_ndbcluster::ndb_err(NdbConnection *trans)
     if (m_rows_to_insert == 1)
       m_dupkey= table->primary_key;
     else
-      // We are batching inserts, offending key is not available
+    {
+      /* We are batching inserts, offending key is not available */
       m_dupkey= (uint) -1;
+    }
   }
   DBUG_RETURN(res);
 }
diff --git a/sql/item.cc b/sql/item.cc
index 9c5bf499d11..c96794ff482 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -65,7 +65,6 @@ Item::Item():
 	place == IN_HAVING)
       thd->lex->current_select->select_n_having_items++;
   }
-  item_flags= 0;
 }
 
 /*
@@ -84,8 +83,7 @@ Item::Item(THD *thd, Item *item):
   unsigned_flag(item->unsigned_flag),
   with_sum_func(item->with_sum_func),
   fixed(item->fixed),
-  collation(item->collation),
-  item_flags(item->item_flags)
+  collation(item->collation)
 {
   next= thd->free_list;				// Put in free list
   thd->free_list= this;
diff --git a/sql/item.h b/sql/item.h
index 82ab5a66cfb..8de2adeb730 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -107,11 +107,6 @@ public:
 
 typedef bool (Item::*Item_processor)(byte *arg);
 
-/*
- See comments for sql_yacc.yy: insert_update_elem rule
- */
-#define MY_ITEM_PREFER_1ST_TABLE 1
-
 class Item {
   Item(const Item &);			/* Prevent use of these */
   void operator=(Item &);
@@ -147,7 +142,6 @@ public:
   my_bool with_sum_func;
   my_bool fixed;                        /* If item fixed with fix_fields */
   DTCollation collation;
-  uint8 item_flags;		/* Flags on how item should be processed */
 
   // alloc & destruct is done as start of select using sql_alloc
   Item();
@@ -333,11 +327,6 @@ public:
     cleanup();
     delete this;
   }
-  virtual bool set_flags_processor(byte *args)
-  {
-    this->item_flags|= *((uint8*)args);
-    return false;
-  }
 };
 
 
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 2bcaa1ecc2d..cc58e34d582 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -576,7 +576,8 @@ int mysql_multi_update_lock(THD *thd,
 			    List<Item> *fields,
 			    SELECT_LEX *select_lex);
 int mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
-			 TABLE_LIST *insert_table_list, TABLE *table,
+			 TABLE_LIST *insert_table_list,
+                         TABLE_LIST *dup_table_list, TABLE *table,
 			 List<Item> &fields, List_item *values,
 			 List<Item> &update_fields,
 			 List<Item> &update_values, enum_duplicates duplic);
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 84c03bee917..9a5b2522be9 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1993,7 +1993,7 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
   const char *name=item->field_name;
   uint length=(uint) strlen(name);
   char name_buff[NAME_LEN+1];
-
+  bool allow_rowid;
 
   if (item->cached_table)
   {
@@ -2085,9 +2085,8 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
 	return (Field*) not_found_field;
     return (Field*) 0;
   }
-  bool allow_rowid= tables && !tables->next;	// Only one table
-  uint table_idx= 0;
-  for (; tables ; tables=tables->next, table_idx++)
+  allow_rowid= tables && !tables->next;         // Only one table
+  for (; tables ; tables=tables->next)
   {
     if (!tables->table)
     {
@@ -2116,8 +2115,6 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
 	return (Field*) 0;
       }
       found= field;
-      if (table_idx == 0 && item->item_flags & MY_ITEM_PREFER_1ST_TABLE) 
-        break;
     }
   }
   if (found)
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index f09d3214c74..deccc1d4dca 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -271,7 +271,8 @@ int mysql_insert(THD *thd,TABLE_LIST *table_list,
   thd->used_tables=0;
   values= its++;
 
-  if (mysql_prepare_insert(thd, table_list, insert_table_list, table,
+  if (mysql_prepare_insert(thd, table_list, insert_table_list,
+                           insert_table_list, table,
 			   fields, values, update_fields,
 			   update_values, duplic))
     goto abort;
@@ -499,22 +500,37 @@ abort:
 
   SYNOPSIS
     mysql_prepare_insert()
-    thd			- thread handler
-    table_list		- global table list
-    insert_table_list	- local table list of INSERT SELECT_LEX
-    values              - values to insert. NULL for INSERT ... SELECT
+    thd			thread handler
+    table_list		global table list (not including first table for
+			INSERT ... SELECT)
+    insert_table_list	Table we are inserting into (for INSERT ... SELECT)
+    dup_table_list	Tables to be used in ON DUPLICATE KEY
+			It's either all global tables or only the table we
+                        insert into, depending on if we are using GROUP BY
+                        in the SELECT clause).
+    values              Values to insert. NULL for INSERT ... SELECT
+
+  TODO (in far future)
+    In cases of:
+    INSERT INTO t1 SELECT a, sum(a) as sum1 from t2 GROUP BY a
+    ON DUPLICATE KEY ...
+    we should be able to refer to sum1 in the ON DUPLICATE KEY part
 
   RETURN VALUE
-    0  - OK
-    -1 - error (message is not sent to user)
+    0   OK
+    -1  error (message is not sent to user)
 */
+
 int mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
-			 TABLE_LIST *insert_table_list, TABLE *table,
+			 TABLE_LIST *insert_table_list,
+                         TABLE_LIST *dup_table_list,
+                         TABLE *table,
 			 List<Item> &fields, List_item *values,
 			 List<Item> &update_fields, List<Item> &update_values,
 			 enum_duplicates duplic)
 {
   DBUG_ENTER("mysql_prepare_insert");
+
   if (duplic == DUP_UPDATE && !table->insert_values)
   {
     /* it should be allocated before Item::fix_fields() */
@@ -528,7 +544,7 @@ int mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
       (values && setup_fields(thd, 0, insert_table_list, *values, 0, 0, 0)) ||
       (duplic == DUP_UPDATE &&
        (check_update_fields(thd, table, insert_table_list, update_fields) ||
-        setup_fields(thd, 0, insert_table_list, update_values, 1, 0, 0))))
+        setup_fields(thd, 0, dup_table_list, update_values, 1, 0, 0))))
     DBUG_RETURN(-1);
   if (values && find_real_table_in_list(table_list->next, table_list->db,
                                         table_list->real_name))
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 233104c9a90..c0283f81315 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -1943,10 +1943,10 @@ mysql_execute_command(THD *thd)
   if (tables || &lex->select_lex != lex->all_selects_list)
     mysql_reset_errors(thd);
 
-  /* 
-     When subselects or time_zone info is used in a query
-     we create a new TABLE_LIST containing all referenced tables
-     and set local variable 'tables' to point to this list.
+  /*
+    When subselects or time_zone info is used in a query
+    we create a new TABLE_LIST containing all referenced tables
+    and set local variable 'tables' to point to this list.
   */
   if ((&lex->select_lex != lex->all_selects_list ||
        lex->time_zone_tables_used) &&
@@ -2831,6 +2831,8 @@ unsent_create_error:
   case SQLCOM_INSERT_SELECT:
   {
     TABLE_LIST *first_local_table= (TABLE_LIST *) select_lex->table_list.first;
+    TABLE_LIST dup_tables;
+    TABLE *insert_table;
     if ((res= insert_precheck(thd, tables)))
       break;
 
@@ -2856,14 +2858,27 @@ unsent_create_error:
     if ((res= open_and_lock_tables(thd, tables)))
       break;
       
+    insert_table= tables->table;
     /* Skip first table, which is the table we are inserting in */
     select_lex->table_list.first= (byte*) first_local_table->next;
-    
-    if (!(res= mysql_prepare_insert(thd, tables, first_local_table, 
-				    tables->table, lex->field_list, 0,
+    tables= (TABLE_LIST *) select_lex->table_list.first;
+    dup_tables= *first_local_table;
+    first_local_table->next= 0;
+    if (select_lex->group_list.elements != 0)
+    {
+      /*
+        When we are using GROUP BY we can't refere to other tables in the
+        ON DUPLICATE KEY part
+      */         
+      dup_tables.next= 0;
+    }
+
+    if (!(res= mysql_prepare_insert(thd, tables, first_local_table,
+				    &dup_tables, insert_table,
+                                    lex->field_list, 0,
 				    lex->update_list, lex->value_list,
 				    lex->duplicates)) &&
-        (result= new select_insert(tables->table, &lex->field_list,
+        (result= new select_insert(insert_table, &lex->field_list,
 				   &lex->update_list, &lex->value_list,
                                    lex->duplicates, lex->ignore)))
     {
@@ -2876,7 +2891,7 @@ unsent_create_error:
       /* revert changes for SP */
       lex->select_lex.resolve_mode= SELECT_LEX::INSERT_MODE;
       delete result;
-      tables->table->insert_values= 0;
+      insert_table->insert_values= 0;
       if (thd->net.report_error)
         res= -1;
     }
@@ -4950,10 +4965,11 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       the slow query log, and the relay log (if it exists).
     */
 
-    /* 
-     Writing this command to the binlog may result in infinite loops when doing
-     mysqlbinlog|mysql, and anyway it does not really make sense to log it
-     automatically (would cause more trouble to users than it would help them)
+    /*
+      Writing this command to the binlog may result in infinite loops when
+      doing mysqlbinlog|mysql, and anyway it does not really make sense to
+      log it automatically (would cause more trouble to users than it would
+      help them)
     */
     tmp_write_to_binlog= 0;
     mysql_log.new_file(1);
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 1f5bb04c802..9e2612c5661 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -918,6 +918,7 @@ static int mysql_test_insert(Prepared_statement *stmt,
 
     table_list->table->insert_values=(byte *)1; // don't allocate insert_values
     if ((res= mysql_prepare_insert(thd, table_list, insert_table_list,
+                                   insert_table_list,
 				   table_list->table, fields, values,
 				   update_fields, update_values, duplic)))
       goto error;
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index 460234de156..bc21649fe54 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -4239,24 +4239,9 @@ insert_update_elem:
 	simple_ident equal expr_or_default
 	{
 	  LEX *lex= Lex;
-          uint8 tmp= MY_ITEM_PREFER_1ST_TABLE;
 	  if (lex->update_list.push_back($1) || 
 	      lex->value_list.push_back($3))
 	    YYABORT;
-          /* 
-            INSERT INTO a1(a) SELECT b1.a FROM b1 ON DUPLICATE KEY 
-              UPDATE a= a + b1.b
-   
-            Set MY_ITEM_PREFER_1ST_TABLE flag to $1 and $3 items
-            to prevent find_field_in_tables() doing further item searching 
-            if it finds item occurence in first table in insert_table_list. 
-            This allows to avoid ambiguity in resolving 'a' field in 
-            example above.
-          */
-          $1->walk(&Item::set_flags_processor,
-                  (byte *) &tmp);
-          $3->walk(&Item::set_flags_processor,
-                  (byte *) &tmp);
 	};
 
 opt_low_priority: