Re: bzr commit into mysql-5.1-bugteam branch (v.narayanan:3197) Bug#47762

[V Narayanan]

Hi Jorgen,

I had already cscoped for all uses of store_val_in_field. I understand 
this situation.
If you had noticed the earlier code error was a bool value, and it was 
being assigned
0, 1 and -1 (error= item->save_in_field(field, 1); ). error correctly 
translated -1 to true.
Hence I felt confident to use it.

Also I have run all the tests. All of them passed for 5.1.

Narayanan

Jørgen Løland wrote:
> I discovered one more thing:
>
> store_val_in_field is used in another location that may break by the 
> suggested three-value return:
>
> sql_select.cc#test_if_ref()
> return !store_val_in_field(field, right_item, CHECK_FIELD_WARN);
>
> V Narayanan wrote:
>> #At 
>>
> file:///home/narayanan/Work/mysql_checkouts/shared_repository_directory/mysql-5.1-bugteam-47762/
> 
>> based on revid:joro@stripped
>>
>>  3197 V Narayanan    2009-10-30
>>       Bug#47762 Incorrect result from MIN() when WHERE tests NOT NULL 
>> column for NULL
>>             The lookup of a NULL value in a column having a NOT NULL
>>       index was resulting in wrong query result. This was happening
>>       due to the return value, from the function that stored the
>>       field value, being ignored.
>>      @ sql/opt_sum.cc
>>         Bug#47762 Incorrect result from MIN() when WHERE tests NOT 
>> NULL column for NULL
>>                 Ensure that the error (or not) from the 
>> store_val_in_field
>>         function is used while determining the usability of the index
>>         (determined by the return value of the matching_cond function).
>>      @ sql/sql_select.cc
>>         Bug#47762 Incorrect result from MIN() when WHERE tests NOT 
>> NULL column for NULL
>>                 Ensure that all the three cases of no error, conversion
>>         and error are reported by the store_val_in_field function.
>>      @ sql/sql_select.h
>>         Bug#47762 Incorrect result from MIN() when WHERE tests NOT 
>> NULL column for NULL
>>                 The store_val_in_field function returns a int instead of
>>         a bool so that it can handle all the three cases, namely,
>>         error, conversion and no error.
>>
>>     modified:
>>       sql/opt_sum.cc
>>       sql/sql_select.cc
>>       sql/sql_select.h
>> === modified file 'sql/opt_sum.cc'
>> --- a/sql/opt_sum.cc    2009-10-14 08:46:50 +0000
>> +++ b/sql/opt_sum.cc    2009-10-30 12:24:33 +0000
>> @@ -599,6 +599,12 @@ static bool matching_cond(bool max_fl, T
>>                            key_part_map *key_part_used, uint *range_fl,
>>                            uint *prefix_len)
>>  {
>> +  /*
>> +    contains the error that may occur when trying to store a
>> +    field value
>> +  */
>> +  int store_value_in_field_error;
>> +
>>    if (!cond)
>>      return 1;
>>    Field *field= field_part->field;
>> @@ -723,8 +729,10 @@ static bool matching_cond(bool max_fl, T
>>      }
>>      else
>>      {
>> -      store_val_in_field(part->field, args[between && max_fl ? 2 :
> 1],
>> -                         CHECK_FIELD_IGNORE);
>> +      store_value_in_field_error=
>> +                         store_val_in_field(part->field,
>> +                                            args[between && max_fl ? 
>> 2 : 1],
>> +                                            CHECK_FIELD_IGNORE);
>>        if (part->null_bit)          *key_ptr++= (uchar) 
>> test(part->field->is_null());
>>        part->field->get_key_image(key_ptr, part->length, Field::itRAW);
>> @@ -751,6 +759,14 @@ static bool matching_cond(bool max_fl, T
>>    }
>>    else if (is_field_part)
>>      *range_fl&= ~(max_fl ? NO_MIN_RANGE : NO_MAX_RANGE);
>> +  +  /*
>> +    If an error has occurred when trying to store the value
>> +    return 0 to indicate failure
>> +  */
>> +  if (store_value_in_field_error == -1)
>> +    return 0;
>> +
>>    return 1;   }
>>  
>>
>> === modified file 'sql/sql_select.cc'
>> --- a/sql/sql_select.cc    2009-10-22 09:40:15 +0000
>> +++ b/sql/sql_select.cc    2009-10-30 12:24:33 +0000
>> @@ -5735,13 +5735,15 @@ get_store_key(THD *thd, KEYUSE *keyuse,    
>> This function is only called for const items on fields which are keys.
>>  
>>    @return
>> -    returns 1 if there was some conversion made when the field was 
>> stored.
>> +    returns  0 to signify no error or conversion
>> +    returns  1 if there was some conversion made when the field was 
>> stored.
>> +    returns -1 to signify error
>>  */
>>  
>> -bool
>> +int
>>  store_val_in_field(Field *field, Item *item, enum_check_fields 
>> check_flag)
>>  {
>> -  bool error;
>> +  int error;
>>    TABLE *table= field->table;
>>    THD *thd= table->in_use;
>>    ha_rows cuted_fields=thd->cuted_fields;
>> @@ -5758,7 +5760,19 @@ store_val_in_field(Field *field, Item *i
>>    error= item->save_in_field(field, 1);
>>    thd->count_cuted_fields= old_count_cuted_fields;
>>    dbug_tmp_restore_column_map(table->write_set, old_map);
>> -  return error || cuted_fields != thd->cuted_fields;
>> +
>> +  /*
>> +    In the value being returned we handle three cases
>> +    1)  0 meaning no error or truncation has occurred
>> +    2)  1 meaning there has been a truncation
>> +    3) -1 meaning there is an error when trying to store the value
>> +  */
>> +  if (error)
>> +    return error;
>> +  else if (cuted_fields != thd->cuted_fields)
>> +    return 1;
>> +  +  return 0;
>>  }
>>  
>>  
>>
>> === modified file 'sql/sql_select.h'
>> --- a/sql/sql_select.h    2009-10-14 08:46:50 +0000
>> +++ b/sql/sql_select.h    2009-10-30 12:24:33 +0000
>> @@ -558,7 +558,7 @@ extern const char *join_type_str[];
>>  void TEST_join(JOIN *join);
>>  
>>  /* Extern functions in sql_select.cc */
>> -bool store_val_in_field(Field *field, Item *val, enum_check_fields 
>> check_flag);
>> +int store_val_in_field(Field *field, Item *val, enum_check_fields 
>> check_flag);
>>  TABLE *create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> 
>> &fields,
>>              ORDER *group, bool distinct, bool save_sum_fields,
>>              ulonglong select_options, ha_rows rows_limit,
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>
>