From 45bda0f72e5d542d109c1d87b0a72935351c1f24 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 21 Jun 2018 10:39:53 +0300 Subject: [PATCH 1/4] MXS-1936 Make qc_mysqlembedded compatible with qc_sqlite qc_mysqlembedded must also be updated to handle the new type QUERY_TYPE_DEALLOC_PREPARE. Some adjustements were also needed elsewhere. --- query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc | 2 +- query_classifier/test/classify.c | 4 ++++ server/core/query_classifier.cc | 9 +++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 07c813cf2..af7d5dbef 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -922,7 +922,7 @@ static uint32_t resolve_query_type(parsing_info_t *pi, THD* thd) break; case SQLCOM_DEALLOCATE_PREPARE: - type |= QUERY_TYPE_WRITE; + type |= QUERY_TYPE_DEALLOC_PREPARE; break; case SQLCOM_SELECT: diff --git a/query_classifier/test/classify.c b/query_classifier/test/classify.c index 7e977803a..21f1b2c86 100644 --- a/query_classifier/test/classify.c +++ b/query_classifier/test/classify.c @@ -140,6 +140,10 @@ char* get_types_as_string(uint32_t types) { s = append(s, "QUERY_TYPE_SHOW_TABLES", &len); } + if (types & QUERY_TYPE_DEALLOC_PREPARE) + { + s = append(s, "QUERY_TYPE_DEALLOC_PREPARE", &len); + } if (!s) { diff --git a/server/core/query_classifier.cc b/server/core/query_classifier.cc index 787abd21b..a493a4334 100644 --- a/server/core/query_classifier.cc +++ b/server/core/query_classifier.cc @@ -604,6 +604,14 @@ struct type_name_info type_to_type_name_info(qc_query_type_t type) } break; + case QUERY_TYPE_DEALLOC_PREPARE: + { + static const char name[] = "QUERY_TYPE_DEALLOC_PREPARE"; + info.name = name; + info.name_len = sizeof(name) - 1; + } + break; + default: { static const char name[] = "UNKNOWN_QUERY_TYPE"; @@ -650,6 +658,7 @@ static const qc_query_type_t QUERY_TYPES[] = QUERY_TYPE_READ_TMP_TABLE, QUERY_TYPE_SHOW_DATABASES, QUERY_TYPE_SHOW_TABLES, + QUERY_TYPE_DEALLOC_PREPARE, }; static const int N_QUERY_TYPES = sizeof(QUERY_TYPES) / sizeof(QUERY_TYPES[0]); From 457d74f6b48340e3adad305a8cbf0cbf3b8b6323 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 21 Jun 2018 10:46:44 +0300 Subject: [PATCH 2/4] MXS-1935 Add test that exposes problem "PREPARE a FROM @var" is not classified. --- query_classifier/test/maxscale.test | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/query_classifier/test/maxscale.test b/query_classifier/test/maxscale.test index 8c34ceb29..d18717b5f 100644 --- a/query_classifier/test/maxscale.test +++ b/query_classifier/test/maxscale.test @@ -108,4 +108,7 @@ CALL p1((SELECT f1()), ?); SELECT PREVIOUS VALUE FOR SEQ; # MXS-1874 -SET STATEMENT max_statement_time=30 FOR SELECT seq FROM seq_0_to_100000; \ No newline at end of file +SET STATEMENT max_statement_time=30 FOR SELECT seq FROM seq_0_to_100000; + +# MXS-1935 +PREPARE a FROM @sql; From 254084fc5ef32726ec1746d4347c9d00a7d9c3e1 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 21 Jun 2018 10:48:02 +0300 Subject: [PATCH 3/4] MXS-1935 Accept "PREPARE name FROM @var" --- query_classifier/qc_sqlite/qc_sqlite.cc | 80 ++++--------------------- query_classifier/test/compare.cc | 15 ++--- 2 files changed, 21 insertions(+), 74 deletions(-) diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 4ec882119..68bc0e612 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -781,36 +781,6 @@ public: return type_mask; } - /** - * Returns some string from an expression. - * - * @param pExpr An expression. - * - * @return Some string referred to in pExpr. - */ - static const char* find_one_string(Expr* pExpr) - { - const char* z = NULL; - - if (pExpr->op == TK_STRING) - { - ss_dassert(pExpr->u.zToken); - z = pExpr->u.zToken; - } - - if (!z && pExpr->pLeft) - { - z = find_one_string(pExpr->pLeft); - } - - if (!z && pExpr->pRight) - { - z = find_one_string(pExpr->pRight); - } - - return z; - } - static int string_to_truth(const char* s) { int truth = -1; @@ -2660,33 +2630,16 @@ public: { ss_dassert(this_thread.initialized); - // If the mode is MODE_ORACLE then if expression contains simply a string - // we can conclude that the statement has been fully parsed, because it will - // be sensible to parse the preparable statement. Otherwise we mark the - // statement as having been partially parsed, since the preparable statement - // will not contain the full statement. - if (m_sql_mode == QC_SQL_MODE_ORACLE) + switch (pStmt->op) { - if (pStmt->op == TK_STRING) - { - m_status = QC_QUERY_PARSED; - } - else - { - m_status = QC_QUERY_PARTIALLY_PARSED; - } - } - else - { - // If the mode is not MODE_ORACLE, then only a string is acceptable. - if (pStmt->op == TK_STRING) - { - m_status = QC_QUERY_PARSED; - } - else - { - m_status = QC_QUERY_INVALID; - } + case TK_STRING: + case TK_VARIABLE: + m_status = QC_QUERY_PARSED; + break; + + default: + m_status = QC_QUERY_PARTIALLY_PARSED; + break; } m_type_mask = QUERY_TYPE_PREPARE_NAMED_STMT; @@ -2702,14 +2655,11 @@ public: m_zPrepare_name[pName->n] = 0; } - // If the expression just contains a string, then zStmt will - // be that string. Otherwise it will be _some_ string from the - // expression. In the latter case we've already marked the result - // to have been partially parsed. - const char* zStmt = find_one_string(pStmt); - - if (zStmt) + if (pStmt->op == TK_STRING) { + const char* zStmt = pStmt->u.zToken; + ss_dassert(zStmt); + size_t preparable_stmt_len = zStmt ? strlen(zStmt) : 0; size_t payload_len = 1 + preparable_stmt_len; size_t packet_len = MYSQL_HEADER_LEN + payload_len; @@ -2731,10 +2681,6 @@ public: memcpy(ptr, zStmt, preparable_stmt_len); } } - else - { - m_status = QC_QUERY_INVALID; - } } else { diff --git a/query_classifier/test/compare.cc b/query_classifier/test/compare.cc index 07f53bf2b..dafb077c9 100644 --- a/query_classifier/test/compare.cc +++ b/query_classifier/test/compare.cc @@ -1302,19 +1302,20 @@ bool compare(QUERY_CLASSIFIER* pClassifier1, GWBUF* pBuf1, { GWBUF* pPreparable1; pClassifier1->qc_get_preparable_stmt(pBuf1, &pPreparable1); - ss_dassert(pPreparable1); GWBUF* pPreparable2; pClassifier2->qc_get_preparable_stmt(pBuf2, &pPreparable2); - ss_dassert(pPreparable2); - string indent = global.indent; - global.indent += string(4, ' '); + if (pPreparable1 && pPreparable2) + { + string indent = global.indent; + global.indent += string(4, ' '); - success = compare(pClassifier1, pPreparable1, - pClassifier2, pPreparable2); + success = compare(pClassifier1, pPreparable1, + pClassifier2, pPreparable2); - global.indent = indent; + global.indent = indent; + } } return success; From 0f61c4b6a4a3d6df7d13f60181fc0b32ce465033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 21 Jun 2018 13:29:02 +0300 Subject: [PATCH 4/4] MXS-872: Also check that mysql.user.default_role exists The column is used so it should be checked that it exists. Also altered the SQL to use statements that do not return resultsets. --- server/modules/authenticator/MySQLAuth/dbusers.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/modules/authenticator/MySQLAuth/dbusers.c b/server/modules/authenticator/MySQLAuth/dbusers.c index 498a9716f..106686773 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.c +++ b/server/modules/authenticator/MySQLAuth/dbusers.c @@ -802,17 +802,18 @@ static bool roles_are_available(MYSQL* conn, SERVICE* service, SERVER* server) { static bool log_missing_privs = true; - if (mxs_mysql_query(conn, "SELECT 1 FROM mysql.roles_mapping LIMIT 1") == 0) + if (mxs_mysql_query(conn, "SET @roles_are_available=(SELECT 1 FROM mysql.roles_mapping LIMIT 1)") == 0 && + mxs_mysql_query(conn, "SET @roles_are_available=(SELECT default_role FROM mysql.user LIMIT 1)") == 0) { - mysql_free_result(mysql_store_result(conn)); rval = true; } else if (log_missing_privs) { log_missing_privs = false; - MXS_WARNING("The user for service '%s' is missing the SELECT grant on " - "`mysql.roles_mapping`. Use of default roles is disabled " - "until the missing privileges are added.", service->name); + MXS_WARNING("The user for service '%s' might be missing the SELECT grant on " + "`mysql.roles_mapping` or `mysql.user`. Use of default roles is disabled " + "until the missing privileges are added. Error was: %s", + service->name, mysql_error(conn)); } }