MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Oystein.Grovlen Date:February 8 2010 6:14pm
Subject:bzr commit into mysql-5.5-next-mr branch (oystein.grovlen:3086)
Bug#49494
View as plain text  
#At file:///home/oysteing/mysql/mysql-next-4284/ based on revid:kostja@stripped

 3086 oystein.grovlen@stripped	2010-02-08
      BUG#49494 CREATE TABLE IF NOT EXISTS does wrong insert when view exists
                with the same name
      
      Problem observed was that with CREATE TABLE IF NOT EXISTS ... SELECT ...
      when a view with the same name as the table to be created, already existed.
      The values from the query was not inserted into the correct columns of
      the underlying tables of the view.
      
      The problem was that select_create::prepare did not take views into account,
      but inserted values directly into the underlying table in the original
      column order, not view column order.
      
      The fix has two major parts:
      1. If table already exists, call mysql_insert_select_prepare() before
         calling handle_select(). (As is done for "INSERT INTO ... SELECT ...").
         This will set up tables and views, including preparing the translation
         table for view columns.
      
      2. In order to be able to handle views, the select_create is changed
         to work on a List<Item_field> instead of directly on the list of
         fields of a table. Then, setup_fields() and check_insert_fields()
         can be called from select_create::prepare() to set up the right
         mapping to the Field objects of the underlying table. (Similar to
         select_insert::prepare())
      
      Since the check for table existence now has to be done before calling
      handle_select(), it makes sense to also generate a warning/error due to
      table existence at the same place.  Hence, this code has been moved from
      select_create::prepare().  This causes "Table already exists" errors to
      be generated earlier than before, and a few test cases had to be changed
      in order to make sure they still test for the same errors as before.
      
      Also added some comments to document select_create class.
     @ mysql-test/r/create.result
        Updated result file.
     @ mysql-test/r/union.result
        Updated result file.
     @ mysql-test/t/create.test
        Added test cases for Bug#49494.
        
        Changed a few test cases to avoid that one get "Table already exists" 
        instead of error we want to test for.  (Table existence is now checked 
        at an earlier stage.)
     @ mysql-test/t/union.test
        Changed test case to avoid that one get "Table already exists" instead
        error we want to test for.  (Table existence is now checked at an earlier
        stage.)
     @ sql/sql_class.h
         - Pass table_arg to select_insert constructor. Makes sure 
           insert_into_view is set up correctly.
         - Rename select_fields to fields_arg since it is now what it is used for.
         - Removed select_create::field since it is not used anymore.
         - Removed select_create::group since it was not in use before this change.
         - Added some comments to document select_create class.
     @ sql/sql_insert.cc
         - Change select_create::prepare() and select_create::store_values() to
           work on a List<Item_field> instead of directly on the list of
           fields of the table.
         - In case fields list has not been set up earlier, do so by calling
           TABLE::fill_item_list and also update the table's write_set.
           (Will not be done by check_insert_fields() in this case since Item_field
            objects have already been fixed.)
         - If fewer values are supplied by query than candidate columns, values 
           should go into right-most columns.  Hence, pop necessary number of
           left-most columns from fields list.
         - Call setup_fields/check_insert_fields which will do the necessary setup
           of fields and values, including translating view columns into table
           columns.
         - Moved error/warning handling of table existence to mysql_execute_command()
     @ sql/sql_parse.cc
         - If table already exists when executing a 
           "CREATE TABLE IF NOT EXISTS ... SELECT ..." statement,
           call mysql_insert_select_prepare() before calling handle_select()
           in order to set up tables and views. Also, tell handle_select() that
           tables has been set up, by passing OPTION_SETUP_TABLES_DONE.
         - By calling mysql_insert_select_prepare(), lex->field_list will be
           populated with item for table fields. Pass this list to the select_create
           constructor. (select_lex_item_list that was passed earlier did not
           seem to be used for anything.) 
         - Make sure to initialize lex->field_list so that it is empty in cases 
           where it is not populated by mysql_insert_select_prepare()

    modified:
      mysql-test/r/create.result
      mysql-test/r/union.result
      mysql-test/t/create.test
      mysql-test/t/union.test
      sql/sql_class.h
      sql/sql_insert.cc
      sql/sql_parse.cc
