[Maria-developers] MDEV-13417 / MDEV - 13418 The order of evaluation of SELECT..INTO assignments
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.
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.
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) 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.
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). 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 | +------+----------+------+----------+------+ 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. 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.
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.
Hello Jerome, On 10/02/2017 01:09 PM, jerome brauge wrote:
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(); }
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? 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@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.
Hello Alexander,
-----Message d'origine----- De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 2 octobre 2017 13:41 À : jerome brauge Cc : MariaDB Developers (maria-developers@lists.launchpad.net); 'Sergei Golubchik' Objet : Re: MDEV-13417 / MDEV - 13418 The order of evaluation of SELECT..INTO assignments
Hello Jerome,
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
On 10/02/2017 01:09 PM, jerome brauge wrote: 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@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.
participants (2)
-
Alexander Barkov -
jerome brauge