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:[email protected]] > Envoyé : vendredi 3 novembre 2017 11:02 > À : jerome brauge > Cc : MariaDB Developers ([email protected]) > Objet : Re: MDEV-13418 Compatibility: The order of evaluation of > SELECT..INTO assignments > > Hello Jerome, > > > On 11/03/2017 12:22 PM, jerome brauge wrote: > > 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 > 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:[email protected]] 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. > >>>> _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
