maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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:bar@xxxxxxxxxxx]
>> 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.
>>>>