From 9fc71eebb6b96d76a2688a08bb810f2856b7a113 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 16:48:11 +1000 Subject: [PATCH 1/9] item_timefunc: identical operands CID 971836 (#1 of 1): Same on both sides (CONSTANT_EXPRESSION_RESULT) pointless_expression: The expression val != end && val != end does not accomplish anything because it evaluates to either of its identical operands, val != end. --- sql/item_timefunc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc index 420fb29f518..0ed1506bbee 100644 --- a/sql/item_timefunc.cc +++ b/sql/item_timefunc.cc @@ -3041,7 +3041,7 @@ get_date_time_result_type(const char *format, uint length) const char *val= format; const char *end= format + length; - for (; val != end && val != end; val++) + for (; val != end; val++) { if (*val == '%' && val+1 != end) { From cb870674d437d1f544ad0ccb2c759207b1735e8e Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 15:40:37 +1000 Subject: [PATCH 2/9] ha_archive::info remove hidden assignment max_data_file_size is overwritten in next statement so this assignment didn't ever get used. Found by Coverity (ID 1409644) --- storage/archive/ha_archive.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc index 872373bbeb6..d39d1187ce6 100644 --- a/storage/archive/ha_archive.cc +++ b/storage/archive/ha_archive.cc @@ -1640,7 +1640,6 @@ int ha_archive::info(uint flag) stats.update_time= (ulong) file_stat.st_mtime; if (flag & HA_STATUS_CONST) { - stats.max_data_file_length= share->rows_recorded * stats.mean_rec_length; stats.max_data_file_length= MAX_FILE_SIZE; stats.create_time= (ulong) file_stat.st_ctime; } From 623c3f673195e143338cb657df554e397097fb42 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 11:26:02 +1000 Subject: [PATCH 3/9] thread_group_close: release mutex in all branches Found by Coverity scan - id 92087 --- sql/threadpool_unix.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/sql/threadpool_unix.cc b/sql/threadpool_unix.cc index 6075c758e40..0ffe08a8051 100644 --- a/sql/threadpool_unix.cc +++ b/sql/threadpool_unix.cc @@ -979,24 +979,26 @@ static void thread_group_close(thread_group_t *thread_group) if (pipe(thread_group->shutdown_pipe)) { - DBUG_VOID_RETURN; + goto end; } /* Wake listener */ if (io_poll_associate_fd(thread_group->pollfd, thread_group->shutdown_pipe[0], NULL)) { - DBUG_VOID_RETURN; + goto end; + } + { + char c= 0; + if (write(thread_group->shutdown_pipe[1], &c, 1) < 0) + goto end; } - char c= 0; - if (write(thread_group->shutdown_pipe[1], &c, 1) < 0) - DBUG_VOID_RETURN; - /* Wake all workers. */ while(wake_thread(thread_group) == 0) { } +end: mysql_mutex_unlock(&thread_group->mutex); DBUG_VOID_RETURN; From 051f90a534758c4da038023a8ebdb57bd414cfe3 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 13:37:14 +1000 Subject: [PATCH 4/9] ma_loghandler: release file_header_lock on error translog_stop_writing doesn't release a lock (though does to a DBUG_ASSERT). Better to just release the lock. Found by Coverity id 972092 --- storage/maria/ma_loghandler.c | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/maria/ma_loghandler.c b/storage/maria/ma_loghandler.c index 16cd0a09af5..fd256a48358 100644 --- a/storage/maria/ma_loghandler.c +++ b/storage/maria/ma_loghandler.c @@ -1269,6 +1269,7 @@ static my_bool translog_set_lsn_for_files(uint32 from_file, uint32 to_file, mysql_file_close(fd, MYF(MY_WME)))) { translog_stop_writing(); + mysql_mutex_unlock(&log_descriptor.file_header_lock); DBUG_RETURN(1); } } From 2328860379eb6e1df240f5428213ed005b35a975 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 13:42:46 +1000 Subject: [PATCH 5/9] ma_loghandler: translog_set_only_in_buffers failed to release lock Release the lock for the error path. Found by Coverity (id 972093). --- storage/maria/ma_loghandler.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/maria/ma_loghandler.c b/storage/maria/ma_loghandler.c index fd256a48358..e2e52546fc9 100644 --- a/storage/maria/ma_loghandler.c +++ b/storage/maria/ma_loghandler.c @@ -2292,10 +2292,11 @@ static void translog_set_only_in_buffers(TRANSLOG_ADDRESS in_buffers) if (cmp_translog_addr(in_buffers, log_descriptor.in_buffers_only) > 0) { if (translog_status != TRANSLOG_OK) - DBUG_VOID_RETURN; + goto end; log_descriptor.in_buffers_only= in_buffers; DBUG_PRINT("info", ("set new in_buffers_only")); } +end: mysql_mutex_unlock(&log_descriptor.sent_to_disk_lock); DBUG_VOID_RETURN; } From 89b81a9a24c08221dc551d8572ef68e426afce3a Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sun, 2 Jul 2017 13:52:34 +1000 Subject: [PATCH 6/9] ma_pagecache: release lock in pagecache_read make_lock_and_pin didn't release the lock so we should. Found by Coverity (id 972095). --- storage/maria/ma_pagecache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/maria/ma_pagecache.c b/storage/maria/ma_pagecache.c index 1a791d43034..bb9f5a03ad7 100644 --- a/storage/maria/ma_pagecache.c +++ b/storage/maria/ma_pagecache.c @@ -3474,6 +3474,7 @@ restart: lock_to_read[lock].unlock_lock, unlock_pin, FALSE)) { + pagecache_pthread_mutex_unlock(&pagecache->cache_lock); DBUG_ASSERT(0); return (uchar*) 0; } From 23ac2dd2a4c5f281479980d0156a2ccebab510b2 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 4 Jul 2017 13:28:47 +1000 Subject: [PATCH 7/9] sql_class: incorrect assignment in Security_context::destroy Found by Coverity (id 971843). Signed-off-by: Daniel Black --- sql/sql_class.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 3c07c1606a2..00d860e7887 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -3670,7 +3670,7 @@ void Security_context::destroy() if (external_user) { my_free(external_user); - user= NULL; + external_user= NULL; } my_free(ip); From a7ed4644a6f3e38dbad3658fc35890ce31cc0749 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Mon, 3 Jul 2017 13:35:32 +0200 Subject: [PATCH 8/9] MDEV-10146: Wrong result (or questionable result and behavior) with aggregate function in uncorrelated SELECT subquery When outer reference resolved in a VIEW it still should mark aggregate function resolving border. --- mysql-test/r/subselect.result | 22 ++++++++++++++++++++ mysql-test/r/subselect_no_mat.result | 22 ++++++++++++++++++++ mysql-test/r/subselect_no_opts.result | 22 ++++++++++++++++++++ mysql-test/r/subselect_no_scache.result | 22 ++++++++++++++++++++ mysql-test/r/subselect_no_semijoin.result | 22 ++++++++++++++++++++ mysql-test/t/subselect.test | 25 +++++++++++++++++++++++ sql/item.cc | 7 +++++++ 7 files changed, 142 insertions(+) diff --git a/mysql-test/r/subselect.result b/mysql-test/r/subselect.result index 789cfe2fdca..000d8927542 100644 --- a/mysql-test/r/subselect.result +++ b/mysql-test/r/subselect.result @@ -7116,3 +7116,25 @@ SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); f1 f2 foo bar DROP TABLE t1; +# +# MDEV-10146: Wrong result (or questionable result and behavior) +# with aggregate function in uncorrelated SELECT subquery +# +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 int); +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +( SELECT MAX(f1) FROM t2 ) +2 +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +( SELECT MAX(f1) FROM t2 ) +2 +INSERT INTO t2 VALUES (4); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +ERROR 21000: Subquery returns more than 1 row +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +ERROR 21000: Subquery returns more than 1 row +drop view v1; +drop table t1,t2; diff --git a/mysql-test/r/subselect_no_mat.result b/mysql-test/r/subselect_no_mat.result index c729c17f94f..914f20d502a 100644 --- a/mysql-test/r/subselect_no_mat.result +++ b/mysql-test/r/subselect_no_mat.result @@ -7113,6 +7113,28 @@ SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); f1 f2 foo bar DROP TABLE t1; +# +# MDEV-10146: Wrong result (or questionable result and behavior) +# with aggregate function in uncorrelated SELECT subquery +# +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 int); +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +( SELECT MAX(f1) FROM t2 ) +2 +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +( SELECT MAX(f1) FROM t2 ) +2 +INSERT INTO t2 VALUES (4); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +ERROR 21000: Subquery returns more than 1 row +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +ERROR 21000: Subquery returns more than 1 row +drop view v1; +drop table t1,t2; set optimizer_switch=default; select @@optimizer_switch like '%materialization=on%'; @@optimizer_switch like '%materialization=on%' diff --git a/mysql-test/r/subselect_no_opts.result b/mysql-test/r/subselect_no_opts.result index dc308ea77e5..1b25e16ba7c 100644 --- a/mysql-test/r/subselect_no_opts.result +++ b/mysql-test/r/subselect_no_opts.result @@ -7111,4 +7111,26 @@ SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); f1 f2 foo bar DROP TABLE t1; +# +# MDEV-10146: Wrong result (or questionable result and behavior) +# with aggregate function in uncorrelated SELECT subquery +# +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 int); +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +( SELECT MAX(f1) FROM t2 ) +2 +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +( SELECT MAX(f1) FROM t2 ) +2 +INSERT INTO t2 VALUES (4); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +ERROR 21000: Subquery returns more than 1 row +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +ERROR 21000: Subquery returns more than 1 row +drop view v1; +drop table t1,t2; set @optimizer_switch_for_subselect_test=null; diff --git a/mysql-test/r/subselect_no_scache.result b/mysql-test/r/subselect_no_scache.result index e7c85c10f2d..90a11fbce7d 100644 --- a/mysql-test/r/subselect_no_scache.result +++ b/mysql-test/r/subselect_no_scache.result @@ -7122,6 +7122,28 @@ SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); f1 f2 foo bar DROP TABLE t1; +# +# MDEV-10146: Wrong result (or questionable result and behavior) +# with aggregate function in uncorrelated SELECT subquery +# +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 int); +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +( SELECT MAX(f1) FROM t2 ) +2 +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +( SELECT MAX(f1) FROM t2 ) +2 +INSERT INTO t2 VALUES (4); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +ERROR 21000: Subquery returns more than 1 row +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +ERROR 21000: Subquery returns more than 1 row +drop view v1; +drop table t1,t2; set optimizer_switch=default; select @@optimizer_switch like '%subquery_cache=on%'; @@optimizer_switch like '%subquery_cache=on%' diff --git a/mysql-test/r/subselect_no_semijoin.result b/mysql-test/r/subselect_no_semijoin.result index b6261f05098..936aae07227 100644 --- a/mysql-test/r/subselect_no_semijoin.result +++ b/mysql-test/r/subselect_no_semijoin.result @@ -7111,5 +7111,27 @@ SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); f1 f2 foo bar DROP TABLE t1; +# +# MDEV-10146: Wrong result (or questionable result and behavior) +# with aggregate function in uncorrelated SELECT subquery +# +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 int); +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +( SELECT MAX(f1) FROM t2 ) +2 +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +( SELECT MAX(f1) FROM t2 ) +2 +INSERT INTO t2 VALUES (4); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +ERROR 21000: Subquery returns more than 1 row +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; +ERROR 21000: Subquery returns more than 1 row +drop view v1; +drop table t1,t2; set @optimizer_switch_for_subselect_test=null; set @join_cache_level_for_subselect_test=NULL; diff --git a/mysql-test/t/subselect.test b/mysql-test/t/subselect.test index a8ad3ba52a5..c5f5b229656 100644 --- a/mysql-test/t/subselect.test +++ b/mysql-test/t/subselect.test @@ -5998,3 +5998,28 @@ INSERT INTO t1 VALUES ('foo','bar'); SELECT * FROM t1 WHERE f2 >= SOME ( SELECT f1 FROM t1 ); SELECT * FROM t1 WHERE f2 <= SOME ( SELECT f1 FROM t1 ); DROP TABLE t1; + +--echo # +--echo # MDEV-10146: Wrong result (or questionable result and behavior) +--echo # with aggregate function in uncorrelated SELECT subquery +--echo # +CREATE TABLE t1 (f1 INT); +CREATE VIEW v1 AS SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); + +CREATE TABLE t2 (f2 int); + +INSERT INTO t2 VALUES (3); +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; + +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; + +INSERT INTO t2 VALUES (4); + +--error ER_SUBQUERY_NO_1_ROW +SELECT ( SELECT MAX(f1) FROM t2 ) FROM v1; +--error ER_SUBQUERY_NO_1_ROW +SELECT ( SELECT MAX(f1) FROM t2 ) FROM t1; + +drop view v1; +drop table t1,t2; diff --git a/sql/item.cc b/sql/item.cc index 1df91dc2534..68411860274 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -5045,6 +5045,13 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference) ((ref_type == REF_ITEM || ref_type == FIELD_ITEM) ? (Item_ident*) (*reference) : 0)); + if (thd->lex->in_sum_func && + thd->lex->in_sum_func->nest_level >= select->nest_level) + { + Item::Type ref_type= (*reference)->type(); + set_if_bigger(thd->lex->in_sum_func->max_arg_level, + select->nest_level); + } /* A reference to a view field had been found and we substituted it instead of this Item (find_field_in_tables From f305a7ce4bccbd56520d874e1d81a4f29bc17a96 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 6 Jul 2017 14:06:37 +0200 Subject: [PATCH 9/9] bugfix: long partition names --- mysql-test/suite/parts/r/longname.result | 33 ++++ mysql-test/suite/parts/t/longname.test | 29 ++++ sql/ha_partition.cc | 195 +++++++++++++---------- sql/sql_partition.cc | 118 ++++++++------ sql/sql_partition.h | 12 +- 5 files changed, 253 insertions(+), 134 deletions(-) create mode 100644 mysql-test/suite/parts/r/longname.result create mode 100644 mysql-test/suite/parts/t/longname.test diff --git a/mysql-test/suite/parts/r/longname.result b/mysql-test/suite/parts/r/longname.result new file mode 100644 index 00000000000..40097d60d15 --- /dev/null +++ b/mysql-test/suite/parts/r/longname.result @@ -0,0 +1,33 @@ +set names utf8; +create database mysqltest1; +CREATE TABLE mysqltest1.test_jfg_table_name_with_64_chars_123456789012345678901234567890 ( +id int(10) unsigned NOT NULL, +id2 int(10) unsigned NOT NULL, +PRIMARY KEY ( id, id2 ) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC +PARTITION BY RANGE ( id ) +SUBPARTITION BY HASH ( id2 ) +SUBPARTITIONS 2 ( +PARTITION test_jfg_partition_name_with_60_chars_1234567890123456789012 VALUES LESS THAN (1000) ENGINE = InnoDB, +PARTITION pmax VALUES LESS THAN MAXVALUE ENGINE = InnoDB); +Warnings: +Warning 1478 InnoDB: ROW_FORMAT=DYNAMIC requires innodb_file_per_table. +Warning 1478 InnoDB: assuming ROW_FORMAT=COMPACT. +Warning 1478 InnoDB: ROW_FORMAT=DYNAMIC requires innodb_file_per_table. +Warning 1478 InnoDB: assuming ROW_FORMAT=COMPACT. +Warning 1478 InnoDB: ROW_FORMAT=DYNAMIC requires innodb_file_per_table. +Warning 1478 InnoDB: assuming ROW_FORMAT=COMPACT. +Warning 1478 InnoDB: ROW_FORMAT=DYNAMIC requires innodb_file_per_table. +Warning 1478 InnoDB: assuming ROW_FORMAT=COMPACT. +CREATE TABLE mysqltest1.éééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé ( +id int(10) unsigned NOT NULL, +id2 int(10) unsigned NOT NULL, +PRIMARY KEY ( id, id2 ) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC +PARTITION BY RANGE ( id ) +SUBPARTITION BY HASH ( id2 ) +SUBPARTITIONS 2 ( +PARTITION çççççççççççççççççççççççççççççççççççççççççççççççççççççççççççç VALUES LESS THAN (1000) ENGINE = InnoDB, +PARTITION pmax VALUES LESS THAN MAXVALUE ENGINE = InnoDB); +ERROR HY000: The path specified for @0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@0n@ is too long. +drop database mysqltest1; diff --git a/mysql-test/suite/parts/t/longname.test b/mysql-test/suite/parts/t/longname.test new file mode 100644 index 00000000000..feea5a790f2 --- /dev/null +++ b/mysql-test/suite/parts/t/longname.test @@ -0,0 +1,29 @@ +source include/have_innodb.inc; +source include/have_partition.inc; +set names utf8; + +create database mysqltest1; +CREATE TABLE mysqltest1.test_jfg_table_name_with_64_chars_123456789012345678901234567890 ( + id int(10) unsigned NOT NULL, + id2 int(10) unsigned NOT NULL, + PRIMARY KEY ( id, id2 ) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC +PARTITION BY RANGE ( id ) + SUBPARTITION BY HASH ( id2 ) + SUBPARTITIONS 2 ( + PARTITION test_jfg_partition_name_with_60_chars_1234567890123456789012 VALUES LESS THAN (1000) ENGINE = InnoDB, + PARTITION pmax VALUES LESS THAN MAXVALUE ENGINE = InnoDB); + +--error ER_PATH_LENGTH +CREATE TABLE mysqltest1.éééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé ( + id int(10) unsigned NOT NULL, + id2 int(10) unsigned NOT NULL, + PRIMARY KEY ( id, id2 ) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC +PARTITION BY RANGE ( id ) + SUBPARTITION BY HASH ( id2 ) + SUBPARTITIONS 2 ( + PARTITION çççççççççççççççççççççççççççççççççççççççççççççççççççççççççççç VALUES LESS THAN (1000) ENGINE = InnoDB, + PARTITION pmax VALUES LESS THAN MAXVALUE ENGINE = InnoDB); + +drop database mysqltest1; diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 43743544c25..e048dbad5e6 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -634,7 +634,7 @@ int ha_partition::create(const char *name, TABLE *table_arg, int ha_partition::drop_partitions(const char *path) { List_iterator part_it(m_part_info->partitions); - char part_name_buff[FN_REFLEN]; + char part_name_buff[FN_REFLEN + 1]; uint num_parts= m_part_info->partitions.elements; uint num_subparts= m_part_info->num_subparts; uint i= 0; @@ -667,9 +667,11 @@ int ha_partition::drop_partitions(const char *path) { partition_element *sub_elem= sub_it++; part= i * num_subparts + j; - create_subpartition_name(part_name_buff, path, - part_elem->partition_name, - sub_elem->partition_name, name_variant); + if ((ret_error= create_subpartition_name(part_name_buff, + sizeof(part_name_buff), path, + part_elem->partition_name, + sub_elem->partition_name, name_variant))) + error= ret_error; file= m_file[part]; DBUG_PRINT("info", ("Drop subpartition %s", part_name_buff)); if ((ret_error= file->ha_delete_table(part_name_buff))) @@ -680,15 +682,19 @@ int ha_partition::drop_partitions(const char *path) } else { - create_partition_name(part_name_buff, path, - part_elem->partition_name, name_variant, - TRUE); - file= m_file[i]; - DBUG_PRINT("info", ("Drop partition %s", part_name_buff)); - if ((ret_error= file->ha_delete_table(part_name_buff))) + if ((ret_error= create_partition_name(part_name_buff, + sizeof(part_name_buff), path, + part_elem->partition_name, name_variant, TRUE))) error= ret_error; - if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) - error= 1; + else + { + file= m_file[i]; + DBUG_PRINT("info", ("Drop partition %s", part_name_buff)); + if ((ret_error= file->ha_delete_table(part_name_buff))) + error= ret_error; + if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) + error= 1; + } } if (part_elem->part_state == PART_IS_CHANGED) part_elem->part_state= PART_NORMAL; @@ -724,8 +730,8 @@ int ha_partition::rename_partitions(const char *path) { List_iterator part_it(m_part_info->partitions); List_iterator temp_it(m_part_info->temp_partitions); - char part_name_buff[FN_REFLEN]; - char norm_name_buff[FN_REFLEN]; + char part_name_buff[FN_REFLEN + 1]; + char norm_name_buff[FN_REFLEN + 1]; uint num_parts= m_part_info->partitions.elements; uint part_count= 0; uint num_subparts= m_part_info->num_subparts; @@ -767,10 +773,11 @@ int ha_partition::rename_partitions(const char *path) { sub_elem= sub_it++; file= m_reorged_file[part_count++]; - create_subpartition_name(norm_name_buff, path, - part_elem->partition_name, - sub_elem->partition_name, - NORMAL_PART_NAME); + if ((ret_error= create_subpartition_name(norm_name_buff, + sizeof(norm_name_buff), path, + part_elem->partition_name, + sub_elem->partition_name, NORMAL_PART_NAME))) + error= ret_error; DBUG_PRINT("info", ("Delete subpartition %s", norm_name_buff)); if ((ret_error= file->ha_delete_table(norm_name_buff))) error= ret_error; @@ -783,16 +790,20 @@ int ha_partition::rename_partitions(const char *path) else { file= m_reorged_file[part_count++]; - create_partition_name(norm_name_buff, path, - part_elem->partition_name, NORMAL_PART_NAME, - TRUE); - DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); - if ((ret_error= file->ha_delete_table(norm_name_buff))) + if ((ret_error= create_partition_name(norm_name_buff, + sizeof(norm_name_buff), path, + part_elem->partition_name, NORMAL_PART_NAME, TRUE))) error= ret_error; - else if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) - error= 1; else - part_elem->log_entry= NULL; /* Indicate success */ + { + DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); + if ((ret_error= file->ha_delete_table(norm_name_buff))) + error= ret_error; + else if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) + error= 1; + else + part_elem->log_entry= NULL; /* Indicate success */ + } } } while (++i < temp_partitions); (void) sync_ddl_log(); @@ -835,10 +846,11 @@ int ha_partition::rename_partitions(const char *path) { sub_elem= sub_it++; part= i * num_subparts + j; - create_subpartition_name(norm_name_buff, path, - part_elem->partition_name, - sub_elem->partition_name, - NORMAL_PART_NAME); + if ((ret_error= create_subpartition_name(norm_name_buff, + sizeof(norm_name_buff), path, + part_elem->partition_name, + sub_elem->partition_name, NORMAL_PART_NAME))) + error= ret_error; if (part_elem->part_state == PART_IS_CHANGED) { file= m_reorged_file[part_count++]; @@ -850,10 +862,11 @@ int ha_partition::rename_partitions(const char *path) (void) sync_ddl_log(); } file= m_new_file[part]; - create_subpartition_name(part_name_buff, path, - part_elem->partition_name, - sub_elem->partition_name, - TEMP_PART_NAME); + if ((ret_error= create_subpartition_name(part_name_buff, + sizeof(part_name_buff), path, + part_elem->partition_name, + sub_elem->partition_name, TEMP_PART_NAME))) + error= ret_error; DBUG_PRINT("info", ("Rename subpartition from %s to %s", part_name_buff, norm_name_buff)); if ((ret_error= file->ha_rename_table(part_name_buff, @@ -867,32 +880,36 @@ int ha_partition::rename_partitions(const char *path) } else { - create_partition_name(norm_name_buff, path, - part_elem->partition_name, NORMAL_PART_NAME, - TRUE); - if (part_elem->part_state == PART_IS_CHANGED) + if ((ret_error= create_partition_name(norm_name_buff, + sizeof(norm_name_buff), path, + part_elem->partition_name, NORMAL_PART_NAME, TRUE)) || + (ret_error= create_partition_name(part_name_buff, + sizeof(part_name_buff), path, + part_elem->partition_name, TEMP_PART_NAME, TRUE))) + error= ret_error; + else { - file= m_reorged_file[part_count++]; - DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); - if ((ret_error= file->ha_delete_table(norm_name_buff))) + if (part_elem->part_state == PART_IS_CHANGED) + { + file= m_reorged_file[part_count++]; + DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); + if ((ret_error= file->ha_delete_table(norm_name_buff))) + error= ret_error; + else if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) + error= 1; + (void) sync_ddl_log(); + } + file= m_new_file[i]; + DBUG_PRINT("info", ("Rename partition from %s to %s", + part_name_buff, norm_name_buff)); + if ((ret_error= file->ha_rename_table(part_name_buff, + norm_name_buff))) error= ret_error; else if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) error= 1; - (void) sync_ddl_log(); + else + part_elem->log_entry= NULL; } - file= m_new_file[i]; - create_partition_name(part_name_buff, path, - part_elem->partition_name, TEMP_PART_NAME, - TRUE); - DBUG_PRINT("info", ("Rename partition from %s to %s", - part_name_buff, norm_name_buff)); - if ((ret_error= file->ha_rename_table(part_name_buff, - norm_name_buff))) - error= ret_error; - else if (deactivate_ddl_log_entry(part_elem->log_entry->entry_pos)) - error= 1; - else - part_elem->log_entry= NULL; } } } while (++i < num_parts); @@ -1488,7 +1505,7 @@ int ha_partition::change_partitions(HA_CREATE_INFO *create_info, { List_iterator part_it(m_part_info->partitions); List_iterator t_it(m_part_info->temp_partitions); - char part_name_buff[FN_REFLEN]; + char part_name_buff[FN_REFLEN + 1]; uint num_parts= m_part_info->partitions.elements; uint num_subparts= m_part_info->num_subparts; uint i= 0; @@ -1698,10 +1715,14 @@ int ha_partition::change_partitions(HA_CREATE_INFO *create_info, do { partition_element *sub_elem= sub_it++; - create_subpartition_name(part_name_buff, path, - part_elem->partition_name, - sub_elem->partition_name, - name_variant); + if ((error= create_subpartition_name(part_name_buff, + sizeof(part_name_buff), path, + part_elem->partition_name, sub_elem->partition_name, + name_variant))) + { + cleanup_new_partition(part_count); + DBUG_RETURN(error); + } part= i * num_subparts + j; DBUG_PRINT("info", ("Add subpartition %s", part_name_buff)); if ((error= prepare_new_partition(table, create_info, @@ -1719,9 +1740,14 @@ int ha_partition::change_partitions(HA_CREATE_INFO *create_info, } else { - create_partition_name(part_name_buff, path, - part_elem->partition_name, name_variant, - TRUE); + if ((error= create_partition_name(part_name_buff, + sizeof(part_name_buff), path, part_elem->partition_name, + name_variant, TRUE))) + { + cleanup_new_partition(part_count); + DBUG_RETURN(error); + } + DBUG_PRINT("info", ("Add partition %s", part_name_buff)); if ((error= prepare_new_partition(table, create_info, new_file_array[i], @@ -1980,8 +2006,8 @@ int ha_partition::del_ren_cre_table(const char *from, { int save_error= 0; int error= HA_ERR_INTERNAL_ERROR; - char from_buff[FN_REFLEN], to_buff[FN_REFLEN], from_lc_buff[FN_REFLEN], - to_lc_buff[FN_REFLEN], buff[FN_REFLEN]; + char from_buff[FN_REFLEN + 1], to_buff[FN_REFLEN + 1], + from_lc_buff[FN_REFLEN], to_lc_buff[FN_REFLEN], buff[FN_REFLEN]; char *name_buffer_ptr; const char *from_path; const char *to_path= NULL; @@ -2028,13 +2054,15 @@ int ha_partition::del_ren_cre_table(const char *from, i= 0; do { - create_partition_name(from_buff, from_path, name_buffer_ptr, - NORMAL_PART_NAME, FALSE); + if ((error= create_partition_name(from_buff, sizeof(from_buff), from_path, + name_buffer_ptr, NORMAL_PART_NAME, FALSE))) + goto rename_error; if (to != NULL) { // Rename branch - create_partition_name(to_buff, to_path, name_buffer_ptr, - NORMAL_PART_NAME, FALSE); + if ((error= create_partition_name(to_buff, sizeof(to_buff), to_path, + name_buffer_ptr, NORMAL_PART_NAME, FALSE))) + goto rename_error; error= (*file)->ha_rename_table(from_buff, to_buff); if (error) goto rename_error; @@ -2081,9 +2109,9 @@ create_error: name_buffer_ptr= m_name_buffer_ptr; for (abort_file= file, file= m_file; file < abort_file; file++) { - create_partition_name(from_buff, from_path, name_buffer_ptr, NORMAL_PART_NAME, - FALSE); - (void) (*file)->ha_delete_table((const char*) from_buff); + if (!create_partition_name(from_buff, sizeof(from_buff), from_path, + name_buffer_ptr, NORMAL_PART_NAME, FALSE)) + (void) (*file)->ha_delete_table(from_buff); name_buffer_ptr= strend(name_buffer_ptr) + 1; } DBUG_RETURN(error); @@ -2092,12 +2120,11 @@ rename_error: for (abort_file= file, file= m_file; file < abort_file; file++) { /* Revert the rename, back from 'to' to the original 'from' */ - create_partition_name(from_buff, from_path, name_buffer_ptr, - NORMAL_PART_NAME, FALSE); - create_partition_name(to_buff, to_path, name_buffer_ptr, - NORMAL_PART_NAME, FALSE); - /* Ignore error here */ - (void) (*file)->ha_rename_table(to_buff, from_buff); + if (!create_partition_name(from_buff, sizeof(from_buff), from_path, + name_buffer_ptr, NORMAL_PART_NAME, FALSE) && + !create_partition_name(to_buff, sizeof(to_buff), to_path, name_buffer_ptr, + NORMAL_PART_NAME, FALSE)) + (void) (*file)->ha_rename_table(to_buff, from_buff); name_buffer_ptr= strend(name_buffer_ptr) + 1; } DBUG_RETURN(error); @@ -2895,7 +2922,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) char *name_buffer_ptr; int error= HA_ERR_INITIALIZATION; handler **file; - char name_buff[FN_REFLEN]; + char name_buff[FN_REFLEN + 1]; bool is_not_tmp_table= (table_share->tmp_table == NO_TMP_TABLE); ulonglong check_table_flags; DBUG_ENTER("ha_partition::open"); @@ -2965,8 +2992,9 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) file= m_is_clone_of->m_file; for (i= 0; i < m_tot_parts; i++) { - create_partition_name(name_buff, name, name_buffer_ptr, NORMAL_PART_NAME, - FALSE); + if ((error= create_partition_name(name_buff, sizeof(name_buff), name, + name_buffer_ptr, NORMAL_PART_NAME, FALSE))) + goto err_handler; if (!(m_file[i]= file[i]->clone(name_buff, m_clone_mem_root))) { error= HA_ERR_INITIALIZATION; @@ -2981,8 +3009,9 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) file= m_file; do { - create_partition_name(name_buff, name, name_buffer_ptr, NORMAL_PART_NAME, - FALSE); + if ((error= create_partition_name(name_buff, sizeof(name_buff), name, + name_buffer_ptr, NORMAL_PART_NAME, FALSE))) + goto err_handler; table->s->connect_string = m_connect_string[(uint)(file-m_file)]; if ((error= (*file)->ha_open(table, name_buff, mode, test_if_locked))) goto err_handler; diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index e6513fc7476..0d9f6e6a1f5 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -5882,8 +5882,8 @@ static bool write_log_changed_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, DDL_LOG_ENTRY ddl_log_entry; partition_info *part_info= lpt->part_info; DDL_LOG_MEMORY_ENTRY *log_entry; - char tmp_path[FN_REFLEN]; - char normal_path[FN_REFLEN]; + char tmp_path[FN_REFLEN + 1]; + char normal_path[FN_REFLEN + 1]; List_iterator part_it(part_info->partitions); uint temp_partitions= part_info->temp_partitions.elements; uint num_elements= part_info->partitions.elements; @@ -5907,14 +5907,15 @@ static bool write_log_changed_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, ddl_log_entry.next_entry= *next_entry; ddl_log_entry.handler_name= ha_resolve_storage_engine_name(sub_elem->engine_type); - create_subpartition_name(tmp_path, path, - part_elem->partition_name, - sub_elem->partition_name, - TEMP_PART_NAME); - create_subpartition_name(normal_path, path, - part_elem->partition_name, - sub_elem->partition_name, - NORMAL_PART_NAME); + if (create_subpartition_name(tmp_path, sizeof(tmp_path), path, + part_elem->partition_name, + sub_elem->partition_name, + TEMP_PART_NAME) || + create_subpartition_name(normal_path, sizeof(normal_path), path, + part_elem->partition_name, + sub_elem->partition_name, + NORMAL_PART_NAME)) + DBUG_RETURN(TRUE); ddl_log_entry.name= normal_path; ddl_log_entry.from_name= tmp_path; if (part_elem->part_state == PART_IS_CHANGED) @@ -5935,12 +5936,13 @@ static bool write_log_changed_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, ddl_log_entry.next_entry= *next_entry; ddl_log_entry.handler_name= ha_resolve_storage_engine_name(part_elem->engine_type); - create_partition_name(tmp_path, path, - part_elem->partition_name, - TEMP_PART_NAME, TRUE); - create_partition_name(normal_path, path, - part_elem->partition_name, - NORMAL_PART_NAME, TRUE); + if (create_partition_name(tmp_path, sizeof(tmp_path), path, + part_elem->partition_name, TEMP_PART_NAME, + TRUE) || + create_partition_name(normal_path, sizeof(normal_path), path, + part_elem->partition_name, NORMAL_PART_NAME, + TRUE)) + DBUG_RETURN(TRUE); ddl_log_entry.name= normal_path; ddl_log_entry.from_name= tmp_path; if (part_elem->part_state == PART_IS_CHANGED) @@ -5979,7 +5981,7 @@ static bool write_log_dropped_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, DDL_LOG_ENTRY ddl_log_entry; partition_info *part_info= lpt->part_info; DDL_LOG_MEMORY_ENTRY *log_entry; - char tmp_path[FN_LEN]; + char tmp_path[FN_REFLEN + 1]; List_iterator part_it(part_info->partitions); List_iterator temp_it(part_info->temp_partitions); uint num_temp_partitions= part_info->temp_partitions.elements; @@ -6018,10 +6020,10 @@ static bool write_log_dropped_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, ddl_log_entry.next_entry= *next_entry; ddl_log_entry.handler_name= ha_resolve_storage_engine_name(sub_elem->engine_type); - create_subpartition_name(tmp_path, path, - part_elem->partition_name, - sub_elem->partition_name, - name_variant); + if (create_subpartition_name(tmp_path, sizeof(tmp_path), path, + part_elem->partition_name, + sub_elem->partition_name, name_variant)) + DBUG_RETURN(TRUE); ddl_log_entry.name= tmp_path; if (write_ddl_log_entry(&ddl_log_entry, &log_entry)) { @@ -6037,9 +6039,10 @@ static bool write_log_dropped_partitions(ALTER_PARTITION_PARAM_TYPE *lpt, ddl_log_entry.next_entry= *next_entry; ddl_log_entry.handler_name= ha_resolve_storage_engine_name(part_elem->engine_type); - create_partition_name(tmp_path, path, - part_elem->partition_name, - name_variant, TRUE); + if (create_partition_name(tmp_path, sizeof(tmp_path), path, + part_elem->partition_name, name_variant, + TRUE)) + DBUG_RETURN(TRUE); ddl_log_entry.name= tmp_path; if (write_ddl_log_entry(&ddl_log_entry, &log_entry)) { @@ -8117,31 +8120,41 @@ static uint32 get_next_subpartition_via_walking(PARTITION_ITERATOR *part_iter) } +/* used in error messages below */ +static const char *longest_str(const char *s1, const char *s2, + const char *s3=0) +{ + if (strlen(s2) > strlen(s1)) s1= s2; + if (s3 && strlen(s3) > strlen(s1)) s1= s3; + return s1; +} + /* Create partition names SYNOPSIS create_partition_name() - out:out Created partition name string + out:out The buffer for the created partition name string + must be *at least* of FN_REFLEN+1 bytes in1 First part in2 Second part name_variant Normal, temporary or renamed partition name RETURN VALUE - NONE + 0 if ok, error if name too long DESCRIPTION This method is used to calculate the partition name, service routine to the del_ren_cre_table method. */ -void create_partition_name(char *out, const char *in1, - const char *in2, uint name_variant, - bool translate) +int create_partition_name(char *out, size_t outlen, const char *in1, + const char *in2, uint name_variant, bool translate) { char transl_part_name[FN_REFLEN]; - const char *transl_part; + const char *transl_part, *end; + DBUG_ASSERT(outlen >= FN_REFLEN + 1); // consistency! same limit everywhere if (translate) { @@ -8151,11 +8164,17 @@ void create_partition_name(char *out, const char *in1, else transl_part= in2; if (name_variant == NORMAL_PART_NAME) - strxmov(out, in1, "#P#", transl_part, NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part, NullS); else if (name_variant == TEMP_PART_NAME) - strxmov(out, in1, "#P#", transl_part, "#TMP#", NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part, "#TMP#", NullS); else if (name_variant == RENAMED_PART_NAME) - strxmov(out, in1, "#P#", transl_part, "#REN#", NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part, "#REN#", NullS); + if (end - out == static_cast(outlen-1)) + { + my_error(ER_PATH_LENGTH, MYF(0), longest_str(in1, transl_part)); + return HA_WRONG_CREATE_OPTION; + } + return 0; } @@ -8164,37 +8183,46 @@ void create_partition_name(char *out, const char *in1, SYNOPSIS create_subpartition_name() - out:out Created partition name string + out:out The buffer for the created partition name string + must be *at least* of FN_REFLEN+1 bytes in1 First part in2 Second part in3 Third part name_variant Normal, temporary or renamed partition name RETURN VALUE - NONE + 0 if ok, error if name too long DESCRIPTION This method is used to calculate the subpartition name, service routine to the del_ren_cre_table method. */ -void create_subpartition_name(char *out, const char *in1, - const char *in2, const char *in3, - uint name_variant) +int create_subpartition_name(char *out, size_t outlen, + const char *in1, const char *in2, + const char *in3, uint name_variant) { - char transl_part_name[FN_REFLEN], transl_subpart_name[FN_REFLEN]; + char transl_part_name[FN_REFLEN], transl_subpart_name[FN_REFLEN], *end; + DBUG_ASSERT(outlen >= FN_REFLEN + 1); // consistency! same limit everywhere tablename_to_filename(in2, transl_part_name, FN_REFLEN); tablename_to_filename(in3, transl_subpart_name, FN_REFLEN); if (name_variant == NORMAL_PART_NAME) - strxmov(out, in1, "#P#", transl_part_name, - "#SP#", transl_subpart_name, NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part_name, + "#SP#", transl_subpart_name, NullS); else if (name_variant == TEMP_PART_NAME) - strxmov(out, in1, "#P#", transl_part_name, - "#SP#", transl_subpart_name, "#TMP#", NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part_name, + "#SP#", transl_subpart_name, "#TMP#", NullS); else if (name_variant == RENAMED_PART_NAME) - strxmov(out, in1, "#P#", transl_part_name, - "#SP#", transl_subpart_name, "#REN#", NullS); + end= strxnmov(out, outlen-1, in1, "#P#", transl_part_name, + "#SP#", transl_subpart_name, "#REN#", NullS); + if (end - out == static_cast(outlen-1)) + { + my_error(ER_PATH_LENGTH, MYF(0), + longest_str(in1, transl_part_name, transl_subpart_name)); + return HA_WRONG_CREATE_OPTION; + } + return 0; } uint get_partition_field_store_length(Field *field) diff --git a/sql/sql_partition.h b/sql/sql_partition.h index 951d58a655b..838006203c0 100644 --- a/sql/sql_partition.h +++ b/sql/sql_partition.h @@ -274,12 +274,12 @@ bool partition_key_modified(TABLE *table, const MY_BITMAP *fields); #define partition_key_modified(X,Y) 0 #endif -void create_partition_name(char *out, const char *in1, - const char *in2, uint name_variant, - bool translate); -void create_subpartition_name(char *out, const char *in1, - const char *in2, const char *in3, - uint name_variant); +int __attribute__((warn_unused_result)) + create_partition_name(char *out, size_t outlen, const char *in1, const char + *in2, uint name_variant, bool translate); +int __attribute__((warn_unused_result)) + create_subpartition_name(char *out, size_t outlen, const char *in1, const + char *in2, const char *in3, uint name_variant); void set_field_ptr(Field **ptr, const uchar *new_buf, const uchar *old_buf); void set_key_field_ptr(KEY *key_info, const uchar *new_buf,