Merge lp:~sergei.glushchenko/percona-server/ps51-128-max-index into lp:percona-server/5.1

Proposed by Sergei Glushchenko
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 488
Proposed branch: lp:~sergei.glushchenko/percona-server/ps51-128-max-index
Merge into: lp:percona-server/5.1
Diff against target: 172 lines (+64/-4)
10 files modified
Percona-Server/mysql-test/include/have_64_keys.inc (+12/-0)
Percona-Server/mysql-test/r/have_64_keys.require (+14/-0)
Percona-Server/mysql-test/t/create.test (+5/-0)
Percona-Server/mysql-test/t/ps_1general.test (+5/-0)
Percona-Server/mysql-test/t/ps_2myisam.test (+5/-0)
Percona-Server/mysql-test/t/ps_3innodb.test (+5/-0)
Percona-Server/mysql-test/t/ps_4heap.test (+5/-0)
Percona-Server/mysql-test/t/ps_5merge.test (+5/-0)
Percona-Server/mysys/my_bitmap.c (+1/-1)
Percona-Server/sql/sql_bitmap.h (+7/-3)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-server/ps51-128-max-index
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Laurynas Biveinis (community) Approve
Review via email: mp+122520@code.launchpad.net

Description of the change

This is a port from
http://bazaar.launchpad.net/~percona-core/percona-server/rnt-5.1/revision/166

Been tested locally and the only failed tests were
main.create
main.ps_1general
main.ps_2myisam
main.ps_4heap
main.ps_5merge
main.ps_3innodb

which are designed with the assumption that MAX_INDEXES=64.

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

#25824

Revision history for this message
Alexey Kopytov (akopytov) wrote :

As discussed on IRC we should look into making the failing test cases pass (or be skipped) with all --with-max-indexes values.

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

include/have_64_keys.inc been added to check whether we have exactly 64 max indexes or not. If we have not
main.create
main.ps_1general
main.ps_2myisam
main.ps_4heap
main.ps_5merge
main.ps_3innodb
are skipped.

http://jenkins.percona.com/view/PS%205.1/job/percona-server-5.1-param/403/

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Minor comments:

    Please add a comment to have_64_keys explaining what part of the
    metadata confirms that max_indexes = 64.
    Typos: line 39: s/check/checks, line 41: s/skipping/skip. These
    are repeated in other test headers.

    Bitmap::to_ulonglong() returns the first ulonglong-sized bytes of
    the bitmap. In case of default bitmap representation that's the
    whole bitmap, in case of the general bitmap that's a prefix only.
    It does suggest that the code in
    opt_range.cc:get_best_group_min_max() might produce suboptimal
    query plans whenever max_indexes > 64 and actual indexes (key
    parts > 64). I am not sure that we should do anything about it.

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Laurynas,
> Bitmap::to_ulonglong() returns the first ulonglong-sized bytes of
> the bitmap. In case of default bitmap representation that's the
> whole bitmap, in case of the general bitmap that's a prefix only.
> It does suggest that the code in
> opt_range.cc:get_best_group_min_max() might produce suboptimal
> query plans whenever max_indexes > 64 and actual indexes (key
> parts > 64). I am not sure that we should do anything about it.

Code in opt_range.cc can be easily modified to compare prefix by using other operations of Bitmap<> class. More interesting is to construct a good testcase for this issue.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Comments been added and typos been fixed

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

    Diff line 12: "a synonym"
    Diff line 46 and friends: s/if it been done/in this case.

No need for another MP, just edit and re-push the branch. Thanks!

review: Approve
Revision history for this message
Alexey Kopytov (akopytov) wrote :

