List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:July 13 2010 7:53pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415
View as plain text  
Sven Sandberg wrote:
> Hi Zhenxing,
[...]
> (5) The explicit type conversion in 
> sql_update.cc:is_order_deterministic() looks suspicious. How can we be 
> sure that order->item is of type Item_field* ? If order->item is always 
> of type Item_field*, then can we instead change the type to Item_field 
> and avoid the explicit type conversion?

OK, I found out: it is not safe to convert this to Item_field* because 
it may not be an Item_field*. In particular, the following causes the 
server to crash:

CREATE TABLE t1 (a INT, b INT PRIMARY KEY);
CREATE TABLE t2 LIKE t1;
INSERT INTO t2 SELECT * FROM t1 ORDER BY CONCAT(a, b) LIMIT 1;

So we have to first check what type of Item it is. If it is not an 
Item_field, I think we can break the loop, because the order is 
deterministic if a prefix of the ORDER BY columns are of type Item_field 
and are a PK. Examples:

ORDER BY CONCAT(no_key_col_1, no_key_col_2), primary_key_col:
   deterministic but marked as unsafe
ORDER BY primary_key_col, CONCAT(no_key_col_1, no_key_col_2)
   deterministic and not marked unsafe
ORDER BY RAND():
   deterministic (we replicate the seed) but marked as unsafe

/Sven

[...]
>> === modified file 'sql/sql_update.cc'
>> --- a/sql/sql_update.cc    2010-05-27 20:07:40 +0000
>> +++ b/sql/sql_update.cc    2010-07-11 13:30:46 +0000
>> @@ -850,6 +850,35 @@ err:
>>  }
>>  
>>  /*
>> +  Test if the result order is deterministic.
>> +
>> +  @retval FALSE not deterministic
>> +  @retval TRUE deterministic
>> + */
>> +bool is_order_deterministic(TABLE *table, ORDER *order)
>> +{
>> +  MY_BITMAP order_set;
>> +  MY_BITMAP key_set;
>> +  uint key= table->s->primary_key;
>> +
>> +  if (order == NULL)
>> +    return FALSE;
>> +  if (key == MAX_KEY)
>> +    return FALSE;
>> +
>> +  bitmap_init(&order_set, NULL, table->s->fields, FALSE);
>> +  for (; order; order=order->next)
>> +  {
>> +    Field *field=((Item_field*) (*order->item)->real_item())->field;
> 
> (5): Please check if this type conversion is safe, and if it is, check 
> if you can change the type of ORDER::item to Item_field*.
> 
>> +    bitmap_set_bit(&order_set, field->field_index);
>> +  }
>> +
>> +  bitmap_init(&key_set, NULL, table->s->fields, FALSE);
>> +  table->mark_columns_used_by_index_no_reset(key, &key_set);
>> +  return bitmap_is_subset(&key_set, &order_set);
>> +}
[...]
Thread
bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415He Zhenxing11 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418)Bug#42415He Zhenxing14 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418)Bug#42415He Zhenxing15 Jul