Hi, Nikita!
On Nov 12, Nikita Malyavin wrote:
> revision-id: bc7d600a27f4c6c14deea5f1d674c621f85b23ba
> (versioning-1.0.5-141-gbc7d600a27f)
> parent(s): 90b292ce31170be33796b07723ffcef56bf8a679
> author: Nikita Malyavin <nikitamalya...@gmail.com>
> committer: Nikita Malyavin <nikitamalya...@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 secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
