Re: [Maria-developers] MDEV-13417 / MDEV - 13418 The order of evaluation of SELECT..INTO assignments

2 Oct 2017

      Hello Alexander,
...
-----Message d'origine-----
De : Alexander Barkov [mailto:bar@mariadb.org]
Envoyé : lundi 2 octobre 2017 08:21
À : jerome brauge; 'Sergei Golubchik'
Cc : MariaDB Developers (maria-developers@lists.launchpad.net)
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@mariadb.org] Envoyé : mercredi 27
septembre 2017 15:16 À : jerome brauge; 'Sergei Golubchik'
Cc : MariaDB Developers (maria-developers@lists.launchpad.net)
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.