From dc4ca8d5886314021d01ec7343030455f36aa375 Mon Sep 17 00:00:00 2001 From: Leonid Fedorov <79837786+mariadb-LeonidFedorov@users.noreply.github.com> Date: Thu, 29 May 2025 17:23:37 +0400 Subject: [PATCH] MCOL-5943: MCOL-4740 update rows counter for multi-table update (#3555) * fix(plugin): MCOL-4740: This fixes update rows counter for multi-table update For UPDATEs involving a single table, the server call to handler::direct_update_rows() is used to correctly set the count for the number of updated rows in the UPDATE statement. However, for UPDATEs involving multi-tables, the server does not call handler::direct_update_rows(). This patch adds support to correctly report the number of updated rows to the client by setting multi_update::updated and multi_update::found in handler::rnd_end(). * fix(plugin): MCOL-4740: this is to addres the original patch QA found in the original patch --------- Co-authored-by: Roman Nozdrin Co-authored-by: drrtuy --- dbcon/mysql/ha_mcs_common.h | 148 ++++++++++++++++++ dbcon/mysql/ha_mcs_execplan.cpp | 112 +------------ dbcon/mysql/ha_mcs_impl.cpp | 54 ++++--- dbcon/mysql/ha_mcs_impl_if.h | 26 --- dbcon/mysql/ha_mcs_pushdown.cpp | 74 ++++++--- dbcon/mysql/ha_mcs_pushdown.h | 1 + dbcon/mysql/ha_view.cpp | 3 +- .../columnstore/bugfixes/mcol-4740.result | 80 ++++++++++ .../columnstore/bugfixes/mcol-4740.test | 82 ++++++++++ 9 files changed, 403 insertions(+), 177 deletions(-) create mode 100644 dbcon/mysql/ha_mcs_common.h create mode 100644 mysql-test/columnstore/bugfixes/mcol-4740.result create mode 100644 mysql-test/columnstore/bugfixes/mcol-4740.test diff --git a/dbcon/mysql/ha_mcs_common.h b/dbcon/mysql/ha_mcs_common.h new file mode 100644 index 000000000..812c2514d --- /dev/null +++ b/dbcon/mysql/ha_mcs_common.h @@ -0,0 +1,148 @@ +/* Copyright (C) 2016-2023 MariaDB Corporation + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License + as published by the Free Software Foundation; version 2 of + the License. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + MA 02110-1301, USA. */ + +#pragma once + +#include "idb_mysql.h" + +namespace ha_mcs_common +{ + +inline bool isMCSTable(TABLE* table_ptr) +{ +#if (defined(_MSC_VER) && defined(_DEBUG)) || defined(SAFE_MUTEX) + + if (!(table_ptr->s && (*table_ptr->s->db_plugin)->name.str)) +#else + if (!(table_ptr->s && (table_ptr->s->db_plugin)->name.str)) +#endif + return true; + +#if (defined(_MSC_VER) && defined(_DEBUG)) || defined(SAFE_MUTEX) + std::string engineName = (*table_ptr->s->db_plugin)->name.str; +#else + std::string engineName = table_ptr->s->db_plugin->name.str; +#endif + + if (engineName == "Columnstore" || engineName == "Columnstore_cache") + return true; + return false; +} + +inline bool isMultiUpdateStatement(const enum_sql_command& command) +{ + return command == SQLCOM_UPDATE_MULTI; +} + +inline bool isUpdateStatement(const enum_sql_command& command) +{ + return command == SQLCOM_UPDATE || isMultiUpdateStatement(command); +} + +inline bool isDeleteStatement(const enum_sql_command& command) +{ + return (command == SQLCOM_DELETE) || (command == SQLCOM_DELETE_MULTI); +} + +inline bool isUpdateOrDeleteStatement(const enum_sql_command& command) +{ + return isUpdateStatement(command) || isDeleteStatement(command); +} + +inline bool isDMLStatement(const enum_sql_command& command) +{ + return (command == SQLCOM_INSERT || command == SQLCOM_INSERT_SELECT || command == SQLCOM_TRUNCATE || + command == SQLCOM_LOAD || isUpdateOrDeleteStatement(command)); +} + +inline bool isForeignTableUpdate(THD* thd) +{ + LEX* lex = thd->lex; + + if (!isUpdateStatement(lex->sql_command)) + return false; + + Item_field* item; + List_iterator_fast field_it(lex->first_select_lex()->item_list); + + while ((item = (Item_field*)field_it++)) + { + if (item->field && item->field->table && !isMCSTable(item->field->table)) + return true; + } + + return false; +} + +// This function is different from isForeignTableUpdate() +// above as it only checks if any of the tables involved +// in the multi-table update statement is a foreign table, +// irrespective of whether the update is performed on the +// foreign table or not, as in isForeignTableUpdate(). +inline bool isUpdateHasForeignTable(THD* thd) +{ + LEX* lex = thd->lex; + + if (!isUpdateStatement(lex->sql_command)) + return false; + + TABLE_LIST* table_ptr = lex->first_select_lex()->get_table_list(); + + for (; table_ptr; table_ptr = table_ptr->next_local) + { + if (table_ptr->table && !isMCSTable(table_ptr->table)) + return true; + } + + return false; +} + +inline bool isMCSTableUpdate(THD* thd) +{ + LEX* lex = thd->lex; + + if (!isUpdateStatement(lex->sql_command)) + return false; + + Item_field* item; + List_iterator_fast field_it(lex->first_select_lex()->item_list); + + while ((item = (Item_field*)field_it++)) + { + if (item->field && item->field->table && isMCSTable(item->field->table)) + return true; + } + + return false; +} + +inline bool isMCSTableDelete(THD* thd) +{ + LEX* lex = thd->lex; + + if (!isDeleteStatement(lex->sql_command)) + return false; + + TABLE_LIST* table_ptr = lex->first_select_lex()->get_table_list(); + + if (table_ptr && table_ptr->table && isMCSTable(table_ptr->table)) + return true; + + return false; +} + +} // namespace ha_mcs_common diff --git a/dbcon/mysql/ha_mcs_execplan.cpp b/dbcon/mysql/ha_mcs_execplan.cpp index 8c9376f98..334243a66 100644 --- a/dbcon/mysql/ha_mcs_execplan.cpp +++ b/dbcon/mysql/ha_mcs_execplan.cpp @@ -4968,10 +4968,10 @@ SimpleColumn* buildSimpleColumn(Item_field* ifp, gp_walk_info& gwi) { // check foreign engine if (ifp->cached_table && ifp->cached_table->table) - prm.columnStore(isMCSTable(ifp->cached_table->table)); + prm.columnStore(ha_mcs_common::isMCSTable(ifp->cached_table->table)); // @bug4509. ifp->cached_table could be null for myisam sometimes else if (ifp->field && ifp->field->table) - prm.columnStore(isMCSTable(ifp->field->table)); + prm.columnStore(ha_mcs_common::isMCSTable(ifp->field->table)); if (prm.columnStore()) { @@ -6874,104 +6874,6 @@ void parse_item(Item* item, vector& field_vec, bool& hasNonSupportI } } -bool isMCSTable(TABLE* table_ptr) -{ -#if (defined(_MSC_VER) && defined(_DEBUG)) || defined(SAFE_MUTEX) - - if (!(table_ptr->s && (*table_ptr->s->db_plugin)->name.str)) -#else - if (!(table_ptr->s && (table_ptr->s->db_plugin)->name.str)) -#endif - return true; - -#if (defined(_MSC_VER) && defined(_DEBUG)) || defined(SAFE_MUTEX) - string engineName = (*table_ptr->s->db_plugin)->name.str; -#else - string engineName = table_ptr->s->db_plugin->name.str; -#endif - - if (engineName == "Columnstore" || engineName == "Columnstore_cache") - return true; - else - return false; -} - -bool isForeignTableUpdate(THD* thd) -{ - LEX* lex = thd->lex; - - if (!isUpdateStatement(lex->sql_command)) - return false; - - Item_field* item; - List_iterator_fast field_it(lex->first_select_lex()->item_list); - - while ((item = (Item_field*)field_it++)) - { - if (item->field && item->field->table && !isMCSTable(item->field->table)) - return true; - } - - return false; -} - -bool isMCSTableUpdate(THD* thd) -{ - LEX* lex = thd->lex; - - if (!isUpdateStatement(lex->sql_command)) - return false; - - Item_field* item; - List_iterator_fast field_it(lex->first_select_lex()->item_list); - - while ((item = (Item_field*)field_it++)) - { - if (item->field && item->field->table && isMCSTable(item->field->table)) - return true; - } - - return false; -} - -bool isMCSTableDelete(THD* thd) -{ - LEX* lex = thd->lex; - - if (!isDeleteStatement(lex->sql_command)) - return false; - - TABLE_LIST* table_ptr = lex->first_select_lex()->get_table_list(); - - if (table_ptr && table_ptr->table && isMCSTable(table_ptr->table)) - return true; - - return false; -} - -// This function is different from isForeignTableUpdate() -// above as it only checks if any of the tables involved -// in the multi-table update statement is a foreign table, -// irrespective of whether the update is performed on the -// foreign table or not, as in isForeignTableUpdate(). -bool isUpdateHasForeignTable(THD* thd) -{ - LEX* lex = thd->lex; - - if (!isUpdateStatement(lex->sql_command)) - return false; - - TABLE_LIST* table_ptr = lex->first_select_lex()->get_table_list(); - - for (; table_ptr; table_ptr = table_ptr->next_local) - { - if (table_ptr->table && !isMCSTable(table_ptr->table)) - return true; - } - - return false; -} - /*@brief set some runtime params to run the query */ /*********************************************************** * DESCRIPTION: @@ -7095,7 +6997,7 @@ int processFrom(bool& isUnion, SELECT_LEX& select_lex, gp_walk_info& gwi, SCSEP& else { // check foreign engine tables - bool columnStore = (table_ptr->table ? isMCSTable(table_ptr->table) : true); + bool columnStore = (table_ptr->table ? ha_mcs_common::isMCSTable(table_ptr->table) : true); // trigger system catalog cache if (columnStore) @@ -7249,7 +7151,7 @@ int processWhere(SELECT_LEX& select_lex, gp_walk_info& gwi, SCSEP& csep, const s else if (select_lex.where) icp = select_lex.where; } - else if (!join && isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) + else if (!join && ha_mcs_common::isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) { isUpdateDelete = true; } @@ -8091,7 +7993,7 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i } //@Bug 3030 Add error check for dml statement - if (isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) { if (after_size - before_size != 0) { @@ -8125,7 +8027,7 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i case REAL_RESULT: case TIME_RESULT: { - if (isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) { } else @@ -8158,7 +8060,7 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i case Item::NULL_ITEM: { - if (isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(gwi.thd->lex->sql_command)) { } else diff --git a/dbcon/mysql/ha_mcs_impl.cpp b/dbcon/mysql/ha_mcs_impl.cpp index f3f56a74d..262a8bdb4 100644 --- a/dbcon/mysql/ha_mcs_impl.cpp +++ b/dbcon/mysql/ha_mcs_impl.cpp @@ -832,7 +832,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c { Message::Args args; - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) args.add("Update"); #if 0 else if (thd->rgi_slave && thd->rgi_slave->m_table_map.count() != 0) @@ -889,7 +889,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c updateCP->isDML(true); //@Bug 2753. the memory already freed by destructor of UpdateSqlStatement - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { ColumnAssignment* columnAssignmentPtr = nullptr; Item_field* item; @@ -1184,7 +1184,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c } // Exit early if there is nothing to update - if (colAssignmentListPtr->empty() && isUpdateStatement(thd->lex->sql_command)) + if (colAssignmentListPtr->empty() && ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { ci->affectedRows = 0; delete colAssignmentListPtr; @@ -1198,7 +1198,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c CalpontSystemCatalog::TableName aTableName; TABLE_LIST* first_table = 0; - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { aTableName.schema = schemaName; aTableName.table = tableName; @@ -1220,7 +1220,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c IDEBUG(cout << "STMT: " << dmlStmt << " and sessionID " << thd->thread_id << endl); VendorDMLStatement dmlStatement(dmlStmt, sessionID); - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) dmlStatement.set_DMLStatementType(DML_UPDATE); else dmlStatement.set_DMLStatementType(DML_DELETE); @@ -1229,7 +1229,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c //@Bug 2753. To make sure the momory is freed. updateStmt.fColAssignmentListPtr = colAssignmentListPtr; - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { TableName* qualifiedTablName = new TableName(); qualifiedTablName->fName = tableName; @@ -1318,7 +1318,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c List items; SELECT_LEX select_lex; - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { items = (thd->lex->first_select_lex()->item_list); thd->lex->first_select_lex()->item_list = thd->lex->value_list; @@ -1467,7 +1467,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c returnedCols.push_back((updateCP->columnMap()).begin()->second); //@Bug 6123. get the correct returned columnlist - if (isDeleteStatement(thd->lex->sql_command)) + if (ha_mcs_common::isDeleteStatement(thd->lex->sql_command)) { returnedCols.clear(); // choose the smallest column to project @@ -1518,7 +1518,7 @@ uint32_t doUpdateDelete(THD* thd, gp_walk_info& gwi, const std::vector& c updateCP->returnedCols(returnedCols); - if (isUpdateStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateStatement(thd->lex->sql_command)) { const ParseTree* ptsub = updateCP->filters(); @@ -1979,7 +1979,7 @@ int ha_mcs_impl_analyze(THD* thd, TABLE* table) if (table->s->db.length && strcmp(table->s->db.str, "information_schema") == 0) return 0; - bool columnStore = (table ? isMCSTable(table) : true); + bool columnStore = (table ? ha_mcs_common::isMCSTable(table) : true); // Skip non columnstore tables. if (!columnStore) return 0; @@ -2156,7 +2156,8 @@ int ha_mcs_impl_direct_update_delete_rows(bool execute, ha_rows* affected_rows, gwi.thd = thd; int rc = 0; - if (thd->slave_thread && !get_replication_slave(thd) && isDMLStatement(thd->lex->sql_command)) + if (thd->slave_thread && !get_replication_slave(thd) && + ha_mcs_common::isDMLStatement(thd->lex->sql_command)) { if (affected_rows) *affected_rows = 0; @@ -2189,7 +2190,7 @@ int ha_mcs::impl_rnd_init(TABLE* table, const std::vector& condStack) gwi.thd = thd; if (thd->slave_thread && !get_replication_slave(thd) && - (isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) + (ha_mcs_common::isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) return 0; // check whether the system is ready to process statement. @@ -2241,7 +2242,7 @@ int ha_mcs::impl_rnd_init(TABLE* table, const std::vector& condStack) UPDATE innotab1 SET a=100 WHERE a NOT IN (SELECT a FROM cstab1 WHERE a=1); */ if (!isReadOnly() && // make sure the current table is being modified - isUpdateOrDeleteStatement(thd->lex->sql_command)) + ha_mcs_common::isUpdateOrDeleteStatement(thd->lex->sql_command)) return doUpdateDelete(thd, gwi, condStack); uint32_t sessionID = tid2sid(thd->thread_id); @@ -2578,10 +2579,10 @@ int ha_mcs_impl_rnd_next(uchar* buf, TABLE* table, long timeZone) THD* thd = current_thd; if (thd->slave_thread && !get_replication_slave(thd) && - (isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) + (ha_mcs_common::isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) return HA_ERR_END_OF_FILE; - if (isMCSTableUpdate(thd) || isMCSTableDelete(thd)) + if (ha_mcs_common::isMCSTableUpdate(thd) || ha_mcs_common::isMCSTableDelete(thd)) return HA_ERR_END_OF_FILE; // @bug 2547 @@ -2661,18 +2662,17 @@ int ha_mcs_impl_rnd_end(TABLE* table, bool is_pushdown_hand) THD* thd = current_thd; if (thd->slave_thread && !get_replication_slave(thd) && - (isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) - return 0; + (ha_mcs_common::isDMLStatement(thd->lex->sql_command) || thd->lex->sql_command == SQLCOM_ALTER_TABLE)) + return rc; cal_connection_info* ci = nullptr; - if (get_fe_conn_info_ptr() != NULL) ci = reinterpret_cast(get_fe_conn_info_ptr()); if ((thd->lex)->sql_command == SQLCOM_ALTER_TABLE) return rc; - if (isMCSTableUpdate(thd) || isMCSTableDelete(thd)) + if (ha_mcs_common::isMCSTableUpdate(thd) || ha_mcs_common::isMCSTableDelete(thd)) return rc; if (!ci) @@ -3764,7 +3764,7 @@ COND* ha_mcs_impl_cond_push(COND* cond, TABLE* table, std::vector& condSt { THD* thd = current_thd; - if (isUpdateOrDeleteStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(thd->lex->sql_command)) { condStack.push_back(cond); return nullptr; @@ -3848,7 +3848,7 @@ COND* ha_mcs_impl_cond_push(COND* cond, TABLE* table, std::vector& condSt inline void disableBinlogForDML(THD* thd) { - if (isDMLStatement(thd->lex->sql_command) && (thd->variables.option_bits & OPTION_BIN_LOG)) + if (ha_mcs_common::isDMLStatement(thd->lex->sql_command) && (thd->variables.option_bits & OPTION_BIN_LOG)) { set_original_option_bits(thd->variables.option_bits, thd); thd->variables.option_bits &= ~OPTION_BIN_LOG; @@ -3858,7 +3858,7 @@ inline void disableBinlogForDML(THD* thd) inline void restoreBinlogForDML(THD* thd) { - if (isDMLStatement(thd->lex->sql_command)) + if (ha_mcs_common::isDMLStatement(thd->lex->sql_command)) { ulonglong orig_option_bits = get_original_option_bits(thd); @@ -4027,7 +4027,8 @@ int ha_mcs_impl_pushdown_init(mcs_handler_info* handler_info, TABLE* table, bool IDEBUG(cout << "pushdown_init for table " << endl); THD* thd = current_thd; - if (thd->slave_thread && !get_replication_slave(thd) && isDMLStatement(thd->lex->sql_command)) + if (thd->slave_thread && !get_replication_slave(thd) && + ha_mcs_common::isDMLStatement(thd->lex->sql_command)) return 0; const char* timeZone = thd->variables.time_zone->get_name()->ptr(); @@ -4069,7 +4070,7 @@ int ha_mcs_impl_pushdown_init(mcs_handler_info* handler_info, TABLE* table, bool // MCOL-4023 We need to test this code path. // Update and delete code - if (isUpdateOrDeleteStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(thd->lex->sql_command)) return doUpdateDelete(thd, gwi, std::vector()); uint32_t sessionID = tid2sid(thd->thread_id); @@ -4493,7 +4494,8 @@ int ha_mcs_impl_select_next(uchar* buf, TABLE* table, long timeZone) { THD* thd = current_thd; - if (thd->slave_thread && !get_replication_slave(thd) && isDMLStatement(thd->lex->sql_command)) + if (thd->slave_thread && !get_replication_slave(thd) && + ha_mcs_common::isDMLStatement(thd->lex->sql_command)) return HA_ERR_END_OF_FILE; int rc = HA_ERR_END_OF_FILE; @@ -4506,7 +4508,7 @@ int ha_mcs_impl_select_next(uchar* buf, TABLE* table, long timeZone) cal_connection_info* ci = reinterpret_cast(get_fe_conn_info_ptr()); - if (isUpdateOrDeleteStatement(thd->lex->sql_command)) + if (ha_mcs_common::isUpdateOrDeleteStatement(thd->lex->sql_command)) return rc; // @bug 2547 diff --git a/dbcon/mysql/ha_mcs_impl_if.h b/dbcon/mysql/ha_mcs_impl_if.h index 7b02d82e0..2cac13f67 100644 --- a/dbcon/mysql/ha_mcs_impl_if.h +++ b/dbcon/mysql/ha_mcs_impl_if.h @@ -401,11 +401,6 @@ void clearDeleteStacks(gp_walk_info& gwi); void parse_item(Item* item, std::vector& field_vec, bool& hasNonSupportItem, uint16& parseInfo, gp_walk_info* gwip = nullptr); const std::string bestTableName(const Item_field* ifp); -bool isMCSTable(TABLE* table_ptr); -bool isForeignTableUpdate(THD* thd); -bool isUpdateHasForeignTable(THD* thd); -bool isMCSTableUpdate(THD* thd); -bool isMCSTableDelete(THD* thd); // execution plan util functions prototypes execplan::ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport, @@ -452,27 +447,6 @@ bool buildEqualityPredicate(execplan::ReturnedColumn* lhs, execplan::ReturnedCol boost::shared_ptr& sop, const Item_func::Functype& funcType, const std::vector& itemList, bool isInSubs = false); -inline bool isUpdateStatement(const enum_sql_command& command) -{ - return ((command == SQLCOM_UPDATE) || (command == SQLCOM_UPDATE_MULTI)); -} - -inline bool isDeleteStatement(const enum_sql_command& command) -{ - return (command == SQLCOM_DELETE) || (command == SQLCOM_DELETE_MULTI); -} - -inline bool isUpdateOrDeleteStatement(const enum_sql_command& command) -{ - return isUpdateStatement(command) || isDeleteStatement(command); -} - -inline bool isDMLStatement(const enum_sql_command& command) -{ - return (command == SQLCOM_INSERT || command == SQLCOM_INSERT_SELECT || command == SQLCOM_TRUNCATE || - command == SQLCOM_LOAD || isUpdateOrDeleteStatement(command)); -} - #ifdef DEBUG_WALK_COND void debug_walk(const Item* item, void* arg); #endif diff --git a/dbcon/mysql/ha_mcs_pushdown.cpp b/dbcon/mysql/ha_mcs_pushdown.cpp index 4cc8fda8f..55681af49 100644 --- a/dbcon/mysql/ha_mcs_pushdown.cpp +++ b/dbcon/mysql/ha_mcs_pushdown.cpp @@ -17,8 +17,40 @@ #include #include +// This makes specific MDB classes' attributes public to implement +// MCOL-4740 temporary solution. Search for MCOL-4740 +// to get the actual place where it is used. +#define updated_leaves \ + updated_leaves; \ + \ + public: + #include "ha_mcs_pushdown.h" +void update_counters_on_multi_update() +{ + if (ha_mcs_common::isMultiUpdateStatement(current_thd->lex->sql_command) && + !ha_mcs_common::isForeignTableUpdate(current_thd)) + { + SELECT_LEX_UNIT* unit = ¤t_thd->lex->unit; + SELECT_LEX* select_lex = unit->first_select(); + auto* multi = (select_lex->join) ? reinterpret_cast(select_lex->join->result) : nullptr; + + if (multi) + { + multi->table_to_update = multi->update_tables ? multi->update_tables->table : 0; + + cal_impl_if::cal_connection_info* ci = + reinterpret_cast(get_fe_conn_info_ptr()); + + if (ci) + { + multi->updated = multi->found = ci->affectedRows; + } + } + } +} + void check_walk(const Item* item, void* arg); void restore_query_state(ha_columnstore_select_handler* handler) @@ -391,7 +423,7 @@ derived_handler* create_columnstore_derived_handler(THD* thd, TABLE_LIST* table_ // MCOL-1482 Disable derived_handler if the multi-table update // statement contains a non-columnstore table. - if (cal_impl_if::isUpdateHasForeignTable(thd)) + if (ha_mcs_common::isUpdateHasForeignTable(thd)) { return handler; } @@ -551,18 +583,18 @@ void ha_columnstore_derived_handler::print_error(int, unsigned long) /*@brief create_columnstore_select_handler_- Creates handler ************************************************************ - * DESCRIPTION: - * Creates a select handler if there is no non-equi JOIN, e.g - * t1.c1 > t2.c2 and logical OR in the filter predicates. - * More details in server/sql/select_handler.h - * PARAMETERS: - * thd - THD pointer. - * sel_lex - SELECT_LEX* that describes the query. - * sel_unit - SELECT_LEX_UNIT* that describes the query. - * RETURN: - * select_handler if possible - * NULL in other case - ***********************************************************/ +* DESCRIPTION: +* Creates a select handler if there is no non-equi JOIN, e.g +* t1.c1 > t2.c2 and logical OR in the filter predicates. +* More details in server/sql/select_handler.h +* PARAMETERS: +* thd - THD pointer. +* sel_lex - SELECT_LEX* that describes the query. +* sel_unit - SELECT_LEX_UNIT* that describes the query. +* RETURN: +* select_handler if possible +* NULL in other case +***********************************************************/ select_handler* create_columnstore_select_handler_(THD* thd, SELECT_LEX* sel_lex, SELECT_LEX_UNIT* sel_unit) { mcs_select_handler_mode_t select_handler_mode = get_select_handler_mode(thd); @@ -578,7 +610,7 @@ select_handler* create_columnstore_select_handler_(THD* thd, SELECT_LEX* sel_lex // MCOL-1482 Disable select_handler for a multi-table update // with a non-columnstore table as the target table of the update // operation. - if (cal_impl_if::isForeignTableUpdate(thd)) + if (ha_mcs_common::isForeignTableUpdate(thd)) { return nullptr; } @@ -646,15 +678,15 @@ select_handler* create_columnstore_select_handler_(THD* thd, SELECT_LEX* sel_lex // or unsupported feature. ha_columnstore_select_handler* handler; - if (sel_unit && sel_lex) // partial pushdown of the SELECT_LEX_UNIT + if (sel_unit && sel_lex) // partial pushdown of the SELECT_LEX_UNIT { handler = new ha_columnstore_select_handler(thd, sel_lex, sel_unit); } - else if (sel_unit) // complete pushdown of the SELECT_LEX_UNIT + else if (sel_unit) // complete pushdown of the SELECT_LEX_UNIT { handler = new ha_columnstore_select_handler(thd, sel_unit); } - else // Query only has a SELECT_LEX, no SELECT_LEX_UNIT + else // Query only has a SELECT_LEX, no SELECT_LEX_UNIT { handler = new ha_columnstore_select_handler(thd, sel_lex); } @@ -749,8 +781,7 @@ select_handler* create_columnstore_select_handler_(THD* thd, SELECT_LEX* sel_lex select_lex != select_lex->master_unit()->fake_select_lex) // (2) thd->lex->set_limit_rows_examined(); - if ((!sel_unit || sel_lex) && !join->tables_list && - (join->table_count || !select_lex->with_sum_func) && + if ((!sel_unit || sel_lex) && !join->tables_list && (join->table_count || !select_lex->with_sum_func) && !select_lex->have_window_funcs()) { if (!thd->is_error()) @@ -1010,6 +1041,11 @@ int ha_columnstore_select_handler::end_scan() scan_ended = true; + // MCOL-4740 multi_update::send_eof(), which outputs the affected + // number of rows to the client, is called after handler::rnd_end(). + // So we set multi_update::updated and multi_update::found here. + update_counters_on_multi_update(); + int rc = ha_mcs_impl_rnd_end(table, true); DBUG_RETURN(rc); diff --git a/dbcon/mysql/ha_mcs_pushdown.h b/dbcon/mysql/ha_mcs_pushdown.h index 2ca977535..44656c466 100644 --- a/dbcon/mysql/ha_mcs_pushdown.h +++ b/dbcon/mysql/ha_mcs_pushdown.h @@ -20,6 +20,7 @@ #include "idb_mysql.h" #include "ha_mcs.h" #include "ha_mcs_sysvars.h" +#include "ha_mcs_common.h" #define NEED_CALPONT_EXTERNS #include "ha_mcs_impl.h" #include "ha_mcs_impl_if.h" diff --git a/dbcon/mysql/ha_view.cpp b/dbcon/mysql/ha_view.cpp index 0193d6e75..e78cea5da 100644 --- a/dbcon/mysql/ha_view.cpp +++ b/dbcon/mysql/ha_view.cpp @@ -42,6 +42,7 @@ using namespace execplan; #include "ha_subquery.h" #include "ha_view.h" +#include "ha_mcs_common.h" namespace cal_impl_if { @@ -131,7 +132,7 @@ void View::transform() else { // check foreign engine tables - bool columnStore = (table_ptr->table ? isMCSTable(table_ptr->table) : true); + bool columnStore = (table_ptr->table ? ha_mcs_common::isMCSTable(table_ptr->table) : true); // trigger system catalog cache if (columnStore) diff --git a/mysql-test/columnstore/bugfixes/mcol-4740.result b/mysql-test/columnstore/bugfixes/mcol-4740.result new file mode 100644 index 000000000..6277926ae --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4740.result @@ -0,0 +1,80 @@ +DROP DATABASE IF EXISTS mcol_4740; +CREATE DATABASE mcol_4740; +USE mcol_4740; +CREATE TABLE mcs_1 (a INT, b INT(11), c VARCHAR(100)) engine=columnstore; +CREATE TABLE mcs_2 (a INT, b INT(11), c VARCHAR(100)) engine=columnstore; +INSERT INTO mcs_1 VALUES (33, 99, 1); +INSERT INTO mcs_1 VALUES (33, 99, 2); +INSERT INTO mcs_1 VALUES (33, 99, 3); +INSERT INTO mcs_2 VALUES (33, 11, 1); +INSERT INTO mcs_2 VALUES (33, 11, 2); +INSERT INTO mcs_2 VALUES (33, 11, 3); +SELECT * FROM mcs_1; +a b c +33 99 1 +33 99 2 +33 99 3 +UPDATE mcs_1 A, mcs_2 B SET A.b = B.b WHERE A.c = B.c LIMIT 1; +affected rows: 1 +info: Rows matched: 1 Changed: 1 Warnings: 0 +SELECT * FROM mcs_1; +a b c +33 11 1 +33 99 2 +33 99 3 +UPDATE mcs_1 A, mcs_2 B SET A.b = A.b + 1, B.b = B.b + 1; +ERROR 42000: The storage engine for the table doesn't support MCS-1012: This version of Columnstore supports update of only one table at a time. +CREATE TABLE `test1` ( +`id` text NOT NULL, +`p_date` int(11) DEFAULT NULL, +`con_bfr` text DEFAULT NULL, +`org_bfr` text DEFAULT NULL, +`pat_bfr` text DEFAULT NULL, +`data_type` text DEFAULT NULL, +`order_no` text DEFAULT NULL, +`tra_bfr` text DEFAULT NULL, +`load_bfr` text DEFAULT NULL, +`status` int(11) DEFAULT NULL +) ENGINE=Columnstore DEFAULT CHARSET=utf8; +INSERT INTO test1 values ('index_srgt_id','0','consultation_date_bfr','organization_code_bfr','patient_id_bfr','data_type','order_no','transaction_time_bfr','load_timestamp_bfr','0'); +INSERT INTO test1 values ('adt-a55-9000000001','20190610','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000002','20190610','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000003','20190611','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000004','20190612','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +SELECT status FROM test1; +status +0 +0 +0 +0 +0 +UPDATE test1 main +INNER JOIN (SELECT id, +con_bfr, +order_no, +ROW_NUMBER() OVER (PARTITION BY IFNULL(p_date,''), +IFNULL(con_bfr,''), +IFNULL(org_bfr,''), +IFNULL(pat_bfr,''), +IFNULL(data_type,'') +ORDER BY IFNULL(tra_bfr,'') DESC, +IFNULL(load_bfr,'') DESC, +id DESC +) AS row_num +FROM test1 tmp +) tmp +ON main.id = tmp.id +AND main.con_bfr = tmp.con_bfr +AND main.order_no = tmp.order_no +AND tmp.row_num = 1 +SET main.status = 1; +affected rows: 4 +info: Rows matched: 4 Changed: 4 Warnings: 0 +SELECT status FROM test1; +status +1 +0 +1 +1 +1 +DROP DATABASE mcol_4740; diff --git a/mysql-test/columnstore/bugfixes/mcol-4740.test b/mysql-test/columnstore/bugfixes/mcol-4740.test new file mode 100644 index 000000000..85075fd99 --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4740.test @@ -0,0 +1,82 @@ +# +# MCOL-4740 UPDATE involving multi-tables returns wrong "Rows matched" +# + +--source ../include/have_columnstore.inc + +--disable_warnings +DROP DATABASE IF EXISTS mcol_4740; +--enable_warnings +CREATE DATABASE mcol_4740; +USE mcol_4740; + +# Test case 1 from MCOL +CREATE TABLE mcs_1 (a INT, b INT(11), c VARCHAR(100)) engine=columnstore; +CREATE TABLE mcs_2 (a INT, b INT(11), c VARCHAR(100)) engine=columnstore; + +INSERT INTO mcs_1 VALUES (33, 99, 1); +INSERT INTO mcs_1 VALUES (33, 99, 2); +INSERT INTO mcs_1 VALUES (33, 99, 3); +INSERT INTO mcs_2 VALUES (33, 11, 1); +INSERT INTO mcs_2 VALUES (33, 11, 2); +INSERT INTO mcs_2 VALUES (33, 11, 3); + +SELECT * FROM mcs_1; + +--enable_info +UPDATE mcs_1 A, mcs_2 B SET A.b = B.b WHERE A.c = B.c LIMIT 1; +--disable_info + +SELECT * FROM mcs_1; + +--error ER_CHECK_NOT_IMPLEMENTED +UPDATE mcs_1 A, mcs_2 B SET A.b = A.b + 1, B.b = B.b + 1; + +# Test case 2 from MCOL +CREATE TABLE `test1` ( + `id` text NOT NULL, + `p_date` int(11) DEFAULT NULL, + `con_bfr` text DEFAULT NULL, + `org_bfr` text DEFAULT NULL, + `pat_bfr` text DEFAULT NULL, + `data_type` text DEFAULT NULL, + `order_no` text DEFAULT NULL, + `tra_bfr` text DEFAULT NULL, + `load_bfr` text DEFAULT NULL, + `status` int(11) DEFAULT NULL +) ENGINE=Columnstore DEFAULT CHARSET=utf8; + +INSERT INTO test1 values ('index_srgt_id','0','consultation_date_bfr','organization_code_bfr','patient_id_bfr','data_type','order_no','transaction_time_bfr','load_timestamp_bfr','0'); +INSERT INTO test1 values ('adt-a55-9000000001','20190610','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000002','20190610','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000003','20190611','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); +INSERT INTO test1 values ('adt-a55-9000000004','20190612','2.01906E+13','1116508857','PTNT-0001','ADT-01','511','2.01801E+13','2.01801E+13','0'); + +SELECT status FROM test1; + +--enable_info +UPDATE test1 main +INNER JOIN (SELECT id, + con_bfr, + order_no, + ROW_NUMBER() OVER (PARTITION BY IFNULL(p_date,''), + IFNULL(con_bfr,''), + IFNULL(org_bfr,''), + IFNULL(pat_bfr,''), + IFNULL(data_type,'') + ORDER BY IFNULL(tra_bfr,'') DESC, + IFNULL(load_bfr,'') DESC, + id DESC + ) AS row_num + FROM test1 tmp + ) tmp + ON main.id = tmp.id + AND main.con_bfr = tmp.con_bfr + AND main.order_no = tmp.order_no + AND tmp.row_num = 1 +SET main.status = 1; +--disable_info + +SELECT status FROM test1; + +DROP DATABASE mcol_4740; \ No newline at end of file