But we also need a 5.5 MP to be updated.

Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added file 'Percona-Server/mysql-test/include/have_64_keys.inc'
--- Percona-Server/mysql-test/include/have_64_keys.inc 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/include/have_64_keys.inc 2012-09-26 10:50:28 +0000
@@ -0,0 +1,12 @@
1# Check that we have MAX_INDEXES=64
2
3--require r/have_64_keys.require
4
5# Check that maximum length of possible_keys and key_len fields is 4096
6# They are defined in THD::send_explain_fields as NAME_CHAR_LEN*MAX_KEY
7# where NAME_CHAR_LEN is always 64 (mysql_com.h) and MAX_KEY is
8# a synonym of MAX_INDEXES
9
10--enable_metadata
11EXPLAIN SELECT 1;
12--disable_metadata
013
=== added file 'Percona-Server/mysql-test/r/have_64_keys.require'
--- Percona-Server/mysql-test/r/have_64_keys.require 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/have_64_keys.require 2012-09-26 10:50:28 +0000
@@ -0,0 +1,14 @@
1EXPLAIN SELECT 1;
2Catalog Database Table Table_alias Column Column_alias Type Length Max length Is_null Flags Decimals Charsetnr
3def id 8 3 1 N 32929 0 63
4def select_type 253 19 6 N 1 31 8
5def table 253 64 0 Y 0 31 8
6def type 253 10 0 Y 0 31 8
7def possible_keys 253 4096 0 Y 0 31 8
8def key 253 64 0 Y 0 31 8
9def key_len 253 4096 0 Y 0 31 8
10def ref 253 1024 0 Y 0 31 8
11def rows 8 10 0 Y 32928 0 63
12def Extra 253 255 14 N 1 31 8
13id select_type table type possible_keys key key_len ref rows Extra
141 SIMPLE NULL NULL NULL NULL NULL NULL NULL No tables used
015
=== modified file 'Percona-Server/mysql-test/t/create.test'
--- Percona-Server/mysql-test/t/create.test 2010-01-16 07:44:24 +0000
+++ Percona-Server/mysql-test/t/create.test 2012-09-26 10:50:28 +0000
@@ -2,6 +2,11 @@
2# Check some special create statements.2# Check some special create statements.
3#3#
44
5# This test checks some limits which are related to MAX_INDEXES constant
6# which could be set to non-default value at build time.
7# We just skip the test in this case.
8--source include/have_64_keys.inc
9
5--disable_warnings10--disable_warnings
6drop table if exists t1,t2,t3,t4,t5;11drop table if exists t1,t2,t3,t4,t5;
7drop database if exists mysqltest;12drop database if exists mysqltest;
813
=== modified file 'Percona-Server/mysql-test/t/ps_1general.test'
--- Percona-Server/mysql-test/t/ps_1general.test 2009-04-01 08:58:55 +0000
+++ Percona-Server/mysql-test/t/ps_1general.test 2012-09-26 10:50:28 +0000
@@ -4,6 +4,11 @@
4# #4# #
5##############################################################5##############################################################
66
7# This test checks some limits which are related to MAX_INDEXES constant
8# which could be set to non-default value at build time.
9# We just skip the test in this case.
10--source include/have_64_keys.inc
11
7# 12#
8# NOTE: PLEASE SEE THE DETAILED DESCRIPTION AT THE BOTTOM OF THIS FILE13# NOTE: PLEASE SEE THE DETAILED DESCRIPTION AT THE BOTTOM OF THIS FILE
9# BEFORE ADDING NEW TEST CASES HERE !!!14# BEFORE ADDING NEW TEST CASES HERE !!!
1015
=== modified file 'Percona-Server/mysql-test/t/ps_2myisam.test'
--- Percona-Server/mysql-test/t/ps_2myisam.test 2005-07-28 00:22:47 +0000
+++ Percona-Server/mysql-test/t/ps_2myisam.test 2012-09-26 10:50:28 +0000
@@ -4,6 +4,11 @@
4# #4# #
5###############################################5###############################################
66
7# This test checks some limits which are related to MAX_INDEXES constant
8# which could be set to non-default value at build time.
9# We just skip the test in this case.
10--source include/have_64_keys.inc
11
7# 12#
8# NOTE: PLEASE SEE ps_1general.test (bottom) 13# NOTE: PLEASE SEE ps_1general.test (bottom)
9# BEFORE ADDING NEW TEST CASES HERE !!!14# BEFORE ADDING NEW TEST CASES HERE !!!
1015
=== modified file 'Percona-Server/mysql-test/t/ps_3innodb.test'
--- Percona-Server/mysql-test/t/ps_3innodb.test 2010-06-09 12:07:34 +0000
+++ Percona-Server/mysql-test/t/ps_3innodb.test 2012-09-26 10:50:28 +0000
@@ -4,6 +4,11 @@
4# #4# #
5###############################################5###############################################
66
7# This test checks some limits which are related to MAX_INDEXES constant
8# which could be set to non-default value at build time.
9# We just skip the test in this case.
10--source include/have_64_keys.inc
11
7# 12#
8# NOTE: PLEASE SEE ps_1general.test (bottom) 13# NOTE: PLEASE SEE ps_1general.test (bottom)
9# BEFORE ADDING NEW TEST CASES HERE !!!14# BEFORE ADDING NEW TEST CASES HERE !!!
1015
=== modified file 'Percona-Server/mysql-test/t/ps_4heap.test'
--- Percona-Server/mysql-test/t/ps_4heap.test 2005-07-28 14:09:54 +0000
+++ Percona-Server/mysql-test/t/ps_4heap.test 2012-09-26 10:50:28 +0000
@@ -4,6 +4,11 @@
4# #4# #
5###############################################5###############################################
66
7# This test checks some limits which are related to MAX_INDEXES constant
8# which could be set to non-default value at build time.
9# We just skip the test in this case.
10--source include/have_64_keys.inc
11
7# 12#
8# NOTE: PLEASE SEE ps_1general.test (bottom) 13# NOTE: PLEASE SEE ps_1general.test (bottom)
9# BEFORE ADDING NEW TEST CASES HERE !!!14# BEFORE ADDING NEW TEST CASES HERE !!!
1015
=== modified file 'Percona-Server/mysql-test/t/ps_5merge.test'
--- Percona-Server/mysql-test/t/ps_5merge.test 2005-07-28 14:09:54 +0000
+++ Percona-Server/mysql-test/t/ps_5merge.test 2012-09-26 10:50:28 +0000
@@ -4,6 +4,11 @@
4# #4# #
5###############################################5###############################################
66
7# This test checks some limits which are related to MAX_INDEXES constant
8# which could be set to non-default value at build time.
9# We just skip the test in this case.
10--source include/have_64_keys.inc
11
7# 12#
8# NOTE: PLEASE SEE ps_1general.test (bottom) 13# NOTE: PLEASE SEE ps_1general.test (bottom)
9# BEFORE ADDING NEW TEST CASES HERE !!!14# BEFORE ADDING NEW TEST CASES HERE !!!
1015
=== modified file 'Percona-Server/mysys/my_bitmap.c'
--- Percona-Server/mysys/my_bitmap.c 2011-07-03 15:47:37 +0000
+++ Percona-Server/mysys/my_bitmap.c 2012-09-26 10:50:28 +0000
@@ -303,7 +303,7 @@
303 m+= prefix_bytes;303 m+= prefix_bytes;
304 if ((prefix_bits= prefix_size & 7))304 if ((prefix_bits= prefix_size & 7))
305 *(m++)= (1 << prefix_bits)-1;305 *(m++)= (1 << prefix_bits)-1;
306 if ((d= no_bytes_in_map(map)-prefix_bytes))306 if ((d= no_bytes_in_map(map) - (m - (uchar *) map->bitmap)))
307 bzero(m, d);307 bzero(m, d);
308}308}
309309
310310
=== modified file 'Percona-Server/sql/sql_bitmap.h'
--- Percona-Server/sql/sql_bitmap.h 2008-01-29 11:14:34 +0000
+++ Percona-Server/sql/sql_bitmap.h 2012-09-26 10:50:28 +0000
@@ -47,7 +47,11 @@
47 void intersect(ulonglong map2buff)47 void intersect(ulonglong map2buff)
48 {48 {
49 MY_BITMAP map2;49 MY_BITMAP map2;
50 bitmap_init(&map2, (uint32 *)&map2buff, sizeof(ulonglong)*8, 0);50 ulonglong buf;
51
52 /* bitmap_init() zeroes the supplied buffer */
53 bitmap_init(&map2, (uint32 *)&buf, sizeof(ulonglong)*8, 0);
54 buf= map2buff;
51 bitmap_intersect(&map, &map2);55 bitmap_intersect(&map, &map2);
52 }56 }
53 /* Use highest bit for all bits above sizeof(ulonglong)*8. */57 /* Use highest bit for all bits above sizeof(ulonglong)*8. */
@@ -87,9 +91,9 @@
87 ulonglong to_ulonglong() const91 ulonglong to_ulonglong() const
88 {92 {
89 if (sizeof(buffer) >= 8)93 if (sizeof(buffer) >= 8)
90 return uint8korr(buffer);94 return uint8korr((uchar *) buffer);
91 DBUG_ASSERT(sizeof(buffer) >= 4);95 DBUG_ASSERT(sizeof(buffer) >= 4);
92 return (ulonglong) uint4korr(buffer);96 return (ulonglong) uint4korr((uchar *) buffer);
93 }97 }
94};98};
9599

Subscribers

People subscribed via source and target branches