From 7f456e58cc7ca663ccb5966f927f899daf718931 Mon Sep 17 00:00:00 2001 From: Gagan Goel Date: Tue, 14 Dec 2021 19:25:03 -0500 Subject: [PATCH] MCOL-4868 UPDATE on a ColumnStore table containing an IN-subquery on a non-ColumnStore table does not work. As part of MCOL-4617, we moved the in-to-exists predicate creation and injection from the server into the engine. However, when query with an IN Subquery contains a non-ColumnStore table, the server still performs the in-to-exists predicate transformation for the foreign engine table. This caused ColumnStore's execution plan to contain incorrect WHERE predicates. As a fix, we call mutate_optimizer_flags() for the WRITE lock, in addition to the READ table lock. And in mutate_optimizer_flags(), we change the optimizer flag from OPTIMIZER_SWITCH_IN_TO_EXISTS to OPTIMIZER_SWITCH_MATERIALIZATION. --- dbcon/mysql/ha_mcs_execplan.cpp | 38 +++++++- dbcon/mysql/ha_mcs_impl.cpp | 8 +- dbcon/mysql/ha_mcs_impl_if.h | 12 ++- dbcon/mysql/ha_mcs_pushdown.cpp | 2 +- .../columnstore/bugfixes/mcol-4868.result | 91 +++++++++++++++++++ .../columnstore/bugfixes/mcol-4868.test | 70 ++++++++++++++ 6 files changed, 209 insertions(+), 12 deletions(-) create mode 100644 mysql-test/columnstore/bugfixes/mcol-4868.result create mode 100644 mysql-test/columnstore/bugfixes/mcol-4868.test diff --git a/dbcon/mysql/ha_mcs_execplan.cpp b/dbcon/mysql/ha_mcs_execplan.cpp index ae985c526..9f6087ad5 100755 --- a/dbcon/mysql/ha_mcs_execplan.cpp +++ b/dbcon/mysql/ha_mcs_execplan.cpp @@ -6546,7 +6546,7 @@ bool isForeignTableUpdate(THD* thd) { LEX* lex = thd->lex; - if (lex->sql_command != SQLCOM_UPDATE_MULTI) + if (!isUpdateStatement(lex->sql_command)) return false; Item_field* item; @@ -6561,6 +6561,40 @@ bool isForeignTableUpdate(THD* thd) 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, @@ -6570,7 +6604,7 @@ bool isUpdateHasForeignTable(THD* thd) { LEX* lex = thd->lex; - if (lex->sql_command != SQLCOM_UPDATE_MULTI) + if (!isUpdateStatement(lex->sql_command)) return false; TABLE_LIST* table_ptr = lex->first_select_lex()->get_table_list(); diff --git a/dbcon/mysql/ha_mcs_impl.cpp b/dbcon/mysql/ha_mcs_impl.cpp index a7b4ea018..5661914bb 100644 --- a/dbcon/mysql/ha_mcs_impl.cpp +++ b/dbcon/mysql/ha_mcs_impl.cpp @@ -2580,7 +2580,7 @@ int ha_mcs_impl_rnd_next(uchar* buf, TABLE* table) thd->lex->sql_command == SQLCOM_LOAD)) return HA_ERR_END_OF_FILE; - if (isUpdateOrDeleteStatement(thd->lex->sql_command, !isForeignTableUpdate(thd))) + if (isMCSTableUpdate(thd) || isMCSTableDelete(thd)) return HA_ERR_END_OF_FILE; // @bug 2547 @@ -2674,7 +2674,7 @@ int ha_mcs_impl_rnd_end(TABLE* table, bool is_pushdown_hand) if ( (thd->lex)->sql_command == SQLCOM_ALTER_TABLE ) return rc; - if (isUpdateOrDeleteStatement(thd->lex->sql_command, !isForeignTableUpdate(thd))) + if (isMCSTableUpdate(thd) || isMCSTableDelete(thd)) return rc; if (!ci) @@ -4010,7 +4010,7 @@ int ha_mcs::impl_external_lock(THD* thd, TABLE* table, int lock_type) } else { - if (lock_type == 0) + if ((lock_type == 0) || (lock_type == 1)) { ci->physTablesList.insert(table); // MCOL-2178 Disable Conversion of Big IN Predicates Into Subqueries @@ -4538,7 +4538,7 @@ int ha_mcs_impl_group_by_next(TABLE* table) thd->lex->sql_command == SQLCOM_LOAD)) return HA_ERR_END_OF_FILE; - if (isUpdateOrDeleteStatement(thd->lex->sql_command, !isForeignTableUpdate(thd))) + if (isMCSTableUpdate(thd) || isMCSTableDelete(thd)) return HA_ERR_END_OF_FILE; if (get_fe_conn_info_ptr() == nullptr) diff --git a/dbcon/mysql/ha_mcs_impl_if.h b/dbcon/mysql/ha_mcs_impl_if.h index f8556bdfb..3c2be9709 100644 --- a/dbcon/mysql/ha_mcs_impl_if.h +++ b/dbcon/mysql/ha_mcs_impl_if.h @@ -368,6 +368,8 @@ 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, bool isRefItem = false); @@ -411,10 +413,10 @@ bool buildEqualityPredicate(execplan::ReturnedColumn* lhs, const std::vector& itemList, bool isInSubs = false); -inline bool isUpdateStatement(const enum_sql_command& command, const bool isMCSTableUpdate = true) +inline bool isUpdateStatement(const enum_sql_command& command) { - return (command == SQLCOM_UPDATE) || - (command == SQLCOM_UPDATE_MULTI && isMCSTableUpdate); + return ((command == SQLCOM_UPDATE) || + (command == SQLCOM_UPDATE_MULTI)); } inline bool isDeleteStatement(const enum_sql_command& command) @@ -423,9 +425,9 @@ inline bool isDeleteStatement(const enum_sql_command& command) (command == SQLCOM_DELETE_MULTI); } -inline bool isUpdateOrDeleteStatement(const enum_sql_command& command, const bool isMCSTableUpdate = true) +inline bool isUpdateOrDeleteStatement(const enum_sql_command& command) { - return isUpdateStatement(command, isMCSTableUpdate) || + return isUpdateStatement(command) || isDeleteStatement(command); } diff --git a/dbcon/mysql/ha_mcs_pushdown.cpp b/dbcon/mysql/ha_mcs_pushdown.cpp index 67a0cde13..c4bac1fe7 100644 --- a/dbcon/mysql/ha_mcs_pushdown.cpp +++ b/dbcon/mysql/ha_mcs_pushdown.cpp @@ -36,7 +36,7 @@ void mutate_optimizer_flags(THD *thd_) // CS restores it later in SH::scan_end() and in case of an error // in SH::scan_init() - ulonglong flags_to_set = OPTIMIZER_SWITCH_IN_TO_EXISTS | + ulonglong flags_to_set = OPTIMIZER_SWITCH_MATERIALIZATION | OPTIMIZER_SWITCH_COND_PUSHDOWN_FOR_DERIVED | OPTIMIZER_SWITCH_COND_PUSHDOWN_FROM_HAVING; diff --git a/mysql-test/columnstore/bugfixes/mcol-4868.result b/mysql-test/columnstore/bugfixes/mcol-4868.result new file mode 100644 index 000000000..dc3a2b981 --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4868.result @@ -0,0 +1,91 @@ +DROP DATABASE IF EXISTS `mcol_4868`; +CREATE DATABASE `mcol_4868`; +USE `mcol_4868`; +CREATE USER IF NOT EXISTS'cejuser'@'localhost' IDENTIFIED BY 'Vagrant1|0000001'; +GRANT ALL PRIVILEGES ON *.* TO 'cejuser'@'localhost'; +FLUSH PRIVILEGES; +CREATE TABLE test_cs (a INT, b VARCHAR(100)) ENGINE=COLUMNSTORE; +INSERT INTO test_cs VALUES (1,'Test1'), (2,'Test2'), (3,'Test3'), (4,'Test4'); +INSERT INTO test_cs VALUES (null,'TestNULL'), (6,NULL), (7,'Test7'); +CREATE TABLE test_innodb (a INT, b VARCHAR(100)); +INSERT INTO test_innodb VALUES (1,'innodb1'), (2,'innodb2'), (3,'innodb3'), (5, 'innodb5'); +SELECT * FROM test_cs; +a b +1 Test1 +2 Test2 +3 Test3 +4 Test4 +NULL TestNULL +6 NULL +7 Test7 +SELECT * FROM test_innodb; +a b +1 innodb1 +2 innodb2 +3 innodb3 +5 innodb5 +SELECT * FROM test_cs WHERE a IN (SELECT a FROM test_innodb); +a b +1 Test1 +2 Test2 +3 Test3 +UPDATE test_cs SET b='Update_cs' WHERE a IN (SELECT a FROM test_innodb); +SELECT * FROM test_cs; +a b +1 Update_cs +2 Update_cs +3 Update_cs +4 Test4 +NULL TestNULL +6 NULL +7 Test7 +SELECT * FROM test_innodb WHERE a IN (SELECT a FROM test_cs); +a b +1 innodb1 +2 innodb2 +3 innodb3 +UPDATE test_innodb SET b='Update_inno' WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_innodb; +a b +1 Update_inno +2 Update_inno +3 Update_inno +5 innodb5 +SELECT * FROM test_cs WHERE a IN (SELECT a FROM test_cs); +a b +1 Update_cs +2 Update_cs +3 Update_cs +4 Test4 +6 NULL +7 Test7 +UPDATE test_cs SET b='Update_cs2' WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_cs; +a b +1 Update_cs2 +2 Update_cs2 +3 Update_cs2 +4 Update_cs2 +NULL TestNULL +6 Update_cs2 +7 Update_cs2 +DELETE FROM test_cs WHERE a IN (SELECT a FROM test_innodb); +SELECT * FROM test_cs; +a b +4 Update_cs2 +NULL TestNULL +6 Update_cs2 +7 Update_cs2 +DELETE FROM test_cs; +INSERT INTO test_cs VALUES (1,'Test1'), (2,'Test2'), (3,'Test3'), (4,'Test4'); +INSERT INTO test_cs VALUES (null,'TestNULL'), (6,NULL), (7,'Test7'); +DELETE FROM test_innodb WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_innodb; +a b +5 innodb5 +DELETE FROM test_cs WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_cs; +a b +NULL TestNULL +DROP USER 'cejuser'@'localhost'; +DROP DATABASE `mcol_4868`; diff --git a/mysql-test/columnstore/bugfixes/mcol-4868.test b/mysql-test/columnstore/bugfixes/mcol-4868.test new file mode 100644 index 000000000..10caf7f6a --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4868.test @@ -0,0 +1,70 @@ +# +# MCOL-4868 UPDATE on a ColumnStore table containing an IN-subquery +# on a non-ColumnStore table does not work. +# + +-- source include/have_innodb.inc +-- source ../include/have_columnstore.inc + +if (!$MASTER_MYPORT) +{ + # Running with --extern + let $MASTER_MYPORT=`SELECT @@port`; +} + +--disable_warnings +DROP DATABASE IF EXISTS `mcol_4868`; +--enable_warnings +CREATE DATABASE `mcol_4868`; +USE `mcol_4868`; + +# +# Enable cross engine join +# Configure user and password in Columnstore.xml file +# +--exec $MCS_MCSSETCONFIG CrossEngineSupport User 'cejuser' +--exec $MCS_MCSSETCONFIG CrossEngineSupport Password 'Vagrant1|0000001' +--exec $MCS_MCSSETCONFIG CrossEngineSupport Port $MASTER_MYPORT +# +# Create corresponding in the server +# +--disable_warnings +CREATE USER IF NOT EXISTS'cejuser'@'localhost' IDENTIFIED BY 'Vagrant1|0000001'; +--enable_warnings +GRANT ALL PRIVILEGES ON *.* TO 'cejuser'@'localhost'; +FLUSH PRIVILEGES; + +CREATE TABLE test_cs (a INT, b VARCHAR(100)) ENGINE=COLUMNSTORE; +INSERT INTO test_cs VALUES (1,'Test1'), (2,'Test2'), (3,'Test3'), (4,'Test4'); +INSERT INTO test_cs VALUES (null,'TestNULL'), (6,NULL), (7,'Test7'); +CREATE TABLE test_innodb (a INT, b VARCHAR(100)); +INSERT INTO test_innodb VALUES (1,'innodb1'), (2,'innodb2'), (3,'innodb3'), (5, 'innodb5'); +SELECT * FROM test_cs; +SELECT * FROM test_innodb; + +SELECT * FROM test_cs WHERE a IN (SELECT a FROM test_innodb); +UPDATE test_cs SET b='Update_cs' WHERE a IN (SELECT a FROM test_innodb); +SELECT * FROM test_cs; + +SELECT * FROM test_innodb WHERE a IN (SELECT a FROM test_cs); +UPDATE test_innodb SET b='Update_inno' WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_innodb; + +SELECT * FROM test_cs WHERE a IN (SELECT a FROM test_cs); +UPDATE test_cs SET b='Update_cs2' WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_cs; + +# Test DELETEs +DELETE FROM test_cs WHERE a IN (SELECT a FROM test_innodb); +SELECT * FROM test_cs; +DELETE FROM test_cs; +INSERT INTO test_cs VALUES (1,'Test1'), (2,'Test2'), (3,'Test3'), (4,'Test4'); +INSERT INTO test_cs VALUES (null,'TestNULL'), (6,NULL), (7,'Test7'); +DELETE FROM test_innodb WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_innodb; +DELETE FROM test_cs WHERE a IN (SELECT a FROM test_cs); +SELECT * FROM test_cs; + +# Cleanup +DROP USER 'cejuser'@'localhost'; +DROP DATABASE `mcol_4868`;