maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Nikita!

On Dec 25, Nikita Malyavin wrote:
> revision-id: 773d24d457a (versioning-1.0.6-100-g773d24d457a)
> parent(s): b26736cdb11
> author: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> committer: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> 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

> +        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;

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx