From bf62f8950af8536b859aab268c7b73283b5821f6 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 7 Nov 2016 11:22:48 +0200 Subject: [PATCH] Remove qc_get_affected_fields Function is no longer used and it was quite unoptimal, so now removed. qc_get_prepare_name, qc_get_prepare_operation and qc_get_field_info that were missing from qc_dummy added at the same time. --- include/maxscale/query_classifier.h | 12 --- query_classifier/qc_dummy/qc_dummy.cc | 25 +++++-- .../qc_mysqlembedded/qc_mysqlembedded.cc | 57 --------------- query_classifier/qc_sqlite/qc_sqlite.c | 73 ------------------- query_classifier/test/compare.cc | 63 ---------------- server/core/query_classifier.c | 8 -- 6 files changed, 19 insertions(+), 219 deletions(-) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index 00a7688c5..b7e3f4639 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -125,7 +125,6 @@ typedef struct query_classifier char** (*qc_get_table_names)(GWBUF* stmt, int* tblsize, bool fullnames); char* (*qc_get_canonical)(GWBUF* stmt); bool (*qc_query_has_clause)(GWBUF* stmt); - char* (*qc_get_affected_fields)(GWBUF* stmt); char** (*qc_get_database_names)(GWBUF* stmt, int* size); char* (*qc_get_prepare_name)(GWBUF* stmt); qc_query_op_t (*qc_get_prepare_operation)(GWBUF* stmt); @@ -225,17 +224,6 @@ void qc_thread_end(void); */ qc_parse_result_t qc_parse(GWBUF* stmt); -/** - * Returns the fields the statement affects, as a string of names separated - * by spaces. Note that the fields do not contain any table information. - * - * @param stmt A buffer containing a COM_QUERY packet. - * - * @return A string containing the fields or NULL if a memory allocation - * failure occurs. The string must be freed by the caller. - */ -char* qc_get_affected_fields(GWBUF* stmt); - /** * Returns information about affected fields. * diff --git a/query_classifier/qc_dummy/qc_dummy.cc b/query_classifier/qc_dummy/qc_dummy.cc index 2ee32e90c..1d326d499 100644 --- a/query_classifier/qc_dummy/qc_dummy.cc +++ b/query_classifier/qc_dummy/qc_dummy.cc @@ -45,11 +45,6 @@ bool qc_is_drop_table_query(GWBUF* querybuf) return false; } -char* qc_get_affected_fields(GWBUF* buf) -{ - return NULL; -} - bool qc_query_has_clause(GWBUF* buf) { return false; @@ -66,6 +61,22 @@ qc_query_op_t qc_get_operation(GWBUF* querybuf) return QUERY_OP_UNDEFINED; } +char* qc_sqlite_get_prepare_name(GWBUF* query) +{ + return NULL; +} + +qc_query_op_t qc_sqlite_get_prepare_operation(GWBUF* query) +{ + return QUERY_OP_UNDEFINED; +} + +void qc_sqlite_get_field_info(GWBUF* query, const QC_FIELD_INFO** infos, size_t* n_infos) +{ + *infos = NULL; + *n_infos = 0; +} + bool qc_init(const char* args) { return true; @@ -125,8 +136,10 @@ extern "C" qc_get_table_names, NULL, qc_query_has_clause, - qc_get_affected_fields, qc_get_database_names, + qc_get_prepare_name, + qc_get_prepare_operation, + qc_get_field_info, }; QUERY_CLASSIFIER* GetModuleObject() diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 2d48cb215..29ec2af53 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -79,7 +79,6 @@ typedef struct parsing_info_st QC_FIELD_INFO* field_infos; size_t field_infos_len; size_t field_infos_capacity; - char* affected_fields; #if defined(SS_DEBUG) skygw_chk_t pi_chk_tail; #endif @@ -1305,60 +1304,6 @@ bool qc_is_drop_table_query(GWBUF* querybuf) return answer; } -char* qc_get_affected_fields(GWBUF* buf) -{ - char* affected_fields = NULL; - - if (ensure_query_is_parsed(buf)) - { - parsing_info_t *pi = get_pinfo(buf); - - if (pi->affected_fields) - { - affected_fields = pi->affected_fields; - } - else - { - const QC_FIELD_INFO* infos; - size_t n_infos; - - qc_get_field_info(buf, &infos, &n_infos); - - size_t buflen = 0; - - for (size_t i = 0; i < n_infos; ++i) - { - buflen += strlen(infos[i].column); - buflen += 1; - } - - buflen += 1; - - affected_fields = (char*)malloc(buflen); - affected_fields[0] = 0; - - for (size_t i = 0; i < n_infos; ++i) - { - strcat(affected_fields, infos[i].column); - strcat(affected_fields, " "); - } - - pi->affected_fields = affected_fields; - } - - ss_dassert(affected_fields); - } - - if (!affected_fields) - { - affected_fields = (char*)""; - } - - affected_fields = strdup(affected_fields); - - return affected_fields; -} - bool qc_query_has_clause(GWBUF* buf) { bool clause = false; @@ -1492,7 +1437,6 @@ static void parsing_info_done(void* ptr) free(pi->field_infos[i].column); } free(pi->field_infos); - free(pi->affected_fields); free(pi); } @@ -2375,7 +2319,6 @@ static QUERY_CLASSIFIER qc = qc_get_table_names, NULL, qc_query_has_clause, - qc_get_affected_fields, qc_get_database_names, qc_get_prepare_name, qc_get_prepare_operation, diff --git a/query_classifier/qc_sqlite/qc_sqlite.c b/query_classifier/qc_sqlite/qc_sqlite.c index eae3f00ab..fc579103c 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.c +++ b/query_classifier/qc_sqlite/qc_sqlite.c @@ -60,7 +60,6 @@ typedef struct qc_sqlite_info uint32_t types; // The types of the query. qc_query_op_t operation; // The operation in question. - char* affected_fields; // The affected fields. bool is_real_query; // SELECT, UPDATE, INSERT, DELETE or a variation. bool has_clause; // Has WHERE or HAVING. char** table_names; // Array of table names used in the query. @@ -305,7 +304,6 @@ static QC_SQLITE_INFO* info_alloc(void) static void info_finish(QC_SQLITE_INFO* info) { - free(info->affected_fields); free_string_array(info->table_names); free_string_array(info->table_fullnames); free(info->created_table_name); @@ -332,7 +330,6 @@ static QC_SQLITE_INFO* info_init(QC_SQLITE_INFO* info) info->types = QUERY_TYPE_UNKNOWN; info->operation = QUERY_OP_UNDEFINED; - info->affected_fields = NULL; info->is_real_query = false; info->has_clause = false; info->table_names = NULL; @@ -2587,7 +2584,6 @@ static bool qc_sqlite_is_real_query(GWBUF* query); static char** qc_sqlite_get_table_names(GWBUF* query, int* tblsize, bool fullnames); static char* qc_sqlite_get_canonical(GWBUF* query); static bool qc_sqlite_query_has_clause(GWBUF* query); -static char* qc_sqlite_get_affected_fields(GWBUF* query); static char** qc_sqlite_get_database_names(GWBUF* query, int* sizep); static bool get_key_and_value(char* arg, const char** pkey, const char** pvalue) @@ -2995,74 +2991,6 @@ static bool qc_sqlite_query_has_clause(GWBUF* query) return has_clause; } -static char* qc_sqlite_get_affected_fields(GWBUF* query) -{ - QC_TRACE(); - ss_dassert(this_unit.initialized); - ss_dassert(this_thread.initialized); - - char* affected_fields = NULL; - QC_SQLITE_INFO* info = get_query_info(query); - - if (info) - { - if (qc_info_is_valid(info->status)) - { - if (!info->affected_fields) - { - if (info->field_infos_len != 0) - { - // The first time qc_sqlite_get_affected_fields() is called - // we copy the column data from info->fields_infos into - // info->affected_fields. - QC_FIELD_INFO* fis = info->field_infos; - size_t fis_len = info->field_infos_len; - size_t buflen = 0; - - for (size_t i = 0; i < fis_len; ++i) - { - buflen += strlen(fis[i].column); - buflen += 1; - } - - buflen += 1; - - affected_fields = MXS_MALLOC(buflen); - MXS_ABORT_IF_NULL(affected_fields); - - affected_fields[0] = 0; - - for (size_t i = 0; i < fis_len; ++i) - { - strcat(affected_fields, fis[i].column); - strcat(affected_fields, " "); - } - - info->affected_fields = affected_fields; - } - } - } - else if (MXS_LOG_PRIORITY_IS_ENABLED(LOG_INFO)) - { - log_invalid_data(query, "cannot report what fields are affected"); - } - } - else - { - MXS_ERROR("The query could not be parsed. Response not valid."); - } - - if (!affected_fields) - { - affected_fields = ""; - } - - affected_fields = MXS_STRDUP(affected_fields); - MXS_ABORT_IF_NULL(affected_fields); - - return affected_fields; -} - static char** qc_sqlite_get_database_names(GWBUF* query, int* sizep) { QC_TRACE(); @@ -3203,7 +3131,6 @@ static QUERY_CLASSIFIER qc = qc_sqlite_get_table_names, NULL, qc_sqlite_query_has_clause, - qc_sqlite_get_affected_fields, qc_sqlite_get_database_names, qc_sqlite_get_prepare_name, qc_sqlite_get_prepare_operation, diff --git a/query_classifier/test/compare.cc b/query_classifier/test/compare.cc index 9fa821aea..9c267846d 100644 --- a/query_classifier/test/compare.cc +++ b/query_classifier/test/compare.cc @@ -715,68 +715,6 @@ ostream& operator << (ostream& o, const std::set& s) return o; } -bool compare_get_affected_fields(QUERY_CLASSIFIER* pClassifier1, GWBUF* pCopy1, - QUERY_CLASSIFIER* pClassifier2, GWBUF* pCopy2) -{ - bool success = false; - const char HEADING[] = "qc_get_affected_fields : "; - - char* rv1 = pClassifier1->qc_get_affected_fields(pCopy1); - char* rv2 = pClassifier2->qc_get_affected_fields(pCopy2); - - std::set fields1; - std::set fields2; - - if (rv1) - { - add_fields(fields1, rv1); - } - - if (rv2) - { - add_fields(fields2, rv2); - } - - stringstream ss; - ss << HEADING; - - if ((!rv1 && !rv2) || (rv1 && rv2 && (fields1 == fields2))) - { - ss << "Ok : " << fields1; - success = true; - } - else - { - ss << "ERR: "; - if (rv1) - { - ss << fields1; - } - else - { - ss << "NULL"; - } - - ss << " != "; - - if (rv2) - { - ss << fields2; - } - else - { - ss << "NULL"; - } - } - - report(success, ss.str()); - - free(rv1); - free(rv2); - - return success; -} - bool compare_get_database_names(QUERY_CLASSIFIER* pClassifier1, GWBUF* pCopy1, QUERY_CLASSIFIER* pClassifier2, GWBUF* pCopy2) { @@ -1117,7 +1055,6 @@ bool compare(QUERY_CLASSIFIER* pClassifier1, QUERY_CLASSIFIER* pClassifier2, con errors += !compare_get_table_names(pClassifier1, pCopy1, pClassifier2, pCopy2, false); errors += !compare_get_table_names(pClassifier1, pCopy1, pClassifier2, pCopy2, true); errors += !compare_query_has_clause(pClassifier1, pCopy1, pClassifier2, pCopy2); - errors += !compare_get_affected_fields(pClassifier1, pCopy1, pClassifier2, pCopy2); errors += !compare_get_database_names(pClassifier1, pCopy1, pClassifier2, pCopy2); errors += !compare_get_prepare_name(pClassifier1, pCopy1, pClassifier2, pCopy2); errors += !compare_get_prepare_operation(pClassifier1, pCopy1, pClassifier2, pCopy2); diff --git a/server/core/query_classifier.c b/server/core/query_classifier.c index 70d7f3cfe..3b9abef57 100644 --- a/server/core/query_classifier.c +++ b/server/core/query_classifier.c @@ -189,14 +189,6 @@ bool qc_query_has_clause(GWBUF* query) return classifier->qc_query_has_clause(query); } -char* qc_get_affected_fields(GWBUF* query) -{ - QC_TRACE(); - ss_dassert(classifier); - - return classifier->qc_get_affected_fields(query); -} - void qc_get_field_info(GWBUF* query, const QC_FIELD_INFO** infos, size_t* n_infos) { QC_TRACE();