From 37282cc6bdaec20ce2a8775336dc5b4a228d71fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 28 Jun 2011 15:28:21 +0300 Subject: [PATCH] Bug#12699505 Memory leak in row_create_index_for_mysql() DB_COL_APPEARS_TWICE_IN_INDEX: Remove. This condition is already checked and reported by MySQL before passing the index definition to the storage engine. row_create_index_for_mysql(): Remove the redundant check for DB_COL_APPEARS_TWICE_IN_INDEX. When enforcing the column prefix index limit, invoke dict_mem_index_free(index) to plug the memory leak. In the loop, use index->n_def instead of dict_index_get_n_fields(index), because the latter would be 0 for indexes that have not been copied to the data dictionary cache. innodb-use-sys-malloc.test: Add test cases for attempting to trigger the error checks in row_create_index_for_mysql(). Before MySQL 5.5 and WL#5743, the leak is only reproducible if ha_innobase::max_supported_key_part_length() returned a higher limit than the one used in row_create_index_for_mysql(). In MySQL 5.5 and later, the leak is reproducible with innodb_large_prefix=true. rb:688 approved by Jimmy Yang --- .../innodb/r/innodb-use-sys-malloc.result | 75 ++++++++++--------- .../innodb/t/innodb-use-sys-malloc-master.opt | 5 +- .../suite/innodb/t/innodb-use-sys-malloc.test | 59 +++++++-------- storage/innobase/handler/ha_innodb.cc | 1 - storage/innobase/include/db0err.h | 2 - storage/innobase/row/row0mysql.c | 37 ++------- storage/innobase/ut/ut0ut.c | 2 - 7 files changed, 75 insertions(+), 106 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb-use-sys-malloc.result b/mysql-test/suite/innodb/r/innodb-use-sys-malloc.result index 2ec4c7c8130..d6ca690be7e 100644 --- a/mysql-test/suite/innodb/r/innodb-use-sys-malloc.result +++ b/mysql-test/suite/innodb/r/innodb-use-sys-malloc.result @@ -9,40 +9,45 @@ SELECT @@GLOBAL.innodb_use_sys_malloc; @@GLOBAL.innodb_use_sys_malloc 1 1 Expected -drop table if exists t1; -create table t1(a int not null) engine=innodb DEFAULT CHARSET=latin1; -insert into t1 values (1),(2),(3),(4),(5),(6),(7); +create table t1(a int not null,key(a,a)) engine=innodb DEFAULT CHARSET=latin1; +ERROR 42S21: Duplicate column name 'a' +create table t1(a int,b text,key(b(768))) engine=innodb DEFAULT CHARSET=latin1; +ERROR HY000: Index column size too large. The maximum column size is 767 bytes. +create table t1(a int not null,b text) engine=innodb DEFAULT CHARSET=latin1; +insert into t1 values (1,''),(2,''),(3,''),(4,''),(5,''),(6,''),(7,''); +create index t1aa on t1(a,a); +ERROR 42S21: Duplicate column name 'a' +create index t1b on t1(b(768)); +ERROR HY000: Index column size too large. The maximum column size is 767 bytes. +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) NOT NULL, + `b` text +) ENGINE=InnoDB DEFAULT CHARSET=latin1 select * from t1; -a -1 -2 -3 -4 -5 -6 -7 -drop table t1; -SELECT @@GLOBAL.innodb_use_sys_malloc; -@@GLOBAL.innodb_use_sys_malloc -1 -1 Expected -SET @@GLOBAL.innodb_use_sys_malloc=0; -ERROR HY000: Variable 'innodb_use_sys_malloc' is a read only variable -Expected error 'Read only variable' -SELECT @@GLOBAL.innodb_use_sys_malloc; -@@GLOBAL.innodb_use_sys_malloc -1 -1 Expected -drop table if exists t1; -create table t1(a int not null) engine=innodb DEFAULT CHARSET=latin1; -insert into t1 values (1),(2),(3),(4),(5),(6),(7); -select * from t1; -a -1 -2 -3 -4 -5 -6 -7 +a b +1 +2 +3 +4 +5 +6 +7 drop table t1; +CREATE TABLE t2(a int primary key, b text) ENGINE=InnoDB DEFAULT CHARSET=latin1; +INSERT INTO t2 VALUES (1,''),(2,''),(3,''),(4,''),(5,''),(6,''),(7,''); +CREATE INDEX t2aa on t2(a,a); +ERROR 42S21: Duplicate column name 'a' +CREATE INDEX t2b on t2(b(768)); +ERROR HY000: Index column size too large. The maximum column size is 767 bytes. +SELECT * FROM t2; +a b +1 +2 +3 +4 +5 +6 +7 +DROP TABLE t2; diff --git a/mysql-test/suite/innodb/t/innodb-use-sys-malloc-master.opt b/mysql-test/suite/innodb/t/innodb-use-sys-malloc-master.opt index 041b063645b..8071b4f7282 100644 --- a/mysql-test/suite/innodb/t/innodb-use-sys-malloc-master.opt +++ b/mysql-test/suite/innodb/t/innodb-use-sys-malloc-master.opt @@ -1,3 +1,2 @@ ---default-storage-engine=MyISAM ---loose-innodb-use-sys-malloc=true ---loose-innodb-use-sys-malloc=true +--innodb-use-sys-malloc=true +--innodb-large-prefix=true diff --git a/mysql-test/suite/innodb/t/innodb-use-sys-malloc.test b/mysql-test/suite/innodb/t/innodb-use-sys-malloc.test index 325dd19d086..7acd3f5fe81 100644 --- a/mysql-test/suite/innodb/t/innodb-use-sys-malloc.test +++ b/mysql-test/suite/innodb/t/innodb-use-sys-malloc.test @@ -1,4 +1,4 @@ ---source include/have_innodb.inc +-- source include/have_innodb.inc #display current value of innodb_use_sys_malloc SELECT @@GLOBAL.innodb_use_sys_malloc; @@ -13,36 +13,33 @@ SELECT @@GLOBAL.innodb_use_sys_malloc; --echo 1 Expected -#do some stuff to see if it works. ---disable_warnings -drop table if exists t1; ---enable_warnings +# Do some stuff to see if it works. +# Also, test the code paths of +# Bug #12699505 MEMORY LEAK IN ROW_CREATE_INDEX_FOR_MYSQL() +# (the leak would only be triggered if +# ha_innobase::max_supported_key_part_length() were set +# higher than the limit used in row_create_index_for_mysql()) -create table t1(a int not null) engine=innodb DEFAULT CHARSET=latin1; -insert into t1 values (1),(2),(3),(4),(5),(6),(7); -select * from t1; -drop table t1; ---source include/have_innodb.inc - -#display current value of innodb_use_sys_malloc -SELECT @@GLOBAL.innodb_use_sys_malloc; ---echo 1 Expected - -#try changing it. Should fail. ---error ER_INCORRECT_GLOBAL_LOCAL_VAR -SET @@GLOBAL.innodb_use_sys_malloc=0; ---echo Expected error 'Read only variable' - -SELECT @@GLOBAL.innodb_use_sys_malloc; ---echo 1 Expected - - -#do some stuff to see if it works. ---disable_warnings -drop table if exists t1; ---enable_warnings - -create table t1(a int not null) engine=innodb DEFAULT CHARSET=latin1; -insert into t1 values (1),(2),(3),(4),(5),(6),(7); +--error ER_DUP_FIELDNAME +create table t1(a int not null,key(a,a)) engine=innodb DEFAULT CHARSET=latin1; +# thanks to --innodb-large-prefix=1 this will not be truncated to b(767) +-- error ER_INDEX_COLUMN_TOO_LONG +create table t1(a int,b text,key(b(768))) engine=innodb DEFAULT CHARSET=latin1; +create table t1(a int not null,b text) engine=innodb DEFAULT CHARSET=latin1; +insert into t1 values (1,''),(2,''),(3,''),(4,''),(5,''),(6,''),(7,''); +--error ER_DUP_FIELDNAME +create index t1aa on t1(a,a); +-- error ER_INDEX_COLUMN_TOO_LONG +create index t1b on t1(b(768)); +SHOW CREATE TABLE t1; select * from t1; + drop table t1; +CREATE TABLE t2(a int primary key, b text) ENGINE=InnoDB DEFAULT CHARSET=latin1; +INSERT INTO t2 VALUES (1,''),(2,''),(3,''),(4,''),(5,''),(6,''),(7,''); +--error ER_DUP_FIELDNAME +CREATE INDEX t2aa on t2(a,a); +-- error ER_INDEX_COLUMN_TOO_LONG +CREATE INDEX t2b on t2(b(768)); +SELECT * FROM t2; +DROP TABLE t2; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 11640acbce4..c4d2226227e 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -990,7 +990,6 @@ convert_error_code_to_mysql( misleading, a new MySQL error code should be introduced */ - case DB_COL_APPEARS_TWICE_IN_INDEX: case DB_CORRUPTION: return(HA_ERR_CRASHED); diff --git a/storage/innobase/include/db0err.h b/storage/innobase/include/db0err.h index 74a2354bce3..28ef64500cc 100644 --- a/storage/innobase/include/db0err.h +++ b/storage/innobase/include/db0err.h @@ -64,8 +64,6 @@ enum db_err { DB_CANNOT_ADD_CONSTRAINT, /* adding a foreign key constraint to a table failed */ DB_CORRUPTION, /* data structure corruption noticed */ - DB_COL_APPEARS_TWICE_IN_INDEX, /* InnoDB cannot handle an index - where same column appears twice */ DB_CANNOT_DROP_CONSTRAINT, /* dropping a foreign key constraint from a table failed */ DB_NO_SAVEPOINT, /* no savepoint exists with the given diff --git a/storage/innobase/row/row0mysql.c b/storage/innobase/row/row0mysql.c index e1ada387729..a56d419d1f0 100644 --- a/storage/innobase/row/row0mysql.c +++ b/storage/innobase/row/row0mysql.c @@ -2015,41 +2015,13 @@ row_create_index_for_mysql( trx_start_if_not_started(trx); - /* Check that the same column does not appear twice in the index. - Starting from 4.0.14, InnoDB should be able to cope with that, but - safer not to allow them. */ - - for (i = 0; i < dict_index_get_n_fields(index); i++) { - ulint j; - - for (j = 0; j < i; j++) { - if (0 == ut_strcmp( - dict_index_get_nth_field(index, j)->name, - dict_index_get_nth_field(index, i)->name)) { - ut_print_timestamp(stderr); - - fputs(" InnoDB: Error: column ", stderr); - ut_print_name(stderr, trx, FALSE, - dict_index_get_nth_field( - index, i)->name); - fputs(" appears twice in ", stderr); - dict_index_name_print(stderr, trx, index); - fputs("\n" - "InnoDB: This is not allowed" - " in InnoDB.\n", stderr); - - err = DB_COL_APPEARS_TWICE_IN_INDEX; - - goto error_handling; - } - } - - /* Check also that prefix_len and actual length - is less than that from DICT_MAX_FIELD_LEN_BY_FORMAT() */ + for (i = 0; i < index->n_def; i++) { + /* Check that prefix_len and actual length + < DICT_MAX_INDEX_COL_LEN */ len = dict_index_get_nth_field(index, i)->prefix_len; - if (field_lengths) { + if (field_lengths && field_lengths[i]) { len = ut_max(len, field_lengths[i]); } @@ -2057,6 +2029,7 @@ row_create_index_for_mysql( if (len > (ulint) DICT_MAX_FIELD_LEN_BY_FORMAT(table)) { err = DB_TOO_BIG_INDEX_COL; + dict_mem_index_free(index); goto error_handling; } } diff --git a/storage/innobase/ut/ut0ut.c b/storage/innobase/ut/ut0ut.c index a9c0d381e16..1ef1a082bb2 100644 --- a/storage/innobase/ut/ut0ut.c +++ b/storage/innobase/ut/ut0ut.c @@ -674,8 +674,6 @@ ut_strerr( return("Cannot add constraint"); case DB_CORRUPTION: return("Data structure corruption"); - case DB_COL_APPEARS_TWICE_IN_INDEX: - return("Column appears twice in index"); case DB_CANNOT_DROP_CONSTRAINT: return("Cannot drop constraint"); case DB_NO_SAVEPOINT: