From cddcc6d7d56f282c4da2d7013e4dd77989331e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sat, 2 Jun 2018 22:24:51 +0300 Subject: [PATCH] MXS-1896: Distinct LOAD DATA LOCAL INFILE from LOAD DATA INFILE The two operations return different types of results and need to be treated differently in order for them to be handled correctly in 2.2. This fixes the unexpected internal state errors that happened in all 2.2 versions due to a wrong assumption made by readwritesplit. This fix is not necessary for newer versions as the LOAD DATA LOCAL INFILE processing is done with a simpler, and more robust, method. --- include/maxscale/query_classifier.h | 1 + maxscale-system-test/CMakeLists.txt | 4 +++ .../mxs1896_load_data_infile.cpp | 32 +++++++++++++++++++ .../qc_mysqlembedded/qc_mysqlembedded.cc | 2 +- query_classifier/qc_sqlite/qc_sqlite.cc | 10 +++--- .../qc_sqlite/sqlite-src-3110100/src/parse.y | 12 ++++--- server/core/query_classifier.cc | 3 ++ .../modules/filter/dbfwfilter/dbfwfilter.hh | 3 +- .../readwritesplit/rwsplit_route_stmt.cc | 2 +- .../schemarouter/schemaroutersession.cc | 2 +- 10 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 maxscale-system-test/mxs1896_load_data_infile.cpp diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index fce557878..116e59a1b 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -104,6 +104,7 @@ typedef enum qc_query_op QUERY_OP_EXPLAIN, QUERY_OP_GRANT, QUERY_OP_INSERT, + QUERY_OP_LOAD_LOCAL, QUERY_OP_LOAD, QUERY_OP_REVOKE, QUERY_OP_SELECT, diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 827a588fc..ff121b91a 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -658,6 +658,10 @@ add_test_executable(mxs1831_unknown_param.cpp mxs1831_unknown_param replication # https://jira.mariadb.org/browse/MXS-1873 add_test_executable(mxs1873_large_sescmd.cpp mxs1873_large_sescmd replication LABELS readwritesplit REPL_BACKEND) +# MXS-1896: LOAD DATA INFILE is mistaken for LOAD DATA LOCAL INFILE +# https://jira.mariadb.org/browse/MXS-1896 +add_test_executable(mxs1896_load_data_infile.cpp mxs1896_load_data_infile replication LABELS readwritesplit REPL_BACKEND) + # 'namedserverfilter' test add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND) diff --git a/maxscale-system-test/mxs1896_load_data_infile.cpp b/maxscale-system-test/mxs1896_load_data_infile.cpp new file mode 100644 index 000000000..31bf863b5 --- /dev/null +++ b/maxscale-system-test/mxs1896_load_data_infile.cpp @@ -0,0 +1,32 @@ +/** + * MXS-1896: LOAD DATA INFILE is mistaken for LOAD DATA LOCAL INFILE + * + * https://jira.mariadb.org/browse/MXS-1896 + */ + +#include "testconnections.h" + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + test.set_timeout(30); + test.maxscales->connect(); + + test.try_query(test.maxscales->conn_rwsplit[0], "DROP TABLE IF EXISTS test.t1"); + test.try_query(test.maxscales->conn_rwsplit[0], "CREATE TABLE test.t1(id INT)"); + test.try_query(test.maxscales->conn_rwsplit[0], "INSERT INTO test.t1 VALUES (1), (2), (3)"); + test.try_query(test.maxscales->conn_rwsplit[0], "SELECT * FROM test.t1 INTO OUTFILE '/tmp/test.csv'"); + test.try_query(test.maxscales->conn_rwsplit[0], "LOAD DATA INFILE '/tmp/test.csv' INTO TABLE test.t1"); + test.try_query(test.maxscales->conn_rwsplit[0], "DROP TABLE test.t1"); + + test.maxscales->disconnect(); + + // Clean up the generated files + for (int i = 0; i < 4; i++) + { + test.repl->ssh_node_f(i, true, "rm -f /tmp/test.csv"); + } + + return test.global_result; +} diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 8ea9ce177..07c813cf2 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -2001,7 +2001,7 @@ int32_t qc_mysql_get_operation(GWBUF* querybuf, int32_t* operation) break; case SQLCOM_LOAD: - *operation = QUERY_OP_LOAD; + *operation = QUERY_OP_LOAD_LOCAL; break; case SQLCOM_GRANT: diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index af49d29d8..a71e501e2 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -2327,13 +2327,13 @@ public: exposed_sqlite3SrcListDelete(pParse->db, pFullName); } - void maxscaleLoadData(Parse* pParse, SrcList* pFullName) + void maxscaleLoadData(Parse* pParse, SrcList* pFullName, int local) { ss_dassert(this_thread.initialized); m_status = QC_QUERY_PARSED; m_type_mask = QUERY_TYPE_WRITE; - m_operation = QUERY_OP_LOAD; + m_operation = local ? QUERY_OP_LOAD_LOCAL: QUERY_OP_LOAD; if (pFullName) { @@ -3299,7 +3299,7 @@ extern void maxscaleExecuteImmediate(Parse*, Token* pName, ExprSpan* pExprSpan, extern void maxscaleExplain(Parse*, Token* pNext); extern void maxscaleFlush(Parse*, Token* pWhat); extern void maxscaleHandler(Parse*, mxs_handler_t, SrcList* pFullName, Token* pName); -extern void maxscaleLoadData(Parse*, SrcList* pFullName); +extern void maxscaleLoadData(Parse*, SrcList* pFullName, int local); extern void maxscaleLock(Parse*, mxs_lock_t, SrcList*); extern void maxscalePrepare(Parse*, Token* pName, Expr* pStmt); extern void maxscalePrivileges(Parse*, int kind); @@ -4201,14 +4201,14 @@ void maxscaleHandler(Parse* pParse, mxs_handler_t type, SrcList* pFullName, Toke QC_EXCEPTION_GUARD(pInfo->maxscaleHandler(pParse, type, pFullName, pName)); } -void maxscaleLoadData(Parse* pParse, SrcList* pFullName) +void maxscaleLoadData(Parse* pParse, SrcList* pFullName, int local) { QC_TRACE(); QcSqliteInfo* pInfo = this_thread.pInfo; ss_dassert(pInfo); - QC_EXCEPTION_GUARD(pInfo->maxscaleLoadData(pParse, pFullName)); + QC_EXCEPTION_GUARD(pInfo->maxscaleLoadData(pParse, pFullName, local)); } void maxscaleLock(Parse* pParse, mxs_lock_t type, SrcList* pTables) 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 21707bee2..bd08e3219 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -120,7 +120,7 @@ extern void maxscaleExecuteImmediate(Parse*, Token* pName, ExprSpan* pExprSpan, extern void maxscaleExplain(Parse*, Token* pNext); extern void maxscaleFlush(Parse*, Token* pWhat); extern void maxscaleHandler(Parse*, mxs_handler_t, SrcList* pFullName, Token* pName); -extern void maxscaleLoadData(Parse*, SrcList* pFullName); +extern void maxscaleLoadData(Parse*, SrcList* pFullName, int local); extern void maxscaleLock(Parse*, mxs_lock_t, SrcList*); extern void maxscalePrepare(Parse*, Token* pName, Expr* pStmt); extern void maxscalePrivileges(Parse*, int kind); @@ -2913,19 +2913,21 @@ handler ::= HANDLER nm(X) CLOSE. { //////////////////////// The LOAD DATA INFILE statement //////////////////////////////////// // +%type ld_local_opt {int} + cmd ::= load_data. ld_priority_opt ::= . ld_priority_opt ::= LOW_PRIORITY. ld_priority_opt ::= CONCURRENT. -ld_local_opt ::= . -ld_local_opt ::= LOCAL. +ld_local_opt(A) ::= . {A = 0;} +ld_local_opt(A) ::= LOCAL. {A = 1;} ld_charset_opt ::= . ld_charset_opt ::= CHARACTER SET ids. -load_data ::= LOAD DATA ld_priority_opt ld_local_opt +load_data ::= LOAD DATA ld_priority_opt ld_local_opt(Y) INFILE STRING ignore_or_replace_opt INTO TABLE fullname(X) /* ld_partition_opt */ @@ -2935,7 +2937,7 @@ load_data ::= LOAD DATA ld_priority_opt ld_local_opt /* ld_col_name_or_user_var_opt */ /* ld_set */. { - maxscaleLoadData(pParse, X); + maxscaleLoadData(pParse, X, Y); } //////////////////////// The LOCK/UNLOCK statement //////////////////////////////////// diff --git a/server/core/query_classifier.cc b/server/core/query_classifier.cc index 2cf3dfa96..787abd21b 100644 --- a/server/core/query_classifier.cc +++ b/server/core/query_classifier.cc @@ -389,6 +389,9 @@ const char* qc_op_to_string(qc_query_op_t op) case QUERY_OP_LOAD: return "QUERY_OP_LOAD"; + case QUERY_OP_LOAD_LOCAL: + return "QUERY_OP_LOAD_LOCAL"; + case QUERY_OP_REVOKE: return "QUERY_OP_REVOKE"; diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index 738ef128d..1cc3dec9a 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -85,6 +85,7 @@ static inline fw_op_t qc_op_to_fw_op(qc_query_op_t op) case QUERY_OP_INSERT: return FW_OP_INSERT; + case QUERY_OP_LOAD_LOCAL: case QUERY_OP_LOAD: return FW_OP_LOAD; @@ -288,4 +289,4 @@ char* create_error(const char* format, ...); */ bool rule_matches(Dbfw* my_instance, DbfwSession* my_session, GWBUF *queue, SRule rule, char* query); -bool rule_is_active(SRule rule); \ No newline at end of file +bool rule_is_active(SRule rule); diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index b0ba764af..3b2059e8e 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -881,7 +881,7 @@ handle_multi_temp_and_load(RWSplitSession *rses, GWBUF *querybuf, else if (is_packet_a_query(packet_type)) { qc_query_op_t queryop = qc_get_operation(querybuf); - if (queryop == QUERY_OP_LOAD) + if (queryop == QUERY_OP_LOAD_LOCAL) { rses->load_data_state = LOAD_DATA_START; rses->rses_load_data_sent = 0; diff --git a/server/modules/routing/schemarouter/schemaroutersession.cc b/server/modules/routing/schemarouter/schemaroutersession.cc index 092cb1ecd..dd77f2d91 100644 --- a/server/modules/routing/schemarouter/schemaroutersession.cc +++ b/server/modules/routing/schemarouter/schemaroutersession.cc @@ -424,7 +424,7 @@ int32_t SchemaRouterSession::routeQuery(GWBUF* pPacket) /** We know where to route this query */ SSRBackend bref = get_bref_from_dcb(target_dcb); - if (op == QUERY_OP_LOAD) + if (op == QUERY_OP_LOAD_LOCAL) { m_load_target = bref->backend()->server; }