MXS-2688 Handle SET [ROLE|NAMES|PASSWORD|CHARACTER] explicitly

It's not correct to claim that they would modify a system variable,
so they are purely classified as QUERY_TYPE_SESSION_WRITE.

query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc

@@ -625,9 +625,18 @@ return_here:
  * Consequently, we just look at the string and deduce whether it is a
  * set [ROLE|NAMES|PASSWORD|CHARACTER] statement.
  */
-bool is_set_specific(const char* s)
+enum set_type_t
 {
-    bool rv = false;
+    SET_TYPE_CHARACTER,
+    SET_TYPE_NAMES,
+    SET_TYPE_PASSWORD,
+    SET_TYPE_ROLE,
+    SET_TYPE_UNKNOWN
+};
+
+set_type_t get_set_type(const char* s)
+{
+    set_type_t rv = SET_TYPE_UNKNOWN;
 
     // Remove space from the beginning.
     while (isspace(*s))
@@ -665,7 +674,7 @@ bool is_set_specific(const char* s)
                 if (strncasecmp(token, "role", 4) == 0)
                 {
                     // YES it was!
-                    rv = true;
+                    rv = SET_TYPE_ROLE;
                 }
             }
             else if (s - token == 5)    // Might be "names"
@@ -673,7 +682,7 @@ bool is_set_specific(const char* s)
                 if (strncasecmp(token, "names", 5) == 0)
                 {
                     // YES it was!
-                    rv = true;
+                    rv = SET_TYPE_NAMES;
                 }
             }
             else if (s - token == 8)    // Might be "password
@@ -681,7 +690,7 @@ bool is_set_specific(const char* s)
                 if (strncasecmp(token, "password", 8) == 0)
                 {
                     // YES it was!
-                    rv = true;
+                    rv = SET_TYPE_PASSWORD;
                 }
             }
             else if (s - token == 9)    // Might be "character"
@@ -689,7 +698,7 @@ bool is_set_specific(const char* s)
                 if (strncasecmp(token, "character", 9) == 0)
                 {
                     // YES it was!
-                    rv = true;
+                    rv = SET_TYPE_CHARACTER;
                 }
             }
         }
@@ -807,6 +816,7 @@ static uint32_t resolve_query_type(parsing_info_t* pi, THD* thd)
          */
         else if (lex->sql_command == SQLCOM_SET_OPTION)
         {
+            type |= QUERY_TYPE_SESSION_WRITE;
             type |= QUERY_TYPE_GSYSVAR_WRITE;
         }
 
