Hi, Aleksey! Looks ok, thanks! No requests to change anything, but still a couple of "why" questions, see below On Jun 07, Aleksey Midenkov wrote:

revision-id: 66f931bcd69 (mariadb-10.3.12-100-g66f931bcd69) parent(s): f4484dfdbf2 author: Aleksey Midenkov committer: Aleksey Midenkov timestamp: 2019-03-26 15:04:52 +0300 message:

MDEV-18727 improve DML operation of System Versioning MDEV-18957 UPDATE with LIMIT clause is wrong for versioned partitioned tables

UPDATE, DELETE: replace linear search of current/historical records with vers_setup_conds().

diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index b8ed887b52d..a00cc2a8fb0 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4975,8 +4975,10 @@ mysql_execute_command(THD *thd) { result= new (thd->mem_root) multi_delete(thd, aux_tables, lex->table_count); - if (unlikely(result)) + if (likely(result))

heh, good catch

{ + if (unlikely(select_lex->vers_setup_conds(thd, aux_tables))) + goto multi_delete_error; res= mysql_select(thd, select_lex->get_table_list(), select_lex->with_wild, diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 6915d4d23ca..28b54e0b3b1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -779,20 +781,35 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) } }

+ bool is_select= false; + switch (thd->lex->sql_command) + { + case SQLCOM_SELECT: + case SQLCOM_INSERT_SELECT: + case SQLCOM_REPLACE_SELECT: + case SQLCOM_DELETE_MULTI: + case SQLCOM_UPDATE_MULTI: + is_select= true; + default: + break; + } + for (table= tables; table; table= table->next_local) { - if (!table->table || !table->table->versioned()) + if (!table->table || table->is_view() || !table->table->versioned())

why?

continue;

vers_select_conds_t &vers_conditions= table->vers_conditions;

#ifdef WITH_PARTITION_STORAGE_ENGINE - /* - if the history is stored in partitions, then partitions - themselves are not versioned - */ - if (table->partition_names && table->table->part_info->vers_info) + Vers_part_info *vers_info; + if (is_select && table->table->part_info && + (vers_info= table->table->part_info->vers_info)) + { + if (table->partition_names)

Why is that? You don't seem to use vers_info anywhere. Nor I see a reason for two nested if()'s intead of one, as before (with an additional `is_select &&` condition)

{ + /* If the history is stored in partitions, then partitions + themselves are not versioned. */ if (vers_conditions.is_set()) { my_error(ER_VERS_QUERY_IN_PARTITION, MYF(0), table->alias.str);

Regards, Sergei Chief Architect MariaDB and security@mariadb.org