Hello,
Øystein Grøvlen a écrit, Le 26.10.2009 10:17:
> Guilhem Bichot wrote:
>
>>> === modified file 'sql/sql_select.cc'
>>> --- a/sql/sql_select.cc 2009-10-02 14:56:07 +0000
>>> +++ b/sql/sql_select.cc 2009-10-09 09:33:14 +0000
>>> @@ -14837,10 +14837,16 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
>>> key_part_info->store_length= key_part_info->length;
>>>
>>> if ((*reg_field)->real_maybe_null())
>>> + {
>>> key_part_info->store_length+= HA_KEY_NULL_LENGTH;
>>> + keyinfo->key_length+= HA_KEY_NULL_LENGTH;
>>> + }
>>> if ((*reg_field)->type() == MYSQL_TYPE_BLOB ||
>>> (*reg_field)->real_type() == MYSQL_TYPE_VARCHAR)
>>> + {
>>> key_part_info->store_length+= HA_KEY_BLOB_LENGTH;
>>> + keyinfo->key_length+= HA_KEY_BLOB_LENGTH;
>>> + }
>>>
>>> key_part_info->type= (uint8) (*reg_field)->key_type();
>>> key_part_info->key_type =
>>
>> Indeed this now looks closer to open_binary_frm().
>>
>> open_binary_frm() also updates keyinfo->extra_length, I wondered
>> whether we need to do the same in create_tmp_table()? In fact,
>> st_key::extra_length is unused apparently: removing it from the
>> struct's definition in sql/structs.h causes neither a compilation
>> problem nor a mtr test failure, could you please simply remove this
>> member now?
>
> I suggest that I do that as a separate patch in order to not obscure
> this bug fix.
Fine with me.
Please, try to bundle that separate patch with the push of the bugfix
(even though they would be two different revisions), so that we don't
forget the separate patch on some todo list ;-)
>> Last, if you like, please put a comment in front of open_binary_frm()
>> to say that changes to this function may sometimes have to be repeated
>> in create_tmp_table().
>
> I will.
I saw the new patch, thanks, it's ok to push.
>> Ok to push after that.
>
> I will wait for Timour's approval before pushing it.
> The last day you were in Trondheim, I discussed my patch with Timour,
> and I promised to investigate other parts of the code which update
> key_length differently. (create_duplicate_weedout_tmp_table() and group
> part of create_tmp_table()). It seems from my experiments that
> key_length is not used for anything in those cases. At least, not
> setting it at all, or setting it do a too small value did not make any
> of the tests in the MTR main suite fail.
Yes, good idea to investigate all this now.