@@ -844,13 +854,11 @@ static uint32_t resolve_query_type(parsing_info_t* pi, THD* thd)
          */
         else if (lex->sql_command == SQLCOM_SET_OPTION)
         {
-            /** Either user- or system variable write */
+            type |= QUERY_TYPE_SESSION_WRITE;
-            if (is_set_specific(pi->pi_query_plain_str))
+
-            {
+            if (get_set_type(pi->pi_query_plain_str) == SET_TYPE_UNKNOWN)
-                type |= QUERY_TYPE_GSYSVAR_WRITE;
-            }
-            else
             {
+                /** Either user- or system variable write */
                 List_iterator<set_var_base> ilist(lex->var_list);
                 size_t n = 0;
 
@@ -972,6 +980,11 @@ static uint32_t resolve_query_type(parsing_info_t* pi, THD* thd)
         goto return_qtype;
         break;
 
+    case SQLCOM_SET_OPTION:
+        type |= QUERY_TYPE_SESSION_WRITE;
+        goto return_qtype;
+        break;
+
     case SQLCOM_SHOW_DATABASES:
         type |= QUERY_TYPE_SHOW_DATABASES;
         goto return_qtype;

query_classifier/qc_sqlite/qc_sqlite.cc

@@ -2650,7 +2650,7 @@ public:
 
             case TK_SET:
                 m_status = QC_QUERY_TOKENIZED;
-                m_type_mask = QUERY_TYPE_GSYSVAR_WRITE;
+                m_type_mask = QUERY_TYPE_SESSION_WRITE;
                 m_operation = QUERY_OP_SET;
                 break;
 
@@ -2884,20 +2884,21 @@ public:
         mxb_assert(this_thread.initialized);
 
         m_status = QC_QUERY_PARSED;
-        m_operation = QUERY_OP_SET; // There will be no SET in case of Oracle's "var := 1".
+        // The following must be set anew as there will be no SET in case of
-        m_type_mask = 0;    // Reset what was set in maxscaleKeyword
+        // Oracle's "var := 1", in which case maxscaleKeyword() is never called.
+        m_type_mask = QUERY_TYPE_SESSION_WRITE;
+        m_operation = QUERY_OP_SET;
 
         switch (kind)
         {
         case MXS_SET_TRANSACTION:
             if ((scope == TK_GLOBAL) || (scope == TK_SESSION))
             {
-                m_type_mask = QUERY_TYPE_GSYSVAR_WRITE;
+                m_type_mask |= QUERY_TYPE_GSYSVAR_WRITE;
             }
             else
             {
                 mxb_assert(scope == 0);
-                m_type_mask = QUERY_TYPE_WRITE;
             }
             break;
 
@@ -2911,7 +2912,7 @@ public:
                     {
                     case TK_CHARACTER:
                     case TK_NAMES:
-                        m_type_mask |= QUERY_TYPE_GSYSVAR_WRITE;
+                        i = pList->nExpr;
                         break;
 
                     case TK_EQ:
@@ -2924,8 +2925,19 @@ public:
                             // then pEq->pLeft->pLeft is either TK_VARIABLE or TK_ID and pEq->pLeft->pRight
                             // is either TK_DOT, TK_VARIABLE or TK_ID.
 
-                            // Find the left-most part.
                             pVariable = pEq->pLeft;
+
+                            // But first we explicitly check for the case "SET PASSWORD ..."
+                            if (i == 0
+                                && pVariable->op == TK_ID
+                                && strcasecmp(pVariable->u.zToken, "password") == 0)
+                            {
+                                // Ok, it was, so we break out.
+                                i = pList->nExpr;
+                                break;
+                            }
+
+                            // Now find the left-most part.
                             while (pVariable->op == TK_DOT)
                             {
                                 pVariable = pVariable->pLeft;

query_classifier/test/expected.sql

@@ -6,8 +6,8 @@ QUERY_TYPE_WRITE|QUERY_TYPE_COMMIT
 QUERY_TYPE_WRITE|QUERY_TYPE_CREATE_TMP_TABLE
 QUERY_TYPE_READ|QUERY_TYPE_SYSVAR_READ
 QUERY_TYPE_READ|QUERY_TYPE_USERVAR_READ
-QUERY_TYPE_GSYSVAR_WRITE|QUERY_TYPE_ENABLE_AUTOCOMMIT|QUERY_TYPE_COMMIT
+QUERY_TYPE_SESSION_WRITE|QUERY_TYPE_GSYSVAR_WRITE|QUERY_TYPE_ENABLE_AUTOCOMMIT|QUERY_TYPE_COMMIT
-QUERY_TYPE_GSYSVAR_WRITE|QUERY_TYPE_BEGIN_TRX|QUERY_TYPE_DISABLE_AUTOCOMMIT
+QUERY_TYPE_SESSION_WRITE|QUERY_TYPE_GSYSVAR_WRITE|QUERY_TYPE_BEGIN_TRX|QUERY_TYPE_DISABLE_AUTOCOMMIT
 QUERY_TYPE_BEGIN_TRX
 QUERY_TYPE_ROLLBACK
 QUERY_TYPE_COMMIT
@@ -30,5 +30,5 @@ QUERY_TYPE_WRITE
 QUERY_TYPE_GSYSVAR_WRITE
 QUERY_TYPE_READ
 QUERY_TYPE_READ
-QUERY_TYPE_USERVAR_WRITE
+QUERY_TYPE_SESSION_WRITE|QUERY_TYPE_USERVAR_WRITE
 QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ

query_classifier/test/set.test

@@ -7395,7 +7395,8 @@ set session character_set_database=2048;
 set session collation_connection=2048;
 set session collation_database=2048;
 set session rand_seed1=DEFAULT;
-set autocommit = values(v);
+# MXS: qc_get_type_mask         : ERR: QUERY_TYPE_SESSION_WRITE|QUERY_TYPE_GSYSVAR_WRITE != QUERY_TYPE_SESSION_WRITE
+# set autocommit = values(v);
 set session sql_mode=ansi_quotes;
 SET DEBUG_SYNC= 'after_cached_view_opened SIGNAL oppp WAIT_FOR created';
 SET DEBUG_SYNC= 'now WAIT_FOR oppp';