Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2746) Bug#38426

[Jørgen Løland]

Rafal Somla wrote:
> Hi Jorgen,
> 
> STATUS
> ------
> Patch approved.
> 
> COMMENTARY
> ----------
> See minor remarks/suggestions below.
> 
>> +--echo #
>> +--echo # Show drivers. +
>> +--echo
>> +# Count of expected driver for this backup_id should be 1
>> +# Replace exp_drv and backup_id in query because it will vary
>> +replace_regex /'.*'.*/'FILTERED DRIVER' AND backup_id=FILTERED ID/ ;
>> +--eval SELECT count(*) FROM mysql.backup_history WHERE drivers LIKE 
>> '$exp_drv' AND backup_id=$bup_id
> 
> You could do it without "replace_regex" as follows:
> 
> --eval SELECT drivers LIKE '$exp_drv' FROM mysql.backup_history
>                                       WHERE backup_id=$bup_id;
> 
> or even:
> 
> --eval SELECT drivers = '$exp_drv' FROM mysql.backup_history
>                                    WHERE backup_id=$bup_id;

I don't think I can - the regex ensures that the result file looks 
identical even though the value of $exp_drv is different for different 
storage engines.

>> @@ -34,6 +35,50 @@ storage_engine_ref get_storage_engine(TH
>>    if (table)
>>    {
>>      se= plugin_ref_to_se_ref(table->s->db_plugin);
>> +
>> +  /*
>> +    Further check for underlying storage engine is needed
>> +    if table is partitioned
>> +  */
>> +
>> +  storage_engine_ref se_tmp= NULL;
>> +
>> +  if (table->part_info)
>> +    {
> 
> Wrong indentation: { shuld be directly below if
> 

I agree - will fix

>> +      partition_info *p_info=  table->part_info;
>> +      List_iterator<partition_element> p_it(p_info->partitions);
>> +      partition_element *p_el;
>> +      +      while ((p_el= p_it++))
>> +        {
>> +          if (!se_tmp)
>> +            {
>> +              se_tmp= get_hton_se(p_el->engine_type);
>> +              ::handlerton *h= se_hton(se_tmp);
>> +
>> +              /* +                 Only snapshot driver supports 
>> partitioning. Use
>> +                 default driver for all non-MVCC storage engines.
>> +               */
>> +              if (h->start_consistent_snapshot == NULL) +              {
>> +                se_tmp= NULL;
> 
> The assignment is redundant.

Will fix

>> +                goto close;
>> +              }
>> +
>> +              continue;
>> +            }

-- 
Jørgen Løland