List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:March 2 2011 11:35am
Subject:Re: bzr commit into mysql-5.5 branch (magnus.blaudd:3353) Bug#60111
View as plain text  
Magnus,

On Wed, Mar 02, 2011 at 10:15:09AM +0100, Magnus Blåudd wrote:

...skip...
> >>        - Implement default version of handler::get_tablespace_name() since
> tablespace
> >>          is now available in share and it's unnecessary for each handler  to
> implement.
> >>         (the function could actually be removed totally now).
> >This assumption (handler::get_tablespace_name() could be removed)
> >makes me thoughtful. If it still makes sense...
> >
> >In 6.0 partitions may go to different tablespaces, storage engines
> >kept tablespace names. This patch suggests that tablespace names
> >are to be kept by SQL. It shouldn't be a problem assuming
> >partitioning handles it as well. Just a different concept from
> >what we had in 6.0.
> >
> >Also I believe there is some progress wrt tablespaces in InnoDB.
> >
> >All in all no objections from my side. But interested parties should
> >be informed, thus I CC Kevin and Mattias.
> 
> Good. But we don't need to think about how this would work in 6.0
> since that source it not used anymore.
The point here is not 6.0 codebase as such. The point is that we
did the same thing before. And we did it slightly different.
Nevertheless it is just something to be aware of, no action needed.

> >...skip...
> >>+CREATE TABLE t1(a int) ENGINE=MyISAM;
> >>+ALTER TABLE t1 STORAGE MEMORY TABLESPACE ts;
> >>+SHOW CREATE TABLE t1;
> >>+Table	Create Table
> >>+t1	CREATE TABLE `t1` (
> >>+  `a` int(11) DEFAULT NULL
> >>+) /*!50100 TABLESPACE ts STORAGE MEMORY */ ENGINE=MyISAM DEFAULT
> CHARSET=latin1
> >>+ALTER TABLE t1 STORAGE DISK tablespace ts2;
> >"tablespace" keyword is upper case in this test, but not here.
> 
> No, it's in lowercase...
> 
> +ALTER TABLE t1 STORAGE DISK tablespace ts2;
> +SHOW CREATE TABLE t1;
Well, I meant you use "TABLESPACE" (upper case) everywhere, except for
this particular query, where you use "tablespace" (lower case).

> >...skip...
> >>+      /* header */
> >>+      const uint format_section_length= uint2korr(next_chunk);
> >>+      const uint format_section_flags= uint4korr(next_chunk+2);
> >>+      /* 2 bytes unused */
> >2 bytes reserved in format section of extra data segment of
> >meta-data file. It makes me think something is not OK with
> >our DD.
> 
> DD = design document?
data dictionary

> Yes, this is how it was implemented originally a couple of years
> ago, and I want to keep compatibility with that.
> 
> On the other hand it's "only" two bytes for the whole table.
Yes, I understand. Just constating the fact that it looks... ehm...
interesting.

> >...skip...
> >>@@ -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);
> >>+      DBUG_PRINT("debug", ("field flags: %u, storage: %u, column_format:
> %u",
> >>+                           field_flags, field_storage,
> field_column_format));
> >>+      (void)field_storage; /* Used in MySQL Cluster */
> >>+      (void)field_column_format; /* Used in MySQL Cluster */
> >>+    }
> >I assume the above code is blank. Why not to ifdef it out? What's the
> >purpose to have it here at all?
> 
> 1) Since MySQL Cluster based on 5.5 use the field_storage and
> fiel_column_format values, writing it this way it's a minmial diff
> for us in 5.5-cluster(we have enough diffs anyway).
> 
> 2) It's a way to "reserve" those bits, anyone working with 5.5 or
> trunk would see that the value are used in MySQL Cluster.
> 
> 
> I should probably change the comment to say "Reserved by MySQL Cluster"
My concern was: we execute commands that do not do anything
useful. If it would make Cluster guys life easier, I'm Ok about
this code hunk. But if I were not in the loop, I would be very
surprised to see active code that does nothing.

Regards,
Sergey
-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
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