From c079d1312ea34fd62df3849608c6a35510010b8d Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 12 Sep 2019 12:38:57 +0300 Subject: [PATCH 1/7] MXS-2674 Recognize timediff as builtin function --- query_classifier/qc_sqlite/builtin_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query_classifier/qc_sqlite/builtin_functions.c b/query_classifier/qc_sqlite/builtin_functions.c index cec511c25..9caf4479a 100644 --- a/query_classifier/qc_sqlite/builtin_functions.c +++ b/query_classifier/qc_sqlite/builtin_functions.c @@ -87,7 +87,7 @@ static const char* BUILTIN_FUNCTIONS[] = "subtime", "sysdate", "time", - "timediff" + "timediff", "timestamp", "timestampadd", "timestampdiff", From 811a2b1df6ce48b4572e31ec54918873c62a6fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 12 Sep 2019 11:31:57 +0300 Subject: [PATCH 2/7] Fix dead link --- Documentation/Documentation-Contents.md | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index bf483b12d..991ac7561 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -48,7 +48,6 @@ These tutorials are for specific use cases and module combinations. - [Replication Proxy with the Binlog Router Tutorial](Tutorials/Replication-Proxy-Binlog-Router-Tutorial.md) - [Simple Schema Sharding Tutorial](Tutorials/Simple-Sharding-Tutorial.md) - [Filter Tutorial](Tutorials/Filter-Tutorial.md) - - [MySQL Cluster Setup](Tutorials/MySQL-Cluster-Setup.md) - [RabbitMQ and Tee Filter Data Archiving Tutorial](Tutorials/RabbitMQ-And-Tee-Archiving.md) - [RabbitMQ Setup and MariaDB MaxScale Integration Tutorial](Tutorials/RabbitMQ-Setup-And-MaxScale-Integration.md) - [Clustrix Monitor Tutorial](Tutorials/Configuring-Clustrix-Monitor.md) From 31029eaec884b0b5bfdb7cda84265b8d2c1dbeba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 13 Sep 2019 14:12:57 +0300 Subject: [PATCH 3/7] MXS-2675: Fix server creation with TLS via REST API The TLS parameters were defined but the main parameter that enables it wasn't automatically added. As the REST API documentation states that this parameter does not need to be defined, the runtime configuration must add it. --- include/maxscale/config.hh | 14 ++++++++++++++ server/core/config_runtime.cc | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/include/maxscale/config.hh b/include/maxscale/config.hh index 917ff0f99..342572025 100644 --- a/include/maxscale/config.hh +++ b/include/maxscale/config.hh @@ -434,6 +434,20 @@ public: */ bool contains(const std::string& key) const; + /** + * Check if any of the given keys are defined + * + * @param keys The keys to check + * + * @return True if at least one of the keys is defined + */ + bool contains_any(const std::initializer_list& keys) const + { + return std::any_of(keys.begin(), keys.end(), [this](const std::string& a) { + return contains(a); + }); + } + /** * Set a key-value combination. If the key doesn't exist, it is added. The function is static * to handle the special case of params being empty. This is needed until the config management diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 52934d148..859447a31 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -1893,6 +1893,11 @@ bool runtime_create_server_from_json(json_t* json) { params.set_multiple(extract_parameters_from_json(json)); + if (params.contains_any({CN_SSL_KEY, CN_SSL_CERT, CN_SSL_CA_CERT})) + { + params.set(CN_SSL, "true"); + } + if (Server* server = Server::server_alloc(name, params)) { if (link_server_to_objects(server, relations) && server->serialize()) From 9969f21414c6e579de23fefe30df49ab89696ca3 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 13 Sep 2019 15:06:42 +0300 Subject: [PATCH 4/7] MXS-2674 Add test that reveals bug --- query_classifier/test/classify.c | 4 ++++ query_classifier/test/expected.sql | 4 ++++ query_classifier/test/input.sql | 8 ++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/query_classifier/test/classify.c b/query_classifier/test/classify.c index 10f612c45..78d616845 100644 --- a/query_classifier/test/classify.c +++ b/query_classifier/test/classify.c @@ -76,6 +76,10 @@ char* get_types_as_string(uint32_t types) { s = append(s, "QUERY_TYPE_SESSION_WRITE", &len); } + if (types & QUERY_TYPE_USERVAR_WRITE) + { + s = append(s, "QUERY_TYPE_USERVAR_WRITE", &len); + } if (types & QUERY_TYPE_USERVAR_READ) { s = append(s, "QUERY_TYPE_USERVAR_READ", &len); diff --git a/query_classifier/test/expected.sql b/query_classifier/test/expected.sql index 55a350bd2..d971d5969 100644 --- a/query_classifier/test/expected.sql +++ b/query_classifier/test/expected.sql @@ -28,3 +28,7 @@ QUERY_TYPE_WRITE QUERY_TYPE_WRITE QUERY_TYPE_WRITE QUERY_TYPE_GSYSVAR_WRITE +QUERY_TYPE_READ +QUERY_TYPE_READ +QUERY_TYPE_USERVAR_WRITE +QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ diff --git a/query_classifier/test/input.sql b/query_classifier/test/input.sql index 3575bcb40..43fc0af57 100644 --- a/query_classifier/test/input.sql +++ b/query_classifier/test/input.sql @@ -25,6 +25,10 @@ SELECT IS_USED_LOCK('lock1'); SELECT RELEASE_LOCK('lock1'); deallocate prepare select_stmt; SELECT a FROM tbl FOR UPDATE; -SELECT a INTO OUTFILE 'out.txt' -SELECT a INTO DUMPFILE 'dump.txt' +SELECT a INTO OUTFILE 'out.txt'; +SELECT a INTO DUMPFILE 'dump.txt'; SELECT a INTO @var; +select timediff(cast('2004-12-30 12:00:00' as time), '12:00:00'); +(select 1 as a from t1) union all (select 1 from dual) limit 1; +SET @saved_cs_client= @@character_set_client; +SELECT 1 AS c1 FROM t1 ORDER BY ( SELECT 1 AS c2 FROM t1 GROUP BY GREATEST(LAST_INSERT_ID(), t1.a) ORDER BY GREATEST(LAST_INSERT_ID(), t1.a) LIMIT 1); From 75315152597c14318f41c36a3ebef93570a02bed Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 13 Sep 2019 15:32:57 +0300 Subject: [PATCH 5/7] MXS-2674 Fix query classification With these changes SET @saved_cs_client= @@character_set_client; will be classified as QUERY_TYPE_USERVAR_WRITE and SELECT 1 AS c1 FROM t1 ORDER BY ( SELECT 1 AS c2 FROM t1 GROUP BY GREATEST(LAST_INSERT_ID(), t1.a) ORDER BY GREATEST(LAST_INSERT_ID(), t1.a) LIMIT 1); will be classified as QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ --- .../qc_mysqlembedded/qc_mysqlembedded.cc | 32 +++++++++++- query_classifier/qc_sqlite/qc_sqlite.cc | 25 +++++++++- .../qc_sqlite/sqlite-src-3110100/src/parse.y | 49 ++++++++++++++----- 3 files changed, 91 insertions(+), 15 deletions(-) diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 477a9e3b0..2a3f8b1b9 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -1665,13 +1665,26 @@ int32_t qc_mysql_query_has_clause(GWBUF* buf, int32_t* has_clause) if (lex) { - if (!lex->describe && !is_show_command(lex->sql_command)) + if (!lex->describe + && !is_show_command(lex->sql_command) + && (lex->sql_command != SQLCOM_ALTER_PROCEDURE) + && (lex->sql_command != SQLCOM_ALTER_TABLE) + && (lex->sql_command != SQLCOM_CALL) + && (lex->sql_command != SQLCOM_CREATE_PROCEDURE) + && (lex->sql_command != SQLCOM_CREATE_TABLE) + && (lex->sql_command != SQLCOM_DROP_FUNCTION) + && (lex->sql_command != SQLCOM_DROP_PROCEDURE) + && (lex->sql_command != SQLCOM_DROP_TABLE) + && (lex->sql_command != SQLCOM_DROP_VIEW) + && (lex->sql_command != SQLCOM_FLUSH) + && (lex->sql_command != SQLCOM_ROLLBACK) + ) { SELECT_LEX* current = lex->all_selects_list; while (current && !*has_clause) { - if (current->where || current->having) + if (current->where || current->having || current->select_limit) { *has_clause = true; } @@ -2741,6 +2754,7 @@ typedef enum collect_source COLLECT_WHERE, COLLECT_HAVING, COLLECT_GROUP_BY, + COLLECT_ORDER_BY } collect_source_t; static void update_field_infos(parsing_info_t* pi, @@ -2781,6 +2795,7 @@ static bool should_function_be_ignored(parsing_info_t* pi, const char* func_name || (strcasecmp(func_name, "get_user_var") == 0) || (strcasecmp(func_name, "get_system_var") == 0) || (strcasecmp(func_name, "not") == 0) + || (strcasecmp(func_name, "collate") == 0) || (strcasecmp(func_name, "set_user_var") == 0) || (strcasecmp(func_name, "set_system_var") == 0)) { @@ -3134,6 +3149,19 @@ static void update_field_infos(parsing_info_t* pi, } } + if (select->order_list.first) + { + ORDER* order = select->order_list.first; + while (order) + { + Item* item = *order->item; + + update_field_infos(pi, select, COLLECT_ORDER_BY, item, &select->item_list); + + order = order->next; + } + } + if (select->where) { update_field_infos(pi, diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 960e29c60..670628234 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -1253,6 +1253,18 @@ public: const ExprList* pExclude, compound_approach_t compound_approach = ANALYZE_COMPOUND_SELECTS) { + if (pSelect->pLimit) + { + // In case there is an ORDER BY statement without a LIMIT, which is + // not accepted by sqlite, a pseudo LIMIT with the value of -1 is injected. + // We need to detect that so as not to incorrectly claim that there is + // a clause. See maxscale_create_pseudo_limit() in parse.y. + if (pSelect->pLimit->op != TK_INTEGER || pSelect->pLimit->u.iValue != -1) + { + m_has_clause = true; + } + } + if (pSelect->pSrc) { const SrcList* pSrc = pSelect->pSrc; @@ -1316,6 +1328,14 @@ public: #endif } + if (pSelect->pOrderBy) + { + update_field_infos_from_exprlist(&aliases, + context, + pSelect->pOrderBy, + pSelect->pEList); + } + if (pSelect->pWith) { update_field_infos_from_with(&aliases, context, pSelect->pWith); @@ -1860,6 +1880,9 @@ public: QcAliases aliases; uint32_t context = 0; update_field_infos_from_select(aliases, context, pSelect, NULL); + + // Non-sensical to claim that a "CREATE ... SELECT ... WHERE ..." statement has a clause. + m_has_clause = false; } else if (pOldTable) { @@ -2030,7 +2053,7 @@ public: m_type_mask = QUERY_TYPE_WRITE; m_operation = QUERY_OP_UPDATE; update_names_from_srclist(&aliases, pTabList); - m_has_clause = (pWhere ? true : false); + m_has_clause = ((pWhere && pWhere->op != TK_IN) ? true : false); if (pChanges) { diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y index 72690b5d1..67f544ad5 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -134,6 +134,8 @@ extern void maxscaleUse(Parse*, Token*); extern void maxscale_update_function_info(const char* name, const Expr* pExpr); extern void maxscale_set_type_mask(unsigned int type_mask); +static Expr* maxscale_create_pseudo_limit(Parse* pParse, Token* pToken, ExprSpan* pLimit); + // Exposed utility functions void exposed_sqlite3ExprDelete(sqlite3 *db, Expr *pExpr) { @@ -1516,12 +1518,14 @@ cmd ::= with(C) DELETE FROM fullname(X) indexed_opt(I) where_opt(W) sqlite3WithPush(pParse, C, 1); sqlite3SrcListIndexedBy(pParse, X, &I); #ifdef MAXSCALE - // We are not interested in the order by or limit information. - // Thus we simply delete it. sqlite also has some limitations, which - // will not bother us if we hide the information from it. - sqlite3ExprListDelete(pParse->db, O); - sqlite3ExprDelete(pParse->db, L.pLimit); - sqlite3ExprDelete(pParse->db, L.pOffset); + Token token; + ExprSpan limit; + if (O && !L.pLimit) { + L.pLimit = maxscale_create_pseudo_limit(pParse, &token, &limit); + sqlite3ExprDelete(pParse->db, L.pOffset); + L.pOffset = 0; + } + W = sqlite3LimitWhere(pParse, X, W, O, L.pLimit, L.pOffset, "DELETE"); mxs_sqlite3DeleteFrom(pParse,X,W,0); #else W = sqlite3LimitWhere(pParse, X, W, O, L.pLimit, L.pOffset, "DELETE"); @@ -1678,12 +1682,15 @@ cmd ::= with(C) UPDATE orconf(R) fullname(X) indexed_opt(I) SET setlist(Y) sqlite3SrcListIndexedBy(pParse, X, &I); sqlite3ExprListCheckLength(pParse,Y,"set list"); #ifdef MAXSCALE - // We are not interested in the order by or limit information. - // Thus we simply delete it. sqlite also has some limitations, which - // will not bother us if we hide the information from it. - sqlite3ExprListDelete(pParse->db, O); - sqlite3ExprDelete(pParse->db, L.pLimit); - sqlite3ExprDelete(pParse->db, L.pOffset); + Token token; + ExprSpan limit; + if (O && !L.pLimit) { + L.pLimit = maxscale_create_pseudo_limit(pParse, &token, &limit); + sqlite3ExprDelete(pParse->db, L.pOffset); + L.pOffset = 0; + } + + W = sqlite3LimitWhere(pParse, X, W, O, L.pLimit, L.pOffset, "UPDATE"); mxs_sqlite3Update(pParse,X,Y,W,R); #else W = sqlite3LimitWhere(pParse, X, W, O, L.pLimit, L.pOffset, "UPDATE"); @@ -3507,3 +3514,21 @@ cmd ::= DECLARE. { } %endif + +%include { + +static Expr* maxscale_create_pseudo_limit(Parse* pParse, Token* pToken, ExprSpan* pLimit) +{ + // sqlite3 does not accept a ORDER BY without LIMIT, but MariaDB + // does. Thus, to make sqlite3LimitWhere return non-NULL we need + // to inject a LIMIT if there is none. We use a value of -1 to + // tell update_field_infos_from_select() that this LIMIT should + // not be counted as a limiting clause. + static char one[] = "-1"; + pToken->z = one; + pToken->n = 1; + spanExpr(pLimit, pParse, TK_INTEGER, pToken); + return pLimit->pExpr; +} + +} From 01ab0c873698c60254555bfcd936c50433edc4a0 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Wed, 12 Jun 2019 13:20:19 +0300 Subject: [PATCH 6/7] MXS-2553 Allow parenthesis around SELECT With this change, a parenthesized top-level SELECT, such as "(SELECT f FROM t)" will be fully parsed. Before this change, the statement was classified as invalid and would thus have been sent to the master. With this change also statements like (SELECT f FROM t1) UNION (SELECT f FROM t2) will be correctly classified, although only partially parsed. --- .../qc_sqlite/sqlite-src-3110100/src/parse.y | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y index 67f544ad5..7ccf8af9b 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -960,7 +960,18 @@ cmd ::= DROP VIEW ifexists(E) fullname(X). { //////////////////////// The SELECT statement ///////////////////////////////// // +%ifdef MAXSCALE +%type pselect {Select*} +%destructor pselect {sqlite3SelectDelete(pParse->db, $$);} + +pselect(A) ::= LP pselect(X) RP. { A = X; } +pselect(A) ::= select(X). { A = X; } + +cmd ::= pselect(X). { +%endif +%ifndef MAXSCALE cmd ::= select(X). { +%endif SelectDest dest = {SRT_Output, 0, 0, 0, 0, 0}; #ifdef MAXSCALE mxs_sqlite3Select(pParse, X, &dest); From 60c33b149c95df1c11ba7300c49af9efce6c1a03 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 16 Sep 2019 09:32:47 +0300 Subject: [PATCH 7/7] MXS-2674 Prevent read of unitialized variable --- .../qc_mysqlembedded/qc_mysqlembedded.cc | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 2a3f8b1b9..019dc699f 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -1665,26 +1665,30 @@ int32_t qc_mysql_query_has_clause(GWBUF* buf, int32_t* has_clause) if (lex) { + int cmd = lex->sql_command; + if (!lex->describe - && !is_show_command(lex->sql_command) - && (lex->sql_command != SQLCOM_ALTER_PROCEDURE) - && (lex->sql_command != SQLCOM_ALTER_TABLE) - && (lex->sql_command != SQLCOM_CALL) - && (lex->sql_command != SQLCOM_CREATE_PROCEDURE) - && (lex->sql_command != SQLCOM_CREATE_TABLE) - && (lex->sql_command != SQLCOM_DROP_FUNCTION) - && (lex->sql_command != SQLCOM_DROP_PROCEDURE) - && (lex->sql_command != SQLCOM_DROP_TABLE) - && (lex->sql_command != SQLCOM_DROP_VIEW) - && (lex->sql_command != SQLCOM_FLUSH) - && (lex->sql_command != SQLCOM_ROLLBACK) + && !is_show_command(cmd) + && (cmd != SQLCOM_ALTER_PROCEDURE) + && (cmd != SQLCOM_ALTER_TABLE) + && (cmd != SQLCOM_CALL) + && (cmd != SQLCOM_CREATE_PROCEDURE) + && (cmd != SQLCOM_CREATE_TABLE) + && (cmd != SQLCOM_DROP_FUNCTION) + && (cmd != SQLCOM_DROP_PROCEDURE) + && (cmd != SQLCOM_DROP_TABLE) + && (cmd != SQLCOM_DROP_VIEW) + && (cmd != SQLCOM_FLUSH) + && (cmd != SQLCOM_ROLLBACK) ) { SELECT_LEX* current = lex->all_selects_list; while (current && !*has_clause) { - if (current->where || current->having || current->select_limit) + if (current->where || current->having || + ((cmd == SQLCOM_SELECT || cmd == SQLCOM_DELETE || cmd == SQLCOM_UPDATE) + && current->select_limit)) { *has_clause = true; }