=== modified file 'mysql-test/r/create.result'
--- a/mysql-test/r/create.result	2010-02-06 10:28:06 +0000
+++ b/mysql-test/r/create.result	2010-02-08 18:14:46 +0000
@@ -591,7 +591,7 @@ b
 1
 drop table t1,t2;
 create table t1 (a int);
-create table t1 select * from t1;
+create table if not exists t1 select * from t1;
 ERROR HY000: You can't specify target table 't1' for update in FROM clause
 create table t2 union = (t1) select * from t1;
 ERROR HY000: 'test.t2' is not BASE TABLE
@@ -810,11 +810,8 @@ ERROR HY000: You can't specify target ta
 select * from t1;
 i
 1
-create table t1 select coalesce('a' collate latin1_swedish_ci,'b' collate latin1_bin);
+create table t2 select coalesce('a' collate latin1_swedish_ci,'b' collate latin1_bin);
 ERROR HY000: Illegal mix of collations (latin1_swedish_ci,EXPLICIT) and (latin1_bin,EXPLICIT) for operation 'coalesce'
-select * from t1;
-i
-1
 alter table t1 add primary key (i);
 create table if not exists t1 (select 2 as i) union all (select 2 as i);
 ERROR 23000: Duplicate entry '2' for key 'PRIMARY'
@@ -1977,3 +1974,59 @@ CREATE TABLE t1 LIKE t2;
 ERROR 42S01: Table 't1' already exists
 DROP TABLE t2;
 DROP TABLE t1;
+#
+# Bug #49494 CREATE TABLE IF NOT EXISTS does wrong insert when 
+#            view exists with the same name
+#
+CREATE TABLE t1 (a INTEGER, b INTEGER);
+CREATE VIEW v1 AS SELECT b, a FROM t1;
+CREATE VIEW v2 AS SELECT a FROM t1;
+CREATE TABLE IF NOT EXISTS v1 SELECT 1,2;
+Warnings:
+Note	1050	Table 'v1' already exists
+SELECT * FROM t1;
+a	b
+2	1
+CREATE TABLE IF NOT EXISTS v1 SELECT 3;
+Warnings:
+Note	1050	Table 'v1' already exists
+SELECT * FROM t1;
+a	b
+2	1
+3	NULL
+CREATE TABLE IF NOT EXISTS v2 SELECT 4;
+Warnings:
+Note	1050	Table 'v2' already exists
+SELECT * FROM t1;
+a	b
+2	1
+3	NULL
+4	NULL
+CREATE TABLE t2 (a INTEGER, b INTEGER);
+INSERT INTO t2 VALUES (5,6),(7,8);
+CREATE TABLE IF NOT EXISTS v1 SELECT * FROM t2;
+Warnings:
+Note	1050	Table 'v1' already exists
+SELECT * FROM t1;
+a	b
+2	1
+3	NULL
+4	NULL
+6	5
+8	7
+CREATE TABLE t3 (a INTEGER);
+INSERT INTO t3 VALUES (9),(10);
+CREATE TABLE IF NOT EXISTS v2 SELECT * FROM t3;
+Warnings:
+Note	1050	Table 'v2' already exists
+SELECT * FROM t1;
+a	b
+2	1
+3	NULL
+4	NULL
+6	5
+8	7
+9	NULL
+10	NULL
+DROP VIEW v1, v2;
+DROP TABLE t1, t2, t3;

=== modified file 'mysql-test/r/union.result'
--- a/mysql-test/r/union.result	2010-01-19 16:36:14 +0000
+++ b/mysql-test/r/union.result	2010-02-08 18:14:46 +0000
@@ -427,7 +427,7 @@ a
 ERROR 42000: Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'
 create temporary table t1 select a from t1 union select a from t2;
 drop temporary table t1;
