maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei!

On Fri, Jun 7, 2019 at 8:45 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> 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:
> >
...
> > 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?

VIEW conditions are processed inside mysql_derived_prepare(). Btw,
this patch was wrong about VIEW update since there was no appropriate
cases in view.test.
New patch is here:
https://github.com/MariaDB/server/pull/1332/commits/a65299bb284636dfac3530e815871edd4da27dd2

as well as additional test cases. And I attached just a fix for VIEW
update bug in this letter.

>
> >        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)

Looks like an outdated patch. In new patch there is branch:

      else if (!vers_conditions.is_set())
      {

But I removed vers_info variable as it doesn't help anything.

>
> >        {
> > +        /* 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



-- 
All the best,

Aleksey Midenkov
@midenok

Attachment: view_fix.diff.gz
Description: application/gzip