Re: [Maria-developers] 773d24d457a: MDEV-15951 system versioning by trx id doesn't work with partitioning
> > +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.
>
No, have rewritten it in another style. Should be sane enough now.
Is it?
--
Yours truly,
Nikita Malyavin
___
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
Re: [Maria-developers] 773d24d457a: MDEV-15951 system versioning by trx id doesn't work with partitioning
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
___
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
Re: [Maria-developers] 773d24d457a: MDEV-15951 system versioning by trx id doesn't work with partitioning
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
> +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 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
