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.
>
> open_binary_frm() tests for BLOB, VARCHAR or GEOMETRY types
> (table.cc:1258). Whereas create_tmp_table() above only tests for BLOB or
> VARCHAR. I thought, it could be that create_tmp_table() should test for
> GEOMETRY too - otherwise maybe we currently have a bug in the
> materialized table contains a geometric-type column? So I modified your
> testcase to include a GEOMETRY column, alas it crashed the server (with
> and without your patch). So I filed it as BUG#48213 "Materialized
> subselect crashes if using GEOMETRY type".
Good. I have assigned myself to that report, and will investigate it
further.
>
> 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.
>
> 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.
--
Øystein