-create table t1 select a from t1 union select a from t2;
+create table if not exists t1 select a from t1 union select a from t2;
 ERROR HY000: You can't specify target table 't1' for update in FROM clause
 select a from t1 union select a from t2 order by t2.a;
 ERROR 42S22: Unknown column 't2.a' in 'order clause'

=== modified file 'mysql-test/t/create.test'
--- a/mysql-test/t/create.test	2010-02-06 10:28:06 +0000
+++ b/mysql-test/t/create.test	2010-02-08 18:14:46 +0000
@@ -482,7 +482,7 @@ drop table t1,t2;
 #
 create table t1 (a int);
 --error 1093
-create table t1 select * from t1;
+create table if not exists t1 select * from t1;
 --error ER_WRONG_OBJECT
 create table t2 union = (t1) select * from t1;
 flush tables with read lock;
@@ -712,8 +712,7 @@ create table if not exists t1 select * f
 select * from t1;
 # Error before select_create::prepare()
 --error ER_CANT_AGGREGATE_2COLLATIONS
-create table t1 select coalesce('a' collate latin1_swedish_ci,'b' collate latin1_bin);
-select * from t1;
+create table t2 select coalesce('a' collate latin1_swedish_ci,'b' collate latin1_bin);
 # Error which happens during insertion of rows
 alter table t1 add primary key (i);
 --error ER_DUP_ENTRY
@@ -1668,3 +1667,31 @@ CREATE TABLE t1 LIKE t2;
 
 DROP TABLE t2;
 DROP TABLE t1;
+
+--echo #
+--echo # Bug #49494 CREATE TABLE IF NOT EXISTS does wrong insert when 
+--echo #            view exists with the same name
+--echo #
+CREATE TABLE t1 (a INTEGER, b INTEGER);
+CREATE VIEW v1 AS SELECT b, a FROM t1;
+CREATE VIEW v2 AS SELECT a FROM t1;
+
+CREATE TABLE IF NOT EXISTS v1 SELECT 1,2;
+SELECT * FROM t1;
+CREATE TABLE IF NOT EXISTS v1 SELECT 3;
+SELECT * FROM t1;
+CREATE TABLE IF NOT EXISTS v2 SELECT 4;
+SELECT * FROM t1;
+
+CREATE TABLE t2 (a INTEGER, b INTEGER);
+INSERT INTO t2 VALUES (5,6),(7,8);
+CREATE TABLE IF NOT EXISTS v1 SELECT * FROM t2;
+SELECT * FROM t1;
+
+CREATE TABLE t3 (a INTEGER);
+INSERT INTO t3 VALUES (9),(10);
+CREATE TABLE IF NOT EXISTS v2 SELECT * FROM t3;
+SELECT * FROM t1;
+
+DROP VIEW v1, v2;
+DROP TABLE t1, t2, t3;

=== modified file 'mysql-test/t/union.test'
--- a/mysql-test/t/union.test	2010-01-19 16:36:14 +0000
+++ b/mysql-test/t/union.test	2010-02-08 18:14:46 +0000
@@ -254,7 +254,7 @@ SELECT * FROM t1 UNION SELECT * FROM t2 
 create temporary table t1 select a from t1 union select a from t2;
 drop temporary table t1;
 --error 1093
-create table t1 select a from t1 union select a from t2;
+create table if not exists t1 select a from t1 union select a from t2;
 --error 1054
 select a from t1 union select a from t2 order by t2.a;
 drop table t1,t2;

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2010-02-06 10:28:06 +0000
+++ b/sql/sql_class.h	2010-02-08 18:14:46 +0000
@@ -2947,24 +2947,52 @@ class select_insert :public select_resul
 };
 
 
