List:Commits« Previous MessageNext Message »
From:Magnus Blåudd Date:March 18 2011 7:00am
Subject:Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111
View as plain text  
Hi Dmitry,

thanks for your review. I have one final questions, see below. Once I go 
that answered, I'll proceed and push this.




>
>> === modified file 'sql/sql_show.cc'
>> --- a/sql/sql_show.cc	2011-02-08 15:47:33 +0000
>> +++ b/sql/sql_show.cc	2011-03-04 08:41:29 +0000
>> @@ -1379,17 +1379,24 @@ int store_create_info(THD *thd, TABLE_LI
>>     if (!(thd->variables.sql_mode&  MODE_NO_TABLE_OPTIONS)&& 
> !foreign_db_mode)
>>     {
>>       show_table_options= TRUE;
>> -    /*
>> -      Get possible table space definitions and append them
>> -      to the CREATE TABLE statement
>> -    */
>>
>> -    if ((for_str= file->get_tablespace_name(thd,0,0)))
>> +    /* TABLESPACE and STORAGE */
>> +    if (share->tablespace ||
>> +        share->default_storage_media != HA_SM_DEFAULT)
>>       {
>> -      packet->append(STRING_WITH_LEN(" /*!50100 TABLESPACE "));
>> -      packet->append(for_str, strlen(for_str));
>> -      packet->append(STRING_WITH_LEN(" STORAGE DISK */"));
>> -      my_free(for_str);
>> +      packet->append(STRING_WITH_LEN(" /*!50100"));
>> +      if (share->tablespace)
>> +      {
>> +        packet->append(STRING_WITH_LEN(" TABLESPACE "));
>> +        packet->append(share->tablespace, strlen(share->tablespace));
>> +      }
>> +
>> +      if (share->default_storage_media == HA_SM_DISK)
>> +        packet->append(STRING_WITH_LEN(" STORAGE DISK"));
>> +      if (share->default_storage_media == HA_SM_MEMORY)
>> +        packet->append(STRING_WITH_LEN(" STORAGE MEMORY"));
>> +
>> +      packet->append(STRING_WITH_LEN(" */"));
>>       }
>>
>>       /*
>
> Maybe it makes sense to also adjust store_schema_partitions_record()
> code so it will no longer use handler::get_tablespace_name()?


Yes, I can do that. But since there are then no use of 
handler::get_tablespace_name(), should I remove that function? My 
suggestion is to remove it.


>
> ...
>
>> === modified file 'sql/table.cc'
>> --- a/sql/table.cc	2011-01-28 12:28:15 +0000
>> +++ b/sql/table.cc	2011-03-04 08:41:29 +0000
>
> ...
>
>> @@ -1438,6 +1483,18 @@ static int open_binary_frm(THD *thd, TAB
>>           error= 8;
>>           goto err;
>>         }
>> +
>> +    if (format_section_fields)
>> +    {
>> +      const uchar field_flags= format_section_fields[i];
>> +      const uchar field_storage= (field_flags&  STORAGE_TYPE_MASK);
>> +      const uchar field_column_format=
>> +        ((field_flags>>  COLUMN_FORMAT_SHIFT)&  STORAGE_TYPE_MASK);
>                                                    ^^^^^^^^^^^^^^^^^
> I think the above should be:                      COLUMN_FORMAT_MASK


Yes, good catch.

>
> ...
>
>> === modified file 'sql/table.h'
>> --- a/sql/table.h	2010-12-29 00:26:31 +0000
>> +++ b/sql/table.h	2011-03-04 08:41:29 +0000
>> @@ -609,6 +609,8 @@ struct TABLE_SHARE
>>     }
>>     enum row_type row_type;		/* How rows are stored */
>>     enum tmp_table_type tmp_table;
>> +  enum ha_storage_media default_storage_media;
>> +  char *tablespace;
>>
>
> Please add comments describing these two new members.
>
> Also I would have put them a bit lower in the list of members
> (e.g. after cached_row_logging_check member).
>
>>     uint ref_count;                       /* How many TABLE objects uses this */
>>     uint blob_ptr_size;			/* 4 or 8 */

OK

>>
>
> ...
>
> Otherwise, I am OK with your patch and think that it can be pushed
> after considering/addressing the above issues.
>


Thanks!

/ Magnus
Thread
bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd23 Feb
Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd2 Mar
  • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Sergey Vojtovich2 Mar
    • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd2 Mar
Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd4 Mar
  • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Sergey Vojtovich4 Mar
Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Dmitry Lenev10 Mar
  • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd18 Mar
    • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Dmitry Lenev21 Mar
      • Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111Magnus Blåudd21 Mar