From 0eadadad25d9e44232e1567897cf9dcb957ccdcd Mon Sep 17 00:00:00 2001 From: Debarun Banerjee Date: Wed, 24 Jun 2015 10:27:12 +0530 Subject: [PATCH] BUG#20310212 PARTITION DDL- CRASH AFTER THD::NOCHECK_REGISTER_ITEM_ Problem : --------- Issue-1: The root cause for the issues is that (col1 > 1) is not a valid partition function and we should have thrown error at much early stage [partition_info::check_partition_info]. We are not checking sub-partition expression when partition expression is NULL. Issue-2: Potential issue for future if any partition function needs to change item tree during open/fix_fields. We should release changed items, if any, before doing closefrm when we open the partitioned table during creation in create_table_impl. Solution : ---------- 1.check_partition_info() - Check for sub-partition expression even if no partition expression. [partition by ... columns(...) subpartition by hash()] 2.create_table_impl() - Assert that the change list is empty before doing closefrm for partitioned table. Currently no supported partition function seems to be changing item tree during open. Reviewed-by: Mattias Jonsson RB: 9345 --- mysql-test/r/partition_error.result | 2 +- mysql-test/t/partition_error.test | 2 +- sql/partition_info.cc | 15 +++++++++++---- sql/sql_table.cc | 6 ++++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/partition_error.result b/mysql-test/r/partition_error.result index 0e4f89b70db..1e0f2cbea10 100644 --- a/mysql-test/r/partition_error.result +++ b/mysql-test/r/partition_error.result @@ -1089,7 +1089,7 @@ partition by key (a) subpartition by hash (sin(a+b)) (partition x1 (subpartition x11, subpartition x12), partition x2 (subpartition x21, subpartition x22)); -ERROR HY000: It is only possible to mix RANGE/LIST partitioning with HASH/KEY partitioning for subpartitioning +ERROR HY000: This partition function is not allowed select load_file('$MYSQLD_DATADIR/test/t1.par'); load_file('$MYSQLD_DATADIR/test/t1.par') NULL diff --git a/mysql-test/t/partition_error.test b/mysql-test/t/partition_error.test index c0f49a4d414..9a6939032a3 100644 --- a/mysql-test/t/partition_error.test +++ b/mysql-test/t/partition_error.test @@ -1145,7 +1145,7 @@ subpartition by hash (rand(a+b)); # # Subpartition by hash, wrong subpartition function # ---error ER_SUBPARTITION_ERROR +--error ER_PARTITION_FUNCTION_IS_NOT_ALLOWED CREATE TABLE t1 ( a int not null, b int not null, diff --git a/sql/partition_info.cc b/sql/partition_info.cc index 958f77f5bd0..336010cc7dd 100644 --- a/sql/partition_info.cc +++ b/sql/partition_info.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2006, 2013, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2006, 2015, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -1109,15 +1109,22 @@ bool partition_info::check_partition_info(THD *thd, handlerton **eng_type, { int err= 0; + /* Check for partition expression. */ if (!list_of_part_fields) { DBUG_ASSERT(part_expr); err= part_expr->walk(&Item::check_partition_func_processor, 0, NULL); - if (!err && is_sub_partitioned() && !list_of_subpart_fields) - err= subpart_expr->walk(&Item::check_partition_func_processor, 0, - NULL); } + + /* Check for sub partition expression. */ + if (!err && is_sub_partitioned() && !list_of_subpart_fields) + { + DBUG_ASSERT(subpart_expr); + err= subpart_expr->walk(&Item::check_partition_func_processor, 0, + NULL); + } + if (err) { my_error(ER_PARTITION_FUNCTION_IS_NOT_ALLOWED, MYF(0)); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b8858cffce6..2951d921e15 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3913,6 +3913,12 @@ static bool check_if_created_table_can_be_opened(THD *thd, result= (open_table_def(thd, &share, 0) || open_table_from_share(thd, &share, "", 0, (uint) READ_ALL, 0, &table, TRUE)); + /* + Assert that the change list is empty as no partition function currently + needs to modify item tree. May need call THD::rollback_item_tree_changes + later before calling closefrm if the change list is not empty. + */ + DBUG_ASSERT(thd->change_list.is_empty()); if (! result) (void) closefrm(&table, 0);