BUG#18326: Do not lock table for writing during prepare of statement

During statement prepare phase the tables were locked as if the
statement is being executed, however this is not necessary.

The solution is to not lock tables on statement prepare phase.
Opening tables is enough to prevent DDL on them, and during statement
prepare we do not access nor modify any data.

mysql-test/r/ps.result

@@ -2488,3 +2488,40 @@ execute stmt2 using @to_format, @dec;
 format(?, ?)
 10,000.00
 deallocate prepare stmt2;
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1 (i INT);
+INSERT INTO t1 VALUES (1);
+CREATE TABLE t2 (i INT);
+INSERT INTO t2 VALUES (2);
+LOCK TABLE t1 READ, t2 WRITE;
+PREPARE stmt1 FROM "SELECT i FROM t1";
+PREPARE stmt2 FROM "INSERT INTO t2 (i) VALUES (3)";
+EXECUTE stmt1;
+i
+1
+EXECUTE stmt2;
+SELECT * FROM t2;
+i
+2
+UNLOCK TABLES;
+SELECT * FROM t2;
+i
+2
+3
+ALTER TABLE t1 ADD COLUMN j INT;
+ALTER TABLE t2 ADD COLUMN j INT;
+INSERT INTO t1 VALUES (4, 5);
+INSERT INTO t2 VALUES (4, 5);
+EXECUTE stmt1;
+i
+1
+4
+EXECUTE stmt2;
+SELECT * FROM t2;
+i	j
+2	NULL
+3	NULL
+4	5
+3	NULL
+DROP TABLE t1, t2;
+End of 5.1 tests.

mysql-test/t/ps.test

@@ -2516,3 +2516,60 @@ set @to_format="10000";
 execute stmt2 using @to_format, @dec;
 deallocate prepare stmt2;
 
+
+#
+# BUG#18326: Do not lock table for writing during prepare of statement
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+
+CREATE TABLE t1 (i INT);
+INSERT INTO t1 VALUES (1);
+CREATE TABLE t2 (i INT);
+INSERT INTO t2 VALUES (2);
+
+LOCK TABLE t1 READ, t2 WRITE;
+
+connect (conn1, localhost, root, , );
+
+# Prepare never acquires the lock, and thus should not block.
+PREPARE stmt1 FROM "SELECT i FROM t1";
+PREPARE stmt2 FROM "INSERT INTO t2 (i) VALUES (3)";
+
+# This should not block because READ lock on t1 is shared.
+EXECUTE stmt1;
+
+# This should block because WRITE lock on t2 is exclusive.
+send EXECUTE stmt2;
+
+connection default;
+
+SELECT * FROM t2;
+UNLOCK TABLES;
+let $wait_condition= SELECT COUNT(*) = 2 FROM t2;
+--source include/wait_condition.inc
+SELECT * FROM t2;
+
+# DDL and DML works even if some client have a prepared statement
+# referencing the table.
+ALTER TABLE t1 ADD COLUMN j INT;
+ALTER TABLE t2 ADD COLUMN j INT;
+INSERT INTO t1 VALUES (4, 5);
+INSERT INTO t2 VALUES (4, 5);
+
+connection conn1;
+
+reap;
+EXECUTE stmt1;
+EXECUTE stmt2;
+SELECT * FROM t2;
+
+disconnect conn1;
+
+connection default;
+
+DROP TABLE t1, t2;
+
+
+--echo End of 5.1 tests.

sql/sql_prepare.cc

@@ -34,6 +34,12 @@ When one prepares a statement:
     [Params meta info (stubs only for now)]  (if Param_count > 0)
     [Columns meta info] (if Column_count > 0)
 
+  During prepare the tables used in a statement are opened, but no
+  locks are acquired.  Table opening will block any DDL during the
+  operation, and we do not need any locks as we neither read nor
+  modify any data during prepare.  Tables are closed after prepare
+  finishes.
+
 When one executes a statement:
 
   - Server gets the command 'COM_STMT_EXECUTE' to execute the
@@ -53,6 +59,10 @@ When one executes a statement:
   - Execute the query without re-parsing and send back the results
     to client
 
+  During execution of prepared statement tables are opened and locked
+  the same way they would for normal (non-prepared) statement
+  execution.  Tables are unlocked and closed after the execution.
+
 When one supplies long data for a placeholder:
 
   - Server gets the long data in pieces with command type
@@ -1120,12 +1130,8 @@ static int mysql_test_update(Prepared_statement *stmt,
   bool need_reopen;
   DBUG_ENTER("mysql_test_update");
 
-  if (update_precheck(thd, table_list))
-    goto error;
-
-  for ( ; ; )
-  {
-    if (open_tables(thd, &table_list, &table_count, 0))
+  if (update_precheck(thd, table_list) ||
+      open_normal_and_derived_tables(thd, table_list, 0))
     goto error;
 
   if (table_list->multitable_view)
@@ -1138,20 +1144,6 @@ static int mysql_test_update(Prepared_statement *stmt,
     DBUG_RETURN(2);
   }
 
-    if (!lock_tables(thd, table_list, table_count, &need_reopen))
-      break;
-    if (!need_reopen)
-      goto error;
-    close_tables_for_reopen(thd, &table_list);
-  }
-
-  /*
-    thd->fill_derived_tables() is false here for sure (because it is
-    preparation of PS, so we even do not check it).
-  */
-  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare))
-    goto error;
-
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   /* TABLE_LIST contain right privilages request */
   want_privilege= table_list->grant.want_privilege;
@@ -1209,7 +1201,7 @@ static bool mysql_test_delete(Prepared_statement *stmt,
   DBUG_ENTER("mysql_test_delete");
 
   if (delete_precheck(thd, table_list) ||
-      open_and_lock_tables(thd, table_list))
+      open_normal_and_derived_tables(thd, table_list, 0))
     goto error;
 
   if (!table_list->table)
