Hi, Nikita! On Nov 12, Nikita Malyavin wrote:
revision-id: bc7d600a27f4c6c14deea5f1d674c621f85b23ba (versioning-1.0.5-141-gbc7d600a27f) parent(s): 90b292ce31170be33796b07723ffcef56bf8a679 author: Nikita Malyavin <nikitamalyavin@gmail.com> committer: Nikita Malyavin <nikitamalyavin@gmail.com> timestamp: 2018-09-21 22:42:03 +1000 message:
MDEV-15951 system versioning by trx id doesn't work with partitioning
Fix partitioning for trx_id-versioned tables. `partition by hash`, `range` and others now work. `partition by system_time` is forbidden. Currently we cannot use row_start and row_end in `partition by`, because insertion of versioned field is done by engine's handler, as well as row_start/row_end's value set up, which is a transaction id -- so it's also forbidden.
The drawback is that it's now impossible to use `partition by key()` without parameters for such tables, because it references row_start and row_end implicitly.
* add handler::vers_can_native() * drop Table_scope_and_contents_source_st::vers_native() * drop partition_element::find_engine_flag as unused * forbid versioning partitioning for trx_id as not supported * adopt vers tests for trx_id partitioning * forbid any row_end referencing in `partition by` clauses, including implicit `by key()`
--- mysql-test/suite/versioning/r/partition.result | 79 ++++++++++++++++-- .../suite/versioning/t/partition.combinations | 3 + mysql-test/suite/versioning/t/partition.test | 97 ++++++++++++++++++++-- sql/ha_partition.h | 10 +++ sql/handler.cc | 34 ++------ sql/handler.h | 8 +- sql/partition_element.h | 15 ---- sql/sql_class.h | 4 +- sql/sql_partition.cc | 9 ++ sql/sql_table.cc | 6 +- sql/table.cc | 4 +- sql/temporary_tables.cc | 10 ++- 12 files changed, 214 insertions(+), 65 deletions(-)
diff --git a/mysql-test/suite/versioning/r/partition.result b/mysql-test/suite/versioning/r/partition.result index bfec0ce2d4b..cf4203b7949 100644 --- a/mysql-test/suite/versioning/r/partition.result +++ b/mysql-test/suite/versioning/r/partition.result @@ -1,6 +1,6 @@ set system_versioning_alter_history=keep; # Check conventional partitioning on temporal tables -create table t1 (x int) +create or replace table t1 (x int, ROW_START, ROW_END, period for system_time(row_start, row_end))
ouch. that looks very weird. please, only replace the type, not the whole column definition. Like everywhere else: --replace_result $sys_datatype_expl SYS_DATATYPE
with system versioning partition by range columns (x) ( partition p0 values less than (100), @@ -501,6 +504,72 @@ set timestamp=1523466002.799571; insert into t1 values (11),(12); set timestamp=1523466004.169435; delete from t1 where pk in (11, 12); +# MDEV-15951 system versioning by trx id doesn't work with partitioning +# currently trx_id does not support partitioning by system_time +create or replace table t1( +i int, +row_start bigint unsigned generated always as row start, +row_end bigint unsigned generated always as row end, +period for system_time(row_start, row_end) +) engine=InnoDB with system versioning partition by system_time (
if you have tests with innodb and trx_id, please, put them in a separate test file, say, partition_innodb.test. partition.test is for common tests that are repeated three times, with myisam, innodb, timestamps, and trx_id. There is no need to repeat your test three times.
diff --git a/mysql-test/suite/versioning/t/partition.combinations b/mysql-test/suite/versioning/t/partition.combinations index 4d73ef5a5ea..561c5656929 100644 --- a/mysql-test/suite/versioning/t/partition.combinations +++ b/mysql-test/suite/versioning/t/partition.combinations @@ -1,5 +1,8 @@ [timestamp] default-storage-engine=innodb
+[trx_id] +default-storage-engine=innodb + [myisam] default-storage-engine=myisam
But then you don't need partition.combinations at all, just add --source engines.inc like other tests do.
diff --git a/sql/ha_partition.h b/sql/ha_partition.h index 8a251016703..ef49011b7f8 100644 --- a/sql/ha_partition.h +++ b/sql/ha_partition.h @@ -413,6 +413,16 @@ class ha_partition :public handler
virtual void return_record_by_parent();
+ virtual bool vers_can_native(THD *thd) + { + // PARTITION BY SYSTEM_TIME is not supported for now + bool can= !thd->lex->part_info + || thd->lex->part_info->part_type != VERSIONING_PARTITION; + for (uint i= 0; i < m_tot_parts && can; i++) + can= can && m_file[i]->vers_can_native(thd); + return can; + } + /* ------------------------------------------------------------------------- MODULE create/delete handler object diff --git a/sql/handler.cc b/sql/handler.cc index 306b0868d15..a7962039463 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -4938,6 +4938,7 @@ int ha_create_table(THD *thd, const char *path, !create_info->tmp_table();
share.frm_image= frm; + share.orig_table_name= table_name;
why? if orig_table_name is NULL, error_table_name() will use table_name, so there's never need to make orig_table_name==table_name. and I don't understand all your other manipulations with orig_table_name. Could you please explain that?
// open an frm image if (share.init_from_binary_frm_image(thd, write_frm_now, @@ -6952,29 +6953,6 @@ bool Vers_parse_info::fix_implicit(THD *thd, Alter_info *alter_info) return false; }
-bool Table_scope_and_contents_source_st::vers_native(THD *thd) const -{ - if (ha_check_storage_engine_flag(db_type, HTON_NATIVE_SYS_VERSIONING)) - return true; - -#ifdef WITH_PARTITION_STORAGE_ENGINE - partition_info *info= thd->work_part_info; - if (info && !(used_fields & HA_CREATE_USED_ENGINE)) - { - if (handlerton *hton= info->default_engine_type) - return ha_check_storage_engine_flag(hton, HTON_NATIVE_SYS_VERSIONING); - - List_iterator_fast<partition_element> it(info->partitions); - while (partition_element *partition_element= it++) - { - if (partition_element->find_engine_flag(HTON_NATIVE_SYS_VERSIONING)) - return true; - } - } -#endif - return false; -}
what was wrong with that (and all other changed code in this file) ?
bool Table_scope_and_contents_source_st::vers_fix_system_fields( THD *thd, Alter_info *alter_info, const TABLE_LIST &create_table, bool create_select)
Regards, Sergei Chief Architect MariaDB and security@mariadb.org