Fix for bug #418

Increased skygw_query_type_t to 16 bits, and corrected the way how those bit fields are checked.
Added tests for cases where autocommit is disabled and corrected old tests.

query_classifier/query_classifier.cc

@@ -355,9 +355,9 @@ return_here:
  * restrictive, for example, QUERY_TYPE_READ is smaller than QUERY_TYPE_WRITE.
  *
  */
-static u_int8_t set_query_type(
+static u_int16_t set_query_type(
-        u_int8_t* qtype,
+        u_int16_t* qtype,
-        u_int8_t  new_type)
+        u_int16_t  new_type)
 {
         *qtype = MAX(*qtype, new_type);
         return *qtype;
@@ -383,7 +383,7 @@ static skygw_query_type_t resolve_query_type(
         THD* thd)
 {
         skygw_query_type_t qtype = QUERY_TYPE_UNKNOWN;
-        u_int8_t           type = QUERY_TYPE_UNKNOWN;
+        u_int16_t           type = QUERY_TYPE_UNKNOWN;
         bool               is_set_autocommit = false;;
         LEX*  lex;
         Item* item;

query_classifier/query_classifier.h

@@ -29,17 +29,17 @@ EXTERN_C_BLOCK_BEGIN
  * is modified
  */
 typedef enum {
-    QUERY_TYPE_UNKNOWN          = 0x00,       /*< Couln't find out or parse error */
+    QUERY_TYPE_UNKNOWN          = 0x000,  /*< Initial value, can't be tested bitwisely */
-    QUERY_TYPE_LOCAL_READ       = 0x01,  /*< Read non-database data, execute in MaxScale */
+    QUERY_TYPE_LOCAL_READ       = 0x001,  /*< Read non-database data, execute in MaxScale */
-    QUERY_TYPE_READ             = 0x02,  /*< No updates */
+    QUERY_TYPE_READ             = 0x002,  /*< No updates */
-    QUERY_TYPE_WRITE            = 0x04,  /*< Master data will be  modified */
+    QUERY_TYPE_WRITE            = 0x004,  /*< Master data will be  modified */
-    QUERY_TYPE_SESSION_WRITE    = 0x08,  /*< Session data will be modified */
+    QUERY_TYPE_SESSION_WRITE    = 0x008,  /*< Session data will be modified */
-    QUERY_TYPE_GLOBAL_WRITE     = 0x10,  /*< Global system variable modification */
+    QUERY_TYPE_GLOBAL_WRITE     = 0x010,  /*< Global system variable modification */
-    QUERY_TYPE_BEGIN_TRX        = 0x20,  /*< BEGIN or START TRANSACTION */
+    QUERY_TYPE_BEGIN_TRX        = 0x020,  /*< BEGIN or START TRANSACTION */
-    QUERY_TYPE_ENABLE_AUTOCOMMIT  = 0x30,/*< SET autocommit=1 */
+    QUERY_TYPE_ENABLE_AUTOCOMMIT  = 0x040,/*< SET autocommit=1 */
-    QUERY_TYPE_DISABLE_AUTOCOMMIT = 0x40,/*< SET autocommit=0 */
+    QUERY_TYPE_DISABLE_AUTOCOMMIT = 0x080,/*< SET autocommit=0 */
-    QUERY_TYPE_ROLLBACK         = 0x50,  /*< ROLLBACK */
+    QUERY_TYPE_ROLLBACK         = 0x100,  /*< ROLLBACK */
-    QUERY_TYPE_COMMIT           = 0x60   /*< COMMIT */
+    QUERY_TYPE_COMMIT           = 0x200   /*< COMMIT */
 } skygw_query_type_t;
 
 #define QUERY_IS_TYPE(mask,type) ((mask & type) == type)

server/modules/routing/readwritesplit/readwritesplit.c

@@ -703,7 +703,8 @@ static int routeQuery(
          */
         if (autocommit_enabled &&
                 transaction_active &&
-                QUERY_IS_TYPE(qtype,(QUERY_TYPE_COMMIT|QUERY_TYPE_ROLLBACK)))
+                (QUERY_IS_TYPE(qtype,QUERY_TYPE_COMMIT) ||
+                QUERY_IS_TYPE(qtype,QUERY_TYPE_ROLLBACK)))
         {
                 transaction_active = false;
         } 
@@ -744,20 +745,7 @@ static int routeQuery(
                 
                 goto return_ret;
         }
-        else if (QUERY_IS_TYPE(
+        else if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ))
-                        qtype, 
-                        (QUERY_TYPE_BEGIN_TRX|QUERY_TYPE_WRITE|QUERY_TYPE_UNKNOWN)))
-        {
-                LOGIF(LT, (skygw_log_write(
-                        LOGFILE_TRACE,
-                        "Begin transaction, write or unspecified type, "
-                        "routing to Master.")));
-                ret = master_dcb->func.write(master_dcb, querybuf);
-                atomic_add(&inst->stats.n_master, 1);
-                
-                goto return_ret;
-        }
-        else
         {
                 LOGIF(LT, (skygw_log_write(
                         LOGFILE_TRACE,
@@ -769,6 +757,17 @@ static int routeQuery(
                 
                 goto return_ret;
         }       
+        else
+        {
+                LOGIF(LT, (skygw_log_write(
+                        LOGFILE_TRACE,
+                        "Begin transaction, write or unspecified type, "
+                        "routing to Master.")));
+                ret = master_dcb->func.write(master_dcb, querybuf);
+                atomic_add(&inst->stats.n_master, 1);
+                
+                goto return_ret;
+        }
 
 return_ret:
         if (plainsqlbuf != NULL)

server/modules/routing/readwritesplit/test/rwsplit.sh

@@ -33,8 +33,8 @@ fi
 TINPUT=test_transaction_routing3.sql
 TRETVAL=2
 a=`$RUNCMD < ./$TINPUT`
-if [ "$a" != "$TRETVAL" ]; then 
+if [ "$a" == "$TMASTER_ID" ]; then 
-        echo "$TINPUT FAILED, return value $a when $TRETVAL was expected">>$TLOG; 
+        echo "$TINPUT FAILED, return value $a when one of the slave IDs was expected">>$TLOG; 
 else 
         echo "$TINPUT PASSED">>$TLOG ; 
 fi
@@ -113,3 +113,22 @@ if [ "$a" == "$TRETVAL" ]; then
 else 
         echo "$TINPUT PASSED">>$TLOG ; 
 fi
+
+TINPUT=test_autocommit_disabled1.sql
+TRETVAL=1
+a=`$RUNCMD < ./$TINPUT`
+if [ "$a" != "$TRETVAL" ]; then
+        echo "$TINPUT FAILED, return value $a when $TRETVAL was expected">>$TLOG;
+else
+        echo "$TINPUT PASSED">>$TLOG ;
+fi
+
+TINPUT=test_autocommit_disabled2.sql
+TRETVAL=1
+a=`$RUNCMD < ./$TINPUT`
+if [ "$a" != "$TRETVAL" ]; then
+        echo "$TINPUT FAILED, return value $a when $TRETVAL was expected">>$TLOG;
+else
+        echo "$TINPUT PASSED">>$TLOG ;
+fi
+

server/modules/routing/readwritesplit/test/test_autocommit_disabled1.sql

@@ -0,0 +1,7 @@
+use test;
+drop table if exists t1;
+create table t1 (id integer);
+set autocommit=0;               -- open transaction
+insert into t1 values(1);       -- write to master
+select count(*) from t1;        -- read from master
+drop table t1;

server/modules/routing/readwritesplit/test/test_autocommit_disabled2.sql

@@ -0,0 +1,9 @@
+use test;
+drop table if exists t1;
+create table t1 (id integer);
+set autocommit=0;               -- open transaction
+begin;                         
+insert into t1 values(1);       -- write to master
+commit;
+select count(*) from t1;        -- read from master since autocommit is disabled
+drop table t1;

server/modules/routing/readwritesplit/test/test_implicit_commit1.sql

@@ -1,5 +1,5 @@
 DROP DATABASE If EXISTS FOO;
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 CREATE DATABASE FOO; -- implicit commit
 SELECT (@@server_id) INTO @a;

server/modules/routing/readwritesplit/test/test_implicit_commit2.sql

@@ -1,7 +1,7 @@
 USE test;
 DROP TABLE IF EXISTS T1;
 DROP EVENT IF EXISTS myevent;
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 CREATE TABLE T1 (id integer);
 CREATE EVENT myevent

server/modules/routing/readwritesplit/test/test_implicit_commit3.sql

@@ -1,6 +1,6 @@
 USE test;
 DROP TABLE IF EXISTS T1;
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 CREATE TABLE T1 (id integer); -- implicit commit
 SELECT (@@server_id) INTO @a;

server/modules/routing/readwritesplit/test/test_implicit_commit5.sql

@@ -1,6 +1,6 @@
 USE test;
 DROP PROCEDURE IF EXISTS simpleproc;
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 DELIMITER //
 CREATE PROCEDURE simpleproc (OUT param1 INT)

server/modules/routing/readwritesplit/test/test_implicit_commit6.sql

@@ -1,6 +1,6 @@
 USE test;
 DROP FUNCTION IF EXISTS hello;
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 CREATE FUNCTION hello (s CHAR(20))
 RETURNS CHAR(50) DETERMINISTIC

server/modules/routing/readwritesplit/test/test_implicit_commit7.sql

@@ -1,7 +1,7 @@
 USE test;
 DROP TABLE IF EXISTS T1;
 CREATE TABLE T1 (id integer); -- implicit commit
-SET autocommit=0;
+SET autocommit=1;
 BEGIN;
 CREATE INDEX foo_t1 on T1 (id); -- implicit commit
 SELECT (@@server_id) INTO @a;

server/modules/routing/readwritesplit/test/test_transaction_routing1.sql

@@ -1,19 +1,6 @@
-USE test; 
+use test; -- in both
-SET autocommit = 0; 
+drop table if exists t1;
-SET @a= -1; 
+create table t1 (id integer);
-SET @b= -2; 
+insert into t1 values(1); -- in master
-START TRANSACTION; 
+commit;
-CREATE TABLE IF NOT EXISTS myCity (a int, b char(20)); 
+select count(*) from t1; -- in slave
-INSERT INTO myCity VALUES (1, 'Milan'); 
-INSERT INTO myCity VALUES (2, 'London'); 
-COMMIT; 
-START TRANSACTION; 
-DELETE FROM myCity; 
-SET @a = (SELECT COUNT(*) FROM myCity); 
-ROLLBACK; 
-START TRANSACTION; 
-SET @b = (SELECT COUNT(*) FROM myCity); 
-START TRANSACTION; 
-DROP TABLE myCity; 
-SELECT (@a+@b) AS res; 
-COMMIT;

server/modules/routing/readwritesplit/test/test_transaction_routing3.sql

@@ -1,10 +1,7 @@
+-- Read from slave after implicit COMMIT
 USE test; 
-SET autocommit = 0; 
 START TRANSACTION; 
-CREATE TABLE IF NOT EXISTS myCity (a int, b char(20)); 
+CREATE TABLE IF NOT EXISTS T2 (id integer); 
-INSERT INTO myCity VALUES (1, 'Milan'); 
+INSERT INTO T2 VALUES (@@server_id);
-INSERT INTO myCity VALUES (2, 'London'); 
+SET AUTOCOMMIT=1;
-COMMIT;
+SELECT id from T2; -- read transaction's modifications from slave
-DELETE FROM myCity;
-SELECT COUNT(*) FROM myCity; -- read transaction's modifications from slave
-COMMIT;