maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

Hello Alexander,

> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
> Envoyé : lundi 2 octobre 2017 08:21
> À : jerome brauge; 'Sergei Golubchik'
> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx)
> Objet : Re: MDEV-13417 / MDEV - 13418 The order of evaluation of
> SELECT..INTO assignments
> 
> Hello Jerome, Sergei,
> 
> 
> On 09/28/2017 12:30 PM, jerome brauge wrote:
> > Hello Alexander, Sergei,
> > So I split it into two distinct patches.
> > I've used the same idea in these two patches :
> > For each field
> >  - stored the old value in record[1] (done by default for update)
> >  - evaluate fieId (in record[0])
> >  - switch field pointer on record[1]
> > Then restore all field pointer on record[0].
> >
> > Main advantage of this solution : very small cpu overhead.
> > It has only a memory footprint (need to allocate record[1] for
> > m_var_table)
> 
> I have some worries with the current approach with the MDEV-13418 part.
> 
> 1.
> I'm not happy with the fact that you decided to disallow group SET for
> variables.
> Sure, It's OK to disallow setting of the same variable in SET multiple times. But
> completely disallowing group SET sounds too strict.
> 
> 2.
> Also, record[1] size can be huge, especially when you have a procedure with
> many variables and/or parameters.
> You earlier told that you have procedures with 45 parameters (max 296
> parameters).

I agree, especially since we have hundreds of local variables.

> 
> 
> 3. I'm not sure that we need consistency for user variables in SET.
> 
> User variables in SELECT work as follows:
> 
> - the same user variable can be set multiple times in SELECT
> - the old value is immediately thrown away
> 
> SELECT @a,@a:=@a+1,@a, @a:=@a+1, @a;
> +------+----------+------+----------+------+
> | @a   | @a:=@a+1 | @a   | @a:=@a+1 | @a   |
> +------+----------+------+----------+------+
> |    1 |        2 |    2 |        3 |    3 |
> +------+----------+------+----------+------+
> 

But the behavior is not the same if you use group SET.
set @a=0;
set @a=1,@a=@a+1;

MariaDB [(none)]> select @a;
+------+
| @a   |
+------+
|    1 |
+------+

It's done  in :

bool my_var_user::set(THD *thd, Item *item)
{
  Item_func_set_user_var *suv= new (thd->mem_root) Item_func_set_user_var(thd, &name, item);
  suv->save_item_result(item);
  return suv->fix_fields(thd, 0) || suv->update();
}


> 
> 
> I'd propose to try the following way instead:
> 
> 
> At an SP parse time, translate:
> DECLARE v1 INT DEFAULT 10;
> DECLARE v2 INT DEFAULT 20;
> SELECT 1,v1+1 INTO v1, v2;
> 
> to
> 
> DECLARE v1 INT DEFAULT 10;
> DECLARE v2 INT DEFAULT 20;
> BEGIN
>   DECLARE _v1 TYPE OF v1 DEFAULT v1;
>   DECLARE _v2 TYPE OF v2 DEFAULT v2;
>   SELECT 1,_v1+1 INTO v1, v2;
> END;
> 
> 
> 
> Exactly the same way, translate:
> 
> 
> DECLARE v1 INT DEFAULT 10;
> DECLARE v2 INT DEDAULT 20;
> SET v1=1,v2=v1+1;
> 
> to
> 
> DECLARE v1 INT DEFAULT 10;
> DECLARE v2 INT DEDAULT 20;
> BEGIN
>   DECLARE _v1 TYPE OF v1 DEFAULT v1;
>   DECLARE _v2 TYPE OF v2 DEFAULT v2;
>   SET v1=1,v2=_v1+1;
> END
> 
> 
> Advantages:
> - Consistent behavior of SET and SELECT..INTO:
>   no needs to prohibit group SET.
> - No needs for record[1]. Only those variables that are in group SET and
> SELECT..INTO need extra memory in record[0].
> 
> 
> Implementation should be easy:
> - after scanning a group SET or a SELECT..INTO,
>   you create a new sp_pcontext frame
> - add "backup" variables to this frame
> - loop over Items created during SET and SELECt..INTO,
>   and in all Item_splocal replace Item_splocal::m_var_idx
>   to the index of the corresponding "backup" variable.

I had thought of a solution like this, but I didn't know how to implement it.
I will try to follow your suggestion.
Thanks.


> Greetings.
> 
> 
> >
> > Best regards,
> > Jérôme.
> >
> >
> >> -----Message d'origine-----
> >> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : mercredi 27
> >> septembre 2017 15:16 À : jerome brauge; 'Sergei Golubchik'
> >> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx)
> >> Objet : Re: MDEV-13417 / MDEV - 13418 The order of evaluation of
> >> SELECT..INTO assignments
> >>
> >> Hi Jerome,
> >>
> >> Can you please split the patch:
> >> extract and send the patch for MDEV-13417 first.
> >> Sergei will review it.
> >>
> >> As for the MDEV-13418 part, I have an idea how to reuse a lot of code
> >> from MDEV-10591.
> >>
> >> Thanks.
> >>
> >>
> >> On 09/07/2017 06:32 PM, jerome brauge wrote:
> >>> Hello Sergei,
> >>> Here is a patch for MDEV-13417 and MDEV-13418.
> >>> Until you choose the right name for this new sql_mode, I temporarily
> >> chosen "SET_CONSISTENCY".
> >>> It is set only for sql_mode=oracle for now.
> >>>
> >>> Can you review it ?
> >>> Thank you.
> >>> Regard.
> >>>