maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey!

Looks ok, thanks!
No requests to change anything,
but still a couple of "why" questions, see below

On Jun 07, Aleksey Midenkov wrote:
> revision-id: 66f931bcd69 (mariadb-10.3.12-100-g66f931bcd69)
> parent(s): f4484dfdbf2
> author: Aleksey Midenkov <midenok@xxxxxxxxx>
> committer: Aleksey Midenkov <midenok@xxxxxxxxx>
> timestamp: 2019-03-26 15:04:52 +0300
> message:
> 
> MDEV-18727 improve DML operation of System Versioning
> MDEV-18957 UPDATE with LIMIT clause is wrong for versioned partitioned tables
> 
> UPDATE, DELETE: replace linear search of current/historical records
> with vers_setup_conds().
> 
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index b8ed887b52d..a00cc2a8fb0 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -4975,8 +4975,10 @@ mysql_execute_command(THD *thd)
>      {
>        result= new (thd->mem_root) multi_delete(thd, aux_tables,
>                                                 lex->table_count);
> -      if (unlikely(result))
> +      if (likely(result))

heh, good catch

>        {
> +        if (unlikely(select_lex->vers_setup_conds(thd, aux_tables)))
> +          goto multi_delete_error;
>          res= mysql_select(thd,
>                            select_lex->get_table_list(),
>                            select_lex->with_wild,
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index 6915d4d23ca..28b54e0b3b1 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -779,20 +781,35 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables)
>      }
>    }
>  
> +  bool is_select= false;
> +  switch (thd->lex->sql_command)
> +  {
> +  case SQLCOM_SELECT:
> +  case SQLCOM_INSERT_SELECT:
> +  case SQLCOM_REPLACE_SELECT:
> +  case SQLCOM_DELETE_MULTI:
> +  case SQLCOM_UPDATE_MULTI:
> +    is_select= true;
> +  default:
> +    break;
> +  }
> +
>    for (table= tables; table; table= table->next_local)
>    {
> -    if (!table->table || !table->table->versioned())
> +    if (!table->table || table->is_view() || !table->table->versioned())

why?

>        continue;
>  
>      vers_select_conds_t &vers_conditions= table->vers_conditions;
>  
>  #ifdef WITH_PARTITION_STORAGE_ENGINE
> -      /*
> -        if the history is stored in partitions, then partitions
> -        themselves are not versioned
> -      */
> -      if (table->partition_names && table->table->part_info->vers_info)
> +    Vers_part_info *vers_info;
> +    if (is_select && table->table->part_info &&
> +        (vers_info= table->table->part_info->vers_info))
> +    {
> +      if (table->partition_names)

Why is that? You don't seem to use vers_info anywhere.
Nor I see a reason for two nested if()'s intead of one, as before
(with an additional `is_select &&` condition)

>        {
> +        /* If the history is stored in partitions, then partitions
> +            themselves are not versioned. */
>          if (vers_conditions.is_set())
>          {
>            my_error(ER_VERS_QUERY_IN_PARTITION, MYF(0), table->alias.str);

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx