Hi Alexander, Thanks for this idea, but for us, cpu is a critical hotspot. I will evaluate how many statements in our application are affected. If this number is not to high, we modify them. In this case, we will need two sql_mode (one for MDEV-13417 and one for MDEV-13418). Best regards. Jérôme.
-----Message d'origine----- De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : vendredi 3 novembre 2017 11:02 À : jerome brauge Cc : MariaDB Developers (maria-developers@lists.launchpad.net) Objet : Re: MDEV-13418 Compatibility: The order of evaluation of SELECT..INTO assignments
Hello Jerome,
Hello Alexander. I've begun to implement your proposal but now I'm not sure that it's a better solution than mine. Let me explain . - first : number of temporary variables can be significant because we don't know when they are really needed and their scope are local to the statement.
declare b1 INTEGER; declare res INTEGER; ... if b1 = 0 then select 1,b1+1 into b1, res from dual; end if; if b1 = 1 then select 2,b1+2 into b1, res from dual; end if;
will be transform in :
declare b1 INTEGER; declare res INTEGER; ... if b1 = 0 then declare _b1 INTEGER default res; declare _res INTEGER default res; select 1,b1+1 into _b1, _res from dual; set b1=_b1; set res=_res; -- _res is not needed, but we don't know because the select statement is not parsed end if; if b1 = 1 then declare _b1 INTEGER default res; declare _res INTEGER default res; select 2,b1+2 into b1, res from dual; set b1=_b1; set res=_res; -- same thing here, and we have declare two variables for each target variables end if;
Perhaps we could - declare these temporary variables only one time in the first frame of the stored procedure (may be tricky) - parse columns of each select to know what variables are really assigned and reused (heavy cost in cpu and time)
- second : if we can't determine variables which must have a temporary variable, number of sp_instr_set and sp_instr_set_var will be too high and
On 11/03/2017 12:22 PM, jerome brauge wrote: their cpu cost is not acceptable.
My first solution has a fixed memory impact (and memory is not an issue
nowadays), and especially a very light cpu cost.
What do you think about ?
I agree that determining variables which must have a temporary pair by full scanning the SELECT list expressions is probably not a good idea.
Declaring these variables only one time is easy. I earlier made about the same trick with cursor parameters. See sp_pcontext::retrieve_field_definitions().
The idea is that we can put such backup variables into a separate child sp_pcontext frame, to make sure that only one backup variable exists if the real variable appears multiple time as a SELECT INTO target. Having a dedicated frame allows: - to add variables in the middle of a BEGIN..END block, without having to re- enumerate local variables of the same block. - handle unique names
See the comment in sp_pcontext::retrieve_field_definitions() about frame positions and run-time positions, and the CURSOR declaration grammar in sql_yacc_ora.yy.
Every sp_pcontext frame should have one sp_pcontext frame for backup variables, which should be put into m_children.
sp_pcontext should probably have a new flag: bool m_is_backup; So we can iterate through m_children and find the backup frame.
Another option is to add:
int m_backup_child_context_index;
which will store -1 if the current frame does not have a backup child yet, or a non-negative value meaning the index in m_children. So a new method could looks like this:
sp_pcontext *get_backup_context() { if (m_backup_child_contex_index < 0) return 0; return m_children(m_backup_child_context_index); }
Regard, Jérôme.
-----Message d'origine----- De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 30 octobre 2017 10:02 À : jerome brauge Objet : Re: MDEV-14139 Anchored data types for variables
On 10/28/2017 07:29 PM, Alexander Barkov wrote:
On 10/27/2017 10:27 PM, Alexander Barkov wrote:
Hello Jerome,
I have implemented "MDEV-14139 Anchored data types for variables". and pushed it to bb-10.2-ext:
https://github.com/MariaDB/server/commit/5dd5253f7e50c21fa758e2eb58f
3
aa9c9754e733
So now it should be easier to implement consistent SET by creating backup variables.
LEX::sp_variable_declarations_vartype_finalize() implements the logic which copies data type from another variable.
The idea is that for all variables, which are assignment targets in a SET or a SELECT INTO statement, we create a backup variable.
It will involve these calls for every such variable:
LEX::sp_variable_declarations_init(thd, 1); sp_pcontext::add_variable(thd, backup_variable_name); LEX::sp_variable_declarations_vartype_finalize(thd, 1, orig_variable_name, def);
where "def" is Item_splocal created for the original variable.
Just an idea: instead of creating sp_instr_set, it's easier to introduce a new class sp_instr_set_var, to copy the value from one variable to another variable.
This operation will not need neither Item, nor sp_lex_keeper. It will only need two offsets: - the source variable offset and - the target variable offset.
Using these offsets, we can access to spcont->m_var_table->field[source] and spcont->spcont->m_var_table->field[target] and copy the value between them using Field::store_field().
This won't however for the ROW variables at the moment, because ROW fields are stored in the Item_spvar_args::m_table member of Item_field_row.
It seems we need a new class Field_row and move Virtual_tmp_table from Item_field_row to Field_row.
I will try to do it.
I have implemented "MDEV-14212 Add Field_row for SP ROW variables" and pushed to bb-10.2-ext.
Also, added a comment:
MDEV-13418 Compatibility: The order of evaluation of SELECT..INTO assignments
Now, when MDEV-14212 is done, these declarations:
DECLARE a_tmp TYPE OF a DEFAULT a; DECLARE b_tmp TYPE OF b DEFAULT b;
and these assignments:
SET a=a_tmp; SET b=b_tmp;
can use a new sp_instr_setvar instead of sp_inst_set.
The new command sp_instr_setvar can do direct copying between two spcont->m_var_table->field[XXX], without a need to create Item and LEX.
Greetings.