← Back to team overview

maria-developers team mailing list archive

Re: Please review MDEV-9653 Assertion `length || !scale' failed in uint my_decimal_length_to_precision(uint, uint, bool)

 

Hi Sergei,


On 03/17/2016 01:16 AM, Sergei Golubchik wrote:
> Hi, Alexander!
> 
> On Mar 16, Alexander Barkov wrote:
>> Hello Sergei,
>>
>> Please review a patch for:
>>
>> MDEV-9653 Assertion `length || !scale' failed in uint
>> my_decimal_length_to_precision(uint, uint, bool)
>>
>> Thanks!
> 
>> diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
>> index 2783a05..73a47ac 100644
>> --- a/sql/item_cmpfunc.cc
>> +++ b/sql/item_cmpfunc.cc
>> @@ -3086,10 +3086,28 @@ void Item_func_case::agg_str_lengths(Item* arg)
>>  
>>  void Item_func_case::agg_num_lengths(Item *arg)
>>  {
>> -  uint len= my_decimal_length_to_precision(arg->max_length, arg->decimals,
>> -                                           arg->unsigned_flag) - arg->decimals;
>> -  set_if_bigger(max_length, len); 
>> -  set_if_bigger(decimals, arg->decimals);
>> +  /*
>> +    In a query like this:
>> +    SELECT CASE WHEN TRUE
>> +             THEN COALESCE(CAST(NULL AS UNSIGNED))
>> +             ELSE 40
>> +           END;
>> +    "arg" corresponding to Item_func_coalesce has:
>> +      arg->max_length==0
>> +      arg->decimals==31 (NOT_FIXED_DEC).
> 
> why is it NOT_FIXED_DEC? it seems to me that it has to be 0 here.

Perhaps my example in the comments made you think that
it happens in a very special case with CAST AS UNSIGNED.
It also happens in simpler cases:

SELECT CASE WHEN TRUE
THEN COALESCE(NULL)
ELSE 40
END;

I guess I should fix the example to this ^^^.


Explicit NULL has STRING_RESULT as cmp_type() and result_type().
So it behaves like strings. It's all around the code.


For COALESCE, IF, IFULL, CASE with explicit NULL input
decimals is set to NOT_FIXED_DEC in here:


bool Item_func::count_string_result_length(enum_field_types field_type_arg,
                                           Item **items, uint nitems)
{
  if (agg_arg_charsets_for_string_result(collation, items, nitems, 1))
    return true;
  if (is_temporal_type(field_type_arg))
    count_datetime_length(items, nitems);
  else
  {
    decimals= NOT_FIXED_DEC;
    count_only_length(items, nitems);
  }
  return false;
}


But the problem is that it happens not only here.
It's also done that way in all string functions:

For example:

MariaDB [test]> SELECT LEFT(null,1);
Field   1:  `LEFT(null,1)`
Type:       VAR_STRING
Collation:  binary (63)
Length:     0
Max_length: 0
Decimals:   31          -- NOT_FIXED_DEC
Flags:      BINARY


MariaDB [test]> SELECT CONCAT(null);
Field   1:  `CONCAT(null)`
Type:       VAR_STRING
Collation:  binary (63)
Length:     0
Max_length: 0
Decimals:   31          -- NOT_FIXED_DEC
Flags:      BINARY

It has always been that way, for years.


I guess other non-function Item types do the same for an explicit NULL
input.


Fixing all Items to have decimals==0 in case
of explicit NULL input looks too dangerous in 10.1.
(btw, in 10.2 it should be easier).


So in 10,1 I decided to fix Item_func_case::agg_num_lengths.

> 
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx
> 


Follow ups

References