@@ -1268,7 +1260,7 @@ static int mysql_test_select(Prepared_statement *stmt,
     goto error;
   }
 
-  if (open_and_lock_tables(thd, tables))
+  if (open_normal_and_derived_tables(thd, tables, 0))
     goto error;
 
   thd->used_tables= 0;                        // Updated by setup_fields
@@ -1329,7 +1321,7 @@ static bool mysql_test_do_fields(Prepared_statement *stmt,
   if (tables && check_table_access(thd, SELECT_ACL, tables, 0))
     DBUG_RETURN(TRUE);
 
-  if (open_and_lock_tables(thd, tables))
+  if (open_normal_and_derived_tables(thd, tables, 0))
     DBUG_RETURN(TRUE);
   DBUG_RETURN(setup_fields(thd, 0, *values, MARK_COLUMNS_NONE, 0, 0));
 }
@@ -1359,7 +1351,7 @@ static bool mysql_test_set_fields(Prepared_statement *stmt,
   set_var_base *var;
 
   if (tables && check_table_access(thd, SELECT_ACL, tables, 0) ||
-      open_and_lock_tables(thd, tables))
+      open_normal_and_derived_tables(thd, tables, 0))
     goto error;
 
   while ((var= it++))
@@ -1385,7 +1377,7 @@ error:
   NOTE
     This function won't directly open tables used in select. They should
     be opened either by calling function (and in this case you probably
-    should use select_like_stmt_test_with_open_n_lock()) or by
+    should use select_like_stmt_test_with_open()) or by
     "specific_prepare" call (like this happens in case of multi-update).
 
   RETURN VALUE
@@ -1413,14 +1405,14 @@ static bool select_like_stmt_test(Prepared_statement *stmt,
 }
 
 /*
-  Check internal SELECT of the prepared command (with opening and
-  locking of used tables).
+  Check internal SELECT of the prepared command (with opening of used
+  tables).
 
   SYNOPSIS
-    select_like_stmt_test_with_open_n_lock()
+    select_like_stmt_test_with_open()
       stmt                      prepared statement
-      tables                    list of tables to be opened and locked
-                                before calling specific_prepare function
+      tables                    list of tables to be opened before calling
+                                specific_prepare function
       specific_prepare          function of command specific prepare
       setup_tables_done_option  options to be passed to LEX::unit.prepare()
 
@@ -1430,19 +1422,20 @@ static bool select_like_stmt_test(Prepared_statement *stmt,
 */
 
 static bool
-select_like_stmt_test_with_open_n_lock(Prepared_statement *stmt,
+select_like_stmt_test_with_open(Prepared_statement *stmt,
                                 TABLE_LIST *tables,
                                 bool (*specific_prepare)(THD *thd),
                                 ulong setup_tables_done_option)
 {
-  DBUG_ENTER("select_like_stmt_test_with_open_n_lock");
+  DBUG_ENTER("select_like_stmt_test_with_open");
 
   /*
-    We should not call LEX::unit.cleanup() after this open_and_lock_tables()
-    call because we don't allow prepared EXPLAIN yet so derived tables will
-    clean up after themself.
+    We should not call LEX::unit.cleanup() after this
+    open_normal_and_derived_tables() call because we don't allow
+    prepared EXPLAIN yet so derived tables will clean up after
+    themself.
   */
-  if (open_and_lock_tables(stmt->thd, tables))
+  if (open_normal_and_derived_tables(stmt->thd, tables, 0))
     DBUG_RETURN(TRUE);
 
   DBUG_RETURN(select_like_stmt_test(stmt, specific_prepare,
@@ -1481,7 +1474,7 @@ static bool mysql_test_create_table(Prepared_statement *stmt)
   if (select_lex->item_list.elements)
   {
     select_lex->context.resolve_in_select_list= TRUE;
-    res= select_like_stmt_test_with_open_n_lock(stmt, tables, 0, 0);
+    res= select_like_stmt_test_with_open(stmt, tables, 0, 0);
   }
 
   /* put tables back for PS rexecuting */
@@ -1541,7 +1534,7 @@ static bool mysql_test_multidelete(Prepared_statement *stmt,
   }
 
   if (multi_delete_precheck(stmt->thd, tables) ||
-      select_like_stmt_test_with_open_n_lock(stmt, tables,
+      select_like_stmt_test_with_open(stmt, tables,
                                       &mysql_multi_delete_prepare,
                                       OPTION_SETUP_TABLES_DONE))
     goto error;
@@ -1559,15 +1552,16 @@ error:
 
 /*
   Wrapper for mysql_insert_select_prepare, to make change of local tables
-  after open_and_lock_tables() call.
+  after open_normal_and_derived_tables() call.
 
   SYNOPSIS
     mysql_insert_select_prepare_tester()
       thd                thread handle
 
   NOTE
-    We need to remove the first local table after open_and_lock_tables,
-    because mysql_handle_derived uses local tables lists.
+    We need to remove the first local table after
+    open_normal_and_derived_tables(), because mysql_handle_derived
+    uses local tables lists.
 */
 
 static bool mysql_insert_select_prepare_tester(THD *thd)
@@ -1624,7 +1618,7 @@ static bool mysql_test_insert_select(Prepared_statement *stmt,
   DBUG_ASSERT(first_local_table != 0);
 
   res=
-    select_like_stmt_test_with_open_n_lock(stmt, tables,
+    select_like_stmt_test_with_open(stmt, tables,
                                     &mysql_insert_select_prepare_tester,
                                     OPTION_SETUP_TABLES_DONE);
   /* revert changes  made by mysql_insert_select_prepare_tester */