-class select_create: public select_insert {
-  ORDER *group;
+/** 
+  Class for creating a table and inserting select results into that table as
+  in "CREATE TABLE ... SELECT ...".
+
+  TODO: Refactor to take better advantage of similarities with select_insert
+  class.
+*/
+class select_create: public select_insert 
+{
+  /** Table to be created. */ 
   TABLE_LIST *create_table;
+  /** Information on table to be created */
   HA_CREATE_INFO *create_info;
+  /** Tables in select expression */
   TABLE_LIST *select_tables;
+  /** More information used to create table */
   Alter_info *alter_info;
-  Field **field;
   /* lock data for tmp table */
   MYSQL_LOCK *m_lock;
   /* m_lock or thd->extra_lock */
   MYSQL_LOCK **m_plock;
 public:
+  /** 
+    Constructor.
+    @param table_arg table to be created. If table already exists, 
+           table_arg->table should refer to this table.  Otherwise,
+           table_arg->table must be NULL.
+    @param create_info_par Information needed to create table. May be modified.
+           Hence, to support re-execution, one should use a copy of 
+           LEX::create_info.
+    @param alter_info_arg More information needed to create table. 
+           May be modified. For same reasons as above, 
+           one should use a copy of LEX::alter_info.
+    @param fields_arg list of fields that values from query will be inserted 
+           into.  If table already exists, this should be set up by caller.
+           Otherwise, list must be empty.
+    @param duplic how to handle duplicates during insertion
+    @param ignore if true, errors during insertion should be ignored
+    @param select_tables_arg table list of tables in select expression
+  */
   select_create (TABLE_LIST *table_arg,
 		 HA_CREATE_INFO *create_info_par,
                  Alter_info *alter_info_arg,
-		 List<Item> &select_fields,enum_duplicates duplic, bool ignore,
+		 List<Item> &fields_arg, enum_duplicates duplic, bool ignore,
                  TABLE_LIST *select_tables_arg)
-    :select_insert (NULL, NULL, &select_fields, 0, 0, duplic, ignore),
+    :select_insert (table_arg, NULL, &fields_arg, 0, 0, duplic, ignore),
     create_table(table_arg),
     create_info(create_info_par),
     select_tables(select_tables_arg),

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2010-02-06 10:28:06 +0000
+++ b/sql/sql_insert.cc	2010-02-08 18:14:46 +0000
@@ -3763,30 +3763,22 @@ select_create::prepare(List<Item> &value
   if (create_table->table)
   {
     /* Table already exists and was open at open_and_lock_tables() stage. */
-    if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS)
-    {
-      /* Mark that table existed */
-      create_info->table_existed= 1;
-      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
-                          ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
-                          create_table->table_name);
-      if (thd->is_current_stmt_binlog_format_row())
-        binlog_show_create_table(&(create_table->table), 1);
-      table= create_table->table;
-    }
-    else
-    {
-      my_error(ER_TABLE_EXISTS_ERROR, MYF(0), create_table->table_name);
-      DBUG_RETURN(-1);
-    }
+    if (thd->is_current_stmt_binlog_format_row())
+      binlog_show_create_table(&(create_table->table), 1);
+    table= create_table->table;
   }
   else
+  {
     if (!(table= create_table_from_items(thd, create_info, create_table,
                                          alter_info, &values,
                                          &extra_lock, hook_ptr)))
       /* abort() deletes table */
       DBUG_RETURN(-1);
 
+    create_table->table= table;
+    create_table->updatable= 1;
+  }
+
   if (extra_lock)
   {
     DBUG_ASSERT(m_plock == NULL);
@@ -3799,18 +3791,39 @@ select_create::prepare(List<Item> &value
     *m_plock= extra_lock;
   }
 
-  if (table->s->fields < values.elements)
+
+  if (fields->elements == 0 && values.elements != 0)
+  {
+    table->fill_item_list(fields);
+
+    /* fill_item_list the Item_field objects it creates. 
+       This means that check_insert_fields will not update write_set.  
+       Hence, we will do it here.
+    */
+    if (fields->elements >= values.elements)
+    {
+      /* Mark all fields that are given values */
+      for (Field **f= table->field + table->s->fields - values.elements ; 
+           *f ; f++)
+        bitmap_set_bit(table->write_set, (*f)->field_index);
+    }
+  }
+   
+  if (fields->elements < values.elements)
   {
     my_error(ER_WRONG_VALUE_COUNT_ON_ROW, MYF(0), 1);
     DBUG_RETURN(-1);
   }
 
- /* First field to copy */
-  field= table->field+table->s->fields - values.elements;
-
-  /* Mark all fields that are given values */
-  for (Field **f= field ; *f ; f++)
-    bitmap_set_bit(table->write_set, (*f)->field_index);
+  /* Values should go into right-most columns */
+  while (fields->elements > values.elements)
+    fields->pop();
+  
+  table_map map= 0;
+  if (setup_fields(thd, 0, values, MARK_COLUMNS_READ, 0, 0) ||
+      check_insert_fields(thd, table_list, *fields, values,
+                          !insert_into_view, 1, &map))
+    DBUG_RETURN(-1);
 
   /* Don't set timestamp if used */
   table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
@@ -3889,7 +3902,7 @@ select_create::binlog_show_create_table(
 
 void select_create::store_values(List<Item> &values)
 {
-  fill_record_n_invoke_before_triggers(thd, field, values, 1,
+  fill_record_n_invoke_before_triggers(thd, *fields, values, 1,
                                        table->triggers, TRG_EVENT_INSERT);
 }
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-02-06 10:28:06 +0000
+++ b/sql/sql_parse.cc	2010-02-08 18:14:46 +0000
@@ -2354,7 +2354,10 @@ case SQLCOM_PREPARE:
     if (select_lex->item_list.elements)		// With select
     {
       select_result *result;
-
+      ulong setup_tables_done_option= 0;  // Will be set if we do setup for 
+                                          // an existing table
+      lex->field_list.empty();  // field_list will be used for the columns
+                                // that will be inserted into
       /*
         If:
         a) we inside an SP and there was NAME_CONST substitution,
@@ -2416,6 +2419,30 @@ case SQLCOM_PREPARE:
 
       if (!(res= open_and_lock_tables_derived(thd, lex->query_tables, TRUE, 0)))
       {
+        if (create_table->table)
+        {
+          /* Table already exists and was opened by open_and_lock_tables() */
+          if (create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS)
+          {
+            create_info.table_existed= 1;
+            push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+                                ER_TABLE_EXISTS_ERROR, 
+                                ER(ER_TABLE_EXISTS_ERROR),
+                                create_table->table_name);
+
+            // Setup tables to bind views etc before calling handle_select
+            if (res= mysql_insert_select_prepare(thd))
+              goto end_with_restore_list;
+            setup_tables_done_option= OPTION_SETUP_TABLES_DONE; 
+          }
+          else
+          {
+            my_error(ER_TABLE_EXISTS_ERROR, MYF(0), create_table->table_name);
+            res = 1;
+            goto end_with_restore_list;
+          }
+        }
+
         /*
           Is table which we are changing used somewhere in other parts
           of query
@@ -2462,7 +2489,7 @@ case SQLCOM_PREPARE:
         if ((result= new select_create(create_table,
                                        &create_info,
                                        &alter_info,
-                                       select_lex->item_list,
+                                       lex->field_list,
                                        lex->duplicates,
                                        lex->ignore,
                                        select_tables)))
@@ -2471,7 +2498,7 @@ case SQLCOM_PREPARE:
             CREATE from SELECT give its SELECT_LEX for SELECT,
             and item_list belong to SELECT
           */
-          res= handle_select(thd, lex, result, 0);
+          res= handle_select(thd, lex, result, setup_tables_done_option);
           delete result;
         }
         


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.5-next-mr branch (oystein.grovlen:3086)Bug#49494Oystein.Grovlen8 Feb