From: Date: April 7 2008 2:31pm
Subject: bk commit into 5.1 tree (ramil:1.2576) BUG#35732
List-Archive: http://lists.mysql.com/commits/44989
X-Bug: 35732
Message-Id: <20080407123125.A69F13400074@ramil.myoffice.izhnet.ru>

Below is the list of changes that have just been committed into a local
5.1 repository of ramil.  When ramil does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-07 17:31:20+05:00, ramil@stripped +3 -0
  Fix for bug #35732: read-only blocks SELECT statements in InnoDB
  
  Problem: SELECTs prohibited for a transactional SE in autocommit mode
  if read_only is set.
  
  Fix: allow them.

  mysql-test/r/read_only_innodb.result@stripped, 2008-04-07 17:31:18+05:00, ramil@stripped +12 -0
    Fix for bug #35732: read-only blocks SELECT statements in InnoDB
      - test result.

  mysql-test/t/read_only_innodb.test@stripped, 2008-04-07 17:31:18+05:00, ramil@stripped +22 -0
    Fix for bug #35732: read-only blocks SELECT statements in InnoDB
      - test case.

  sql/handler.cc@stripped, 2008-04-07 17:31:18+05:00, ramil@stripped +26 -23
    Fix for bug #35732: read-only blocks SELECT statements in InnoDB
      - in autocommit mode thd->transaction.all list is empty thus 
        is_real_trans set to TRUE for any SELECTs, so using it in the
        "read_only" check is insufficient.
        ha_check_and_coalesce_trx_read_only() changed to return number
        of engines with read-write changes. This value is used in the
        "read-only" check.

diff -Nrup a/mysql-test/r/read_only_innodb.result b/mysql-test/r/read_only_innodb.result
--- a/mysql-test/r/read_only_innodb.result	2006-11-21 07:40:31 +04:00
+++ b/mysql-test/r/read_only_innodb.result	2008-04-07 17:31:18 +05:00
@@ -16,3 +16,15 @@ ERROR HY000: The MySQL server is running
 set global read_only=0;
 drop table table_11733 ;
 drop user test@localhost;
+GRANT CREATE, SELECT, DROP ON *.* TO test@localhost;
+CREATE TABLE t1(a INT) ENGINE=INNODB;
+INSERT INTO t1 VALUES (0), (1);
+SET GLOBAL read_only=1;
+SELECT * FROM t1;
+a
+0
+1
+SET GLOBAL read_only=0;
+DROP TABLE t1;
+DROP USER test@localhost;
+echo End of 5.1 tests 
diff -Nrup a/mysql-test/t/read_only_innodb.test b/mysql-test/t/read_only_innodb.test
--- a/mysql-test/t/read_only_innodb.test	2006-11-21 07:40:31 +04:00
+++ b/mysql-test/t/read_only_innodb.test	2008-04-07 17:31:18 +05:00
@@ -36,8 +36,30 @@ select * from table_11733 ;
 -- error ER_OPTION_PREVENTS_STATEMENT
 COMMIT;
 
+disconnect con1;
+
 connection default;
 set global read_only=0;
 drop table table_11733 ;
 drop user test@localhost;
 
+#
+# Bug #35732: read-only blocks SELECT statements in InnoDB
+#
+GRANT CREATE, SELECT, DROP ON *.* TO test@localhost;
+CREATE TABLE t1(a INT) ENGINE=INNODB;
+INSERT INTO t1 VALUES (0), (1);
+SET GLOBAL read_only=1;
+
+connect(con1, localhost, test, , test);
+connection con1;
+SELECT * FROM t1;
+
+disconnect con1;
+
+connection default;
+SET GLOBAL read_only=0;
+DROP TABLE t1;
+DROP USER test@localhost;
+
+--echo echo End of 5.1 tests 
diff -Nrup a/sql/handler.cc b/sql/handler.cc
--- a/sql/handler.cc	2008-03-12 12:13:19 +04:00
+++ b/sql/handler.cc	2008-04-07 17:31:18 +05:00
@@ -954,16 +954,19 @@ int ha_prepare(THD *thd)
   A helper function to evaluate if two-phase commit is mandatory.
   As a side effect, propagates the read-only/read-write flags
   of the statement transaction to its enclosing normal transaction.
-
-  @retval TRUE   we must run a two-phase commit. Returned
-                 if we have at least two engines with read-write changes.
-  @retval FALSE  Don't need two-phase commit. Even if we have two
-                 transactional engines, we can run two independent
-                 commits if changes in one of the engines are read-only.
+  
+  If we have at least two engines with read-write changes we must
+  run a two-phase commit. Otherwise we can run several independent
+  commits as the only transactional engine has read-write changes
+  and others are read-only.
+
+  @retval   0   All engines are read-only.
+  @retval   1   We have the only engine with read-write changes.
+  @retval  >1   More than one engine have read-write changes.
 */
 
 static
-bool
+uint
 ha_check_and_coalesce_trx_read_only(THD *thd, Ha_trx_info *ha_list,
                                     bool all)
 {
@@ -1000,7 +1003,7 @@ ha_check_and_coalesce_trx_read_only(THD 
       break;
     }
   }
-  return rw_ha_count > 1;
+  return rw_ha_count;
 }
 
 
@@ -1063,7 +1066,7 @@ int ha_commit_trans(THD *thd, bool all)
 #ifdef USING_TRANSACTIONS
   if (ha_info)
   {
-    bool must_2pc;
+    uint rw_ha_count;
 
     if (is_real_trans && wait_if_global_read_lock(thd, 0, 0))
     {
@@ -1071,27 +1074,27 @@ int ha_commit_trans(THD *thd, bool all)
       DBUG_RETURN(1);
     }
 
-    if (   is_real_trans
-        && opt_readonly
-        && ! (thd->security_ctx->master_access & SUPER_ACL)
-        && ! thd->slave_thread
-       )
-    {
-      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
-      ha_rollback_trans(thd, all);
-      error= 1;
-      goto end;
-    }
-
     DBUG_EXECUTE_IF("crash_commit_before", abort(););
 
     /* Close all cursors that can not survive COMMIT */
     if (is_real_trans)                          /* not a statement commit */
       thd->stmt_map.close_transient_cursors();
 
-    must_2pc= ha_check_and_coalesce_trx_read_only(thd, ha_info, all);
+    rw_ha_count= ha_check_and_coalesce_trx_read_only(thd, ha_info, all);
+
+    if (is_real_trans &&
+        opt_readonly &&
+        (rw_ha_count > 0) && 
+        !(thd->security_ctx->master_access & SUPER_ACL) &&
+        !thd->slave_thread)
+    {
+      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
+      ha_rollback_trans(thd, all);
+      error= 1;
+      goto end;
+    }
 
-    if (!trans->no_2pc && must_2pc)
+    if (!trans->no_2pc && (rw_ha_count > 1))
     {
       for (; ha_info && !error; ha_info= ha_info->next())
       {