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);
>> +}
[...]
