From 3ae219848322dea1e42b9ba25e3066bbc9aa217d Mon Sep 17 00:00:00 2001 From: Aakanksha Verma Date: Wed, 30 Jan 2019 23:09:57 +0530 Subject: [PATCH 1/4] Bug #19811005 ALTER TABLE ADD INDEX DOES NOT UPDATE INDEX_LENGTH IN I_S TABLES PROBLEM ======= An add index doesn't update index length stats in information schema TABLES table. FIX === Update the dict_table_t variable with index length stats that is actually calculated post alter . As this variable is used to populated the information schema index length statistics. Reviewed by: Bin su RB: 21277 --- storage/innobase/dict/dict0stats.cc | 5 +++-- storage/xtradb/dict/dict0stats.cc | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/storage/innobase/dict/dict0stats.cc b/storage/innobase/dict/dict0stats.cc index 364add4791d..06364689173 100644 --- a/storage/innobase/dict/dict0stats.cc +++ b/storage/innobase/dict/dict0stats.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2009, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2009, 2019, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2015, 2017, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under @@ -2499,7 +2499,6 @@ dict_stats_report_error( return (err); } - /** Save the table's statistics into the persistent statistics storage. @param[in] table_orig table whose stats to save @param[in] only_for_index if this is non-NULL, then stats for indexes @@ -3208,6 +3207,8 @@ dict_stats_update_for_index( if (dict_stats_persistent_storage_check(false)) { dict_table_stats_lock(index->table, RW_X_LATCH); dict_stats_analyze_index(index); + index->table->stat_sum_of_other_index_sizes + += index->stat_index_size; dict_table_stats_unlock(index->table, RW_X_LATCH); dict_stats_save(index->table, &index->id); DBUG_VOID_RETURN; diff --git a/storage/xtradb/dict/dict0stats.cc b/storage/xtradb/dict/dict0stats.cc index c1463e98ce0..06364689173 100644 --- a/storage/xtradb/dict/dict0stats.cc +++ b/storage/xtradb/dict/dict0stats.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2009, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2009, 2019, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2015, 2017, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under @@ -3207,6 +3207,8 @@ dict_stats_update_for_index( if (dict_stats_persistent_storage_check(false)) { dict_table_stats_lock(index->table, RW_X_LATCH); dict_stats_analyze_index(index); + index->table->stat_sum_of_other_index_sizes + += index->stat_index_size; dict_table_stats_unlock(index->table, RW_X_LATCH); dict_stats_save(index->table, &index->id); DBUG_VOID_RETURN; @@ -4006,7 +4008,6 @@ dict_stats_save_defrag_stats( { dberr_t ret; - if (index->is_readable()) { } else { return (dict_stats_report_error(index->table, true)); From ac97ad4eabc8da788b1239f4cc6ad2ff5cf11da0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 25 Apr 2019 13:16:44 +0300 Subject: [PATCH 2/4] Bug#19811005: Add a simple test case --- mysql-test/suite/innodb/r/innodb-index.result | 27 +++++++++++++++++++ mysql-test/suite/innodb/t/innodb-index.test | 20 ++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/mysql-test/suite/innodb/r/innodb-index.result b/mysql-test/suite/innodb/r/innodb-index.result index 52344e1e471..64c5586173e 100644 --- a/mysql-test/suite/innodb/r/innodb-index.result +++ b/mysql-test/suite/innodb/r/innodb-index.result @@ -1213,3 +1213,30 @@ test.t1 check status OK DROP TABLE t1; SET GLOBAL innodb_file_format=@save_format; SET GLOBAL innodb_large_prefix=@save_prefix; +# +# Bug#19811005 ALTER TABLE ADD INDEX DOES NOT UPDATE INDEX_LENGTH +# IN I_S TABLES +# +CREATE TABLE t1(a INT, b INT) ENGINE=INNODB, STATS_PERSISTENT=1; +SELECT cast(DATA_LENGTH/@@innodb_page_size as int) D, +cast(INDEX_LENGTH/@@innodb_page_size as int) I +FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA='test'; +D I +1 0 +ALTER TABLE t1 ADD INDEX (a); +affected rows: 0 +info: Records: 0 Duplicates: 0 Warnings: 0 +SELECT cast(DATA_LENGTH/@@innodb_page_size as int) D, +cast(INDEX_LENGTH/@@innodb_page_size as int) I +FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA='test'; +D I +1 1 +ALTER TABLE t1 ADD INDEX (b); +affected rows: 0 +info: Records: 0 Duplicates: 0 Warnings: 0 +SELECT cast(DATA_LENGTH/@@innodb_page_size as int) D, +cast(INDEX_LENGTH/@@innodb_page_size as int) I +FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA='test'; +D I +1 2 +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/innodb-index.test b/mysql-test/suite/innodb/t/innodb-index.test index d28930de815..86ac1a8123e 100644 --- a/mysql-test/suite/innodb/t/innodb-index.test +++ b/mysql-test/suite/innodb/t/innodb-index.test @@ -593,3 +593,23 @@ CHECK TABLE t1; DROP TABLE t1; SET GLOBAL innodb_file_format=@save_format; SET GLOBAL innodb_large_prefix=@save_prefix; + +--echo # +--echo # Bug#19811005 ALTER TABLE ADD INDEX DOES NOT UPDATE INDEX_LENGTH +--echo # IN I_S TABLES +--echo # +let $i_s_query=SELECT cast(DATA_LENGTH/@@innodb_page_size as int) D, +cast(INDEX_LENGTH/@@innodb_page_size as int) I +FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA='test'; + +CREATE TABLE t1(a INT, b INT) ENGINE=INNODB, STATS_PERSISTENT=1; +eval $i_s_query; +--enable_info +ALTER TABLE t1 ADD INDEX (a); +--disable_info +eval $i_s_query; +--enable_info +ALTER TABLE t1 ADD INDEX (b); +--disable_info +eval $i_s_query; +DROP TABLE t1; From 9e7bcb05d409faec0d4bac7578afad355796068a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 25 Apr 2019 14:01:55 +0300 Subject: [PATCH 3/4] Clean up ib_sequence::m_max_value Correctly document the usage of m_max_value. Remove the const qualifier, so that the implicit assignment operator can be used. Make all members of ib_sequence private, and add an accessor member function max_value(). --- storage/innobase/handler/handler0alter.cc | 2 +- storage/innobase/include/handler0alter.h | 11 ++++++++--- storage/xtradb/handler/handler0alter.cc | 2 +- storage/xtradb/include/handler0alter.h | 11 ++++++++--- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 91992058889..fbded2f0a53 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -2721,7 +2721,7 @@ prepare_inplace_alter_table_dict( (ha_alter_info->handler_ctx); DBUG_ASSERT((ctx->add_autoinc != ULINT_UNDEFINED) - == (ctx->sequence.m_max_value > 0)); + == (ctx->sequence.max_value() > 0)); DBUG_ASSERT(!ctx->num_to_drop_index == !ctx->drop_index); DBUG_ASSERT(!ctx->num_to_drop_fk == !ctx->drop_fk); DBUG_ASSERT(!add_fts_doc_id || add_fts_doc_id_idx); diff --git a/storage/innobase/include/handler0alter.h b/storage/innobase/include/handler0alter.h index 3dd6c99eb6d..63379ad9371 100644 --- a/storage/innobase/include/handler0alter.h +++ b/storage/innobase/include/handler0alter.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 2005, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2019, MariaDB Corporation. 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 the Free Software @@ -96,9 +97,13 @@ struct ib_sequence_t { return(m_next_value); } - /** Maximum calumn value if adding an AUTOINC column else 0. Once - we reach the end of the sequence it will be set to ~0. */ - const ulonglong m_max_value; + /** @return maximum column value + @retval 0 if not adding AUTO_INCREMENT column */ + ulonglong max_value() const { return m_max_value; } + +private: + /** Maximum value if adding an AUTO_INCREMENT column, else 0 */ + ulonglong m_max_value; /** Value of auto_increment_increment */ ulong m_increment; diff --git a/storage/xtradb/handler/handler0alter.cc b/storage/xtradb/handler/handler0alter.cc index afe1bdac2e4..0a246d897c1 100644 --- a/storage/xtradb/handler/handler0alter.cc +++ b/storage/xtradb/handler/handler0alter.cc @@ -2727,7 +2727,7 @@ prepare_inplace_alter_table_dict( (ha_alter_info->handler_ctx); DBUG_ASSERT((ctx->add_autoinc != ULINT_UNDEFINED) - == (ctx->sequence.m_max_value > 0)); + == (ctx->sequence.max_value() > 0)); DBUG_ASSERT(!ctx->num_to_drop_index == !ctx->drop_index); DBUG_ASSERT(!ctx->num_to_drop_fk == !ctx->drop_fk); DBUG_ASSERT(!add_fts_doc_id || add_fts_doc_id_idx); diff --git a/storage/xtradb/include/handler0alter.h b/storage/xtradb/include/handler0alter.h index 3dd6c99eb6d..63379ad9371 100644 --- a/storage/xtradb/include/handler0alter.h +++ b/storage/xtradb/include/handler0alter.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 2005, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2019, MariaDB Corporation. 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 the Free Software @@ -96,9 +97,13 @@ struct ib_sequence_t { return(m_next_value); } - /** Maximum calumn value if adding an AUTOINC column else 0. Once - we reach the end of the sequence it will be set to ~0. */ - const ulonglong m_max_value; + /** @return maximum column value + @retval 0 if not adding AUTO_INCREMENT column */ + ulonglong max_value() const { return m_max_value; } + +private: + /** Maximum value if adding an AUTO_INCREMENT column, else 0 */ + ulonglong m_max_value; /** Value of auto_increment_increment */ ulong m_increment; From 1cd31bc13248bef52f7b62a37c394bd302488b15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 25 Apr 2019 14:04:44 +0300 Subject: [PATCH 4/4] Bug#28573894 ALTER PARTITIONED TABLE ADD AUTO_INCREMENT DIFF RESULT DEPENDING ON ALGORITHM For partitioned table, ensure that the AUTO_INCREMENT values will be assigned from the same sequence. This is based on the following change in MySQL 5.6.44: commit aaba359c13d9200747a609730dafafc3b63cd4d6 Author: Rahul Malik Date: Mon Feb 4 13:31:41 2019 +0530 Bug#28573894 ALTER PARTITIONED TABLE ADD AUTO_INCREMENT DIFF RESULT DEPENDING ON ALGORITHM Problem: When a partition table is in-place altered to add an auto-increment column, then its values are starting over for each partition. Analysis: In the case of in-place alter, InnoDB is creating a new sequence object for each partition. It is default initialized. So auto-increment columns start over for each partition. Fix: Assign old sequence of the partition to the sequence of next partition so it won't start over. RB#21148 Reviewed by Bin Su --- mysql-test/r/partition_innodb.result | 42 +++++++++++++++++++++++ mysql-test/t/partition_innodb.test | 24 +++++++++++++ sql/ha_partition.cc | 11 ++++-- sql/handler.h | 5 +-- storage/innobase/handler/handler0alter.cc | 19 +++++++++- storage/xtradb/handler/handler0alter.cc | 19 +++++++++- 6 files changed, 113 insertions(+), 7 deletions(-) diff --git a/mysql-test/r/partition_innodb.result b/mysql-test/r/partition_innodb.result index c00a75fed90..5deab4d84fc 100644 --- a/mysql-test/r/partition_innodb.result +++ b/mysql-test/r/partition_innodb.result @@ -918,3 +918,45 @@ ERROR HY000: CHECK OPTION failed 'test.v' SET GLOBAL innodb_stats_persistent= @save_isp; DROP view v; DROP TABLE t; +# +# Bug#28573894 ALTER PARTITIONED TABLE ADD AUTO_INCREMENT DIFF RESULT +# +CREATE TABLE t (a VARCHAR(10) NOT NULL,b INT,PRIMARY KEY (b)) ENGINE=INNODB +PARTITION BY RANGE (b) +(PARTITION pa VALUES LESS THAN (2), +PARTITION pb VALUES LESS THAN (20), +PARTITION pc VALUES LESS THAN (30), +PARTITION pd VALUES LESS THAN (40)); +INSERT INTO t +VALUES('A',0),('B',1),('C',2),('D',3),('E',4),('F',5),('G',25),('H',35); +CREATE TABLE t_copy LIKE t; +INSERT INTO t_copy SELECT * FROM t; +ALTER TABLE t ADD COLUMN r INT UNSIGNED NOT NULL AUTO_INCREMENT, +ADD UNIQUE KEY (r,b); +affected rows: 0 +info: Records: 0 Duplicates: 0 Warnings: 0 +ALTER TABLE t_copy ADD COLUMN r INT UNSIGNED NOT NULL AUTO_INCREMENT, +ADD UNIQUE KEY (r,b), ALGORITHM=COPY; +affected rows: 8 +info: Records: 8 Duplicates: 0 Warnings: 0 +SELECT * FROM t; +a b r +A 0 1 +B 1 2 +C 2 3 +D 3 4 +E 4 5 +F 5 6 +G 25 7 +H 35 8 +SELECT * FROM t_copy; +a b r +A 0 1 +B 1 2 +C 2 3 +D 3 4 +E 4 5 +F 5 6 +G 25 7 +H 35 8 +DROP TABLE t,t_copy; diff --git a/mysql-test/t/partition_innodb.test b/mysql-test/t/partition_innodb.test index 54994bc5a66..15c9625770b 100644 --- a/mysql-test/t/partition_innodb.test +++ b/mysql-test/t/partition_innodb.test @@ -1028,3 +1028,27 @@ SET GLOBAL innodb_stats_persistent= @save_isp; DROP view v; DROP TABLE t; +--echo # +--echo # Bug#28573894 ALTER PARTITIONED TABLE ADD AUTO_INCREMENT DIFF RESULT +--echo # +CREATE TABLE t (a VARCHAR(10) NOT NULL,b INT,PRIMARY KEY (b)) ENGINE=INNODB +PARTITION BY RANGE (b) +(PARTITION pa VALUES LESS THAN (2), + PARTITION pb VALUES LESS THAN (20), + PARTITION pc VALUES LESS THAN (30), + PARTITION pd VALUES LESS THAN (40)); + +INSERT INTO t +VALUES('A',0),('B',1),('C',2),('D',3),('E',4),('F',5),('G',25),('H',35); +CREATE TABLE t_copy LIKE t; +INSERT INTO t_copy SELECT * FROM t; + +--enable_info +ALTER TABLE t ADD COLUMN r INT UNSIGNED NOT NULL AUTO_INCREMENT, +ADD UNIQUE KEY (r,b); +ALTER TABLE t_copy ADD COLUMN r INT UNSIGNED NOT NULL AUTO_INCREMENT, +ADD UNIQUE KEY (r,b), ALGORITHM=COPY; +--disable_info +SELECT * FROM t; +SELECT * FROM t_copy; +DROP TABLE t,t_copy; diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 2167bea8d7c..80bf7ef17fe 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -1,6 +1,6 @@ /* - Copyright (c) 2005, 2017, Oracle and/or its affiliates. - Copyright (c) 2009, 2017, MariaDB + Copyright (c) 2005, 2019, Oracle and/or its affiliates. + Copyright (c) 2009, 2019, MariaDB 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 @@ -8329,7 +8329,12 @@ bool ha_partition::inplace_alter_table(TABLE *altered_table, for (index= 0; index < m_tot_parts && !error; index++) { - ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index]; + if ((ha_alter_info->handler_ctx= + part_inplace_ctx->handler_ctx_array[index]) != NULL + && index != 0) + ha_alter_info->handler_ctx->set_shared_data + (*part_inplace_ctx->handler_ctx_array[index - 1]); + if (m_file[index]->ha_inplace_alter_table(altered_table, ha_alter_info)) error= true; diff --git a/sql/handler.h b/sql/handler.h index af492e75da6..99dfefd4c5a 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1,8 +1,8 @@ #ifndef HANDLER_INCLUDED #define HANDLER_INCLUDED /* - Copyright (c) 2000, 2016, Oracle and/or its affiliates. - Copyright (c) 2009, 2018, MariaDB + Copyright (c) 2000, 2019, Oracle and/or its affiliates. + Copyright (c) 2009, 2019, MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License @@ -1833,6 +1833,7 @@ public: inplace_alter_handler_ctx() {} virtual ~inplace_alter_handler_ctx() {} + virtual void set_shared_data(const inplace_alter_handler_ctx& ctx) {} }; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index fbded2f0a53..0d0982fa498 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2005, 2018, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2005, 2019, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2013, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under @@ -2178,6 +2178,23 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx @return whether the table will be rebuilt */ bool need_rebuild () const { return(old_table != new_table); } + /** Share context between partitions. + @param[in] ctx context from another partition of the table */ + void set_shared_data(const inplace_alter_handler_ctx& ctx) + { + if (add_autoinc != ULINT_UNDEFINED) { + const ha_innobase_inplace_ctx& ha_ctx = + static_cast + (ctx); + /* When adding an AUTO_INCREMENT column to a + partitioned InnoDB table, we must share the + sequence for all partitions. */ + ut_ad(ha_ctx.add_autoinc == add_autoinc); + ut_ad(ha_ctx.sequence.last()); + sequence = ha_ctx.sequence; + } + } + private: // Disable copying ha_innobase_inplace_ctx(const ha_innobase_inplace_ctx&); diff --git a/storage/xtradb/handler/handler0alter.cc b/storage/xtradb/handler/handler0alter.cc index 0a246d897c1..808f8529a77 100644 --- a/storage/xtradb/handler/handler0alter.cc +++ b/storage/xtradb/handler/handler0alter.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2005, 2018, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2005, 2019, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2017, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under @@ -2181,6 +2181,23 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx @return whether the table will be rebuilt */ bool need_rebuild () const { return(old_table != new_table); } + /** Share context between partitions. + @param[in] ctx context from another partition of the table */ + void set_shared_data(const inplace_alter_handler_ctx& ctx) + { + if (add_autoinc != ULINT_UNDEFINED) { + const ha_innobase_inplace_ctx& ha_ctx = + static_cast + (ctx); + /* When adding an AUTO_INCREMENT column to a + partitioned InnoDB table, we must share the + sequence for all partitions. */ + ut_ad(ha_ctx.add_autoinc == add_autoinc); + ut_ad(ha_ctx.sequence.last()); + sequence = ha_ctx.sequence; + } + } + private: // Disable copying ha_innobase_inplace_ctx(const ha_innobase_inplace_ctx&);