From 75315152597c14318f41c36a3ebef93570a02bed Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 13 Sep 2019 15:32:57 +0300 Subject: [PATCH] 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; +} + +}