← Back to team overview

maria-developers team mailing list archive

Re: MDEV-13417 / MDEV - 13418 The order of evaluation of SELECT..INTO assignments

 

Hello Alexander,

> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
> Envoyé : lundi 2 octobre 2017 13:41
> À : jerome brauge
> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx); 'Sergei
> Golubchik'
> Objet : Re: MDEV-13417 / MDEV - 13418 The order of evaluation of
> SELECT..INTO assignments
> 
> Hello Jerome,
> 
> 
> On 10/02/2017 01:09 PM, jerome brauge wrote:
> > 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(); }
> 
> Oops. Right, SET is implemented differently than SELECT @a:=@a+1.
> I didn't know. Let's leave the question with user variables open for now.
> 
> >
> >> 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 for being flexible!
> 
> Note, "var TYPE OF var1" is not actually implemented yet.
> Only "var TYPE OF tab1.col1" is implemented.
> 
> I can do "var TYPE OF var1" first, as a separate patch, as we'll need it for
> Oracle compatibility anyway.
> Would you like me to?
> 

Yes, It will be easier for me after that.
Thank.

> 
> The tricky part here is to make this work correctly when an anchored type
> variable refers to another anchored variable.
> 
> Some examples:
> 
> 1. Scalar variables
> DECLARE v1 TYPE OF t1.col1;
> DECLARE v2 TYPE OF v1;
> DECLARE v3 TYPE OF v2;
> 
> 2. Explicit ROW anchor
> 
> - a. Entire explicit ROW anchor
> 
> DECLARE v1 ROW (a INT, b VARCHAR(10));
> DECLARE v2 TYPE OF v1;
> 
> - b. Explicit ROW field anchor
> 
> DECLARE v1 ROW (a INT, b VARCHAR(10));
> DECLARE v2 TYPE OF v1.a;
> 
> 
> 3. Table ROW anchor
> 
> - a. Entire table ROW anchor
> 
> DECLARE v1 ROWTYPE OF t1;
> DECLARE v2 TYPE OF v1;
> 
> - b. Table ROW field anchor
> 
> DECLARE v1 ROWTYPE OF t1;
> DECLARE v2 TYPE OF v1.col1;
> 
> 4. Cursor ROW anchor
> (similar to #4, but with v1 being a "ROWTYPE OF cursor1".
> 
> and so on.
> 
> 
> Thanks.
> 
> 
> > 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.
> >>>>>


References