On Wed, Dec 26, 2018 at 8:53 AM Sergei Golubchik wrote:

Hi, Nikita!

On Dec 25, Nikita Malyavin wrote:

...

revision-id: 773d24d457a (versioning-1.0.6-100-g773d24d457a) parent(s): b26736cdb11 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2018-12-21 03:13:47 +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()`

diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 9e6c333d3c9..48bcb828d56 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -341,7 +341,16 @@ static bool set_up_field_array(THD *thd, TABLE *table, while ((field= *(ptr++))) { if (field->flags & GET_FIXED_FIELDS_FLAG) + { + if (table->versioned(VERS_TRX_ID) + && unlikely(field->flags & VERS_SYSTEM_FIELD)) + { + my_error(ER_VERS_NOT_SUPPORTED, MYF(0), + "Using ROW START/END for patitioning", "transactional");

This is incorrect usage of error message parameters. The error message can be translated, parameters can be reordered - you should not use English as parameter values. Consider:

transactional cистемно-версионированные таблицы не поддерживают Using ROW START/END for patitioning

But looks like ER_VERS_NOT_SUPPORTED is not used anywhere at the moment. So you can change it to be anything you want, even remove all parameters:

Transactional system versioned tables do not support partitioning by ROW START or ROW END

Done as well.

...

+ DBUG_RETURN(TRUE); + } num_fields++; + } } if (unlikely(num_fields > MAX_REF_PARTS)) { diff --git a/sql/table.cc b/sql/table.cc index ce7a34a8fe2..0c2ff1eea5a 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1776,7 +1776,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, goto err; DBUG_PRINT("info", ("Columns with system versioning: [%d, %d]", row_start, row_end)); versioned= VERS_TIMESTAMP; - vers_can_native= plugin_hton(se_plugin)->flags & HTON_NATIVE_SYS_VERSIONING; + vers_can_native= handler_file->vers_can_native(thd);

Interesting. How does that work? Your ha_partition::vers_can_native() is

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;

But it doesn't look like thd->lex->part_info->part_type is initialized at this point in time. Try to add to your test case

flush tables; select * from t1;

Yes, part_info is going to be filled only if creating, or altering table. It works as follows: If we're creating/altering, then check thd->lex->part_info; else, check m_file array, which is initialized at that point.

Added this as the comment for clarity. -- Yours truly, Nikita Malyavin