From 5d8b06241e064aee085811031de7c4723a3adcce Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Tue, 11 Dec 2018 15:06:45 +0300 Subject: [PATCH] MDEV-15563 Case 4 prototype: UNSIGNED -> bigger signed * Serialise unsigned_len * Test cases --- .../innodb/r/instant_alter_extend.result | 62 +++++++++++++++++++ .../suite/innodb/t/instant_alter_extend.test | 34 ++++++++++ sql/field.cc | 18 ++++-- storage/innobase/dict/dict0mem.cc | 2 + storage/innobase/handler/handler0alter.cc | 22 ++++++- storage/innobase/include/dict0mem.h | 24 ++++++- storage/innobase/include/row0sel.h | 15 +---- storage/innobase/row/row0sel.cc | 16 +++-- 8 files changed, 161 insertions(+), 32 deletions(-) diff --git a/mysql-test/suite/innodb/r/instant_alter_extend.result b/mysql-test/suite/innodb/r/instant_alter_extend.result index 1798f47104a..361b5f8e759 100644 --- a/mysql-test/suite/innodb/r/instant_alter_extend.result +++ b/mysql-test/suite/innodb/r/instant_alter_extend.result @@ -186,6 +186,68 @@ test.t2 check status OK call check_table('t2'); name mtype prtype len x 6 403 4 +# Case 4: instant conversion unsigned -> signed +create or replace table t(x int) row_format=redundant; +alter table t modify x int unsigned, algorithm=instant; +ERROR 0A000: ALGORITHM=INSTANT is not supported. Reason: Cannot change column type INPLACE. Try ALGORITHM=COPY +alter table t modify x bigint unsigned, algorithm=instant; +ERROR 0A000: ALGORITHM=INSTANT is not supported. Reason: Cannot change column type INPLACE. Try ALGORITHM=COPY +create or replace table t(x tinyint unsigned) row_format=redundant; +insert into t values (255); +alter table t modify x smallint unsigned, algorithm=instant; +insert into t values (255); +select * from t; +x +255 +255 +check table t extended; +Table Op Msg_type Msg_text +test.t check status OK +call check_table('t'); +name mtype prtype len +x 6 602 2 +alter table t modify x mediumint, algorithm=instant; +insert into t values (255); +select * from t; +x +255 +255 +255 +check table t extended; +Table Op Msg_type Msg_text +test.t check status OK +call check_table('t'); +name mtype prtype len +x 6 409 3 +alter table t modify x int, algorithm=instant; +insert into t values (255); +select * from t; +x +255 +255 +255 +255 +check table t extended; +Table Op Msg_type Msg_text +test.t check status OK +call check_table('t'); +name mtype prtype len +x 6 403 4 +alter table t modify x bigint, algorithm=instant; +insert into t values (255); +select * from t; +x +255 +255 +255 +255 +255 +check table t extended; +Table Op Msg_type Msg_text +test.t check status OK +call check_table('t'); +name mtype prtype len +x 6 408 8 # Check innobase_col_to_mysql() len < flen create or replace table t1 (x mediumint); insert into t1 values (1); diff --git a/mysql-test/suite/innodb/t/instant_alter_extend.test b/mysql-test/suite/innodb/t/instant_alter_extend.test index 4ffa39c4f13..fb866d53a6f 100644 --- a/mysql-test/suite/innodb/t/instant_alter_extend.test +++ b/mysql-test/suite/innodb/t/instant_alter_extend.test @@ -175,6 +175,40 @@ select * from t2; check table t2 extended; call check_table('t2'); +--echo # Case 4: instant conversion unsigned -> signed +create or replace table t(x int) row_format=redundant; +--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON +alter table t modify x int unsigned, algorithm=instant; +--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON +alter table t modify x bigint unsigned, algorithm=instant; + +create or replace table t(x tinyint unsigned) row_format=redundant; +insert into t values (255); + +alter table t modify x smallint unsigned, algorithm=instant; +insert into t values (255); +select * from t; +check table t extended; +call check_table('t'); + +alter table t modify x mediumint, algorithm=instant; +insert into t values (255); +select * from t; +check table t extended; +call check_table('t'); + +alter table t modify x int, algorithm=instant; +insert into t values (255); +select * from t; +check table t extended; +call check_table('t'); + +alter table t modify x bigint, algorithm=instant; +insert into t values (255); +select * from t; +check table t extended; +call check_table('t'); + --echo # Check innobase_col_to_mysql() len < flen create or replace table t1 (x mediumint); insert into t1 values (1); diff --git a/sql/field.cc b/sql/field.cc index eef1061be4a..358dec99297 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -9499,19 +9499,27 @@ bool Field_num::eq_def(const Field *field) const uint Field_num::is_equal(Create_field *new_field) { - if ((new_field->flags ^ flags) & (UNSIGNED_FLAG | AUTO_INCREMENT_FLAG)) + if ((new_field->flags ^ flags) & AUTO_INCREMENT_FLAG) + return IS_EQUAL_NO; + if (new_field->pack_length < pack_length()) return IS_EQUAL_NO; const Type_handler *th= type_handler(), *new_th = new_field->type_handler(); - if (th == new_th && new_field->pack_length == pack_length()) + const bool signed_equal= !((new_field->flags ^ flags) & UNSIGNED_FLAG); + if (signed_equal && th == new_th && new_field->pack_length == pack_length()) return IS_EQUAL_YES; if (table->file->ha_table_flags() & HA_EXTENDED_TYPES_CONVERSION) { - if (th->result_type() == new_th->result_type() && - new_field->pack_length >= pack_length()) - return IS_EQUAL_PACK_LENGTH_EXT; + if (th->result_type() == new_th->result_type()) + { + if (signed_equal) + return IS_EQUAL_PACK_LENGTH_EXT; + if (!(new_field->flags & UNSIGNED_FLAG) && + new_field->pack_length > pack_length()) + return IS_EQUAL_PACK_LENGTH_EXT; + } } return IS_EQUAL_NO; diff --git a/storage/innobase/dict/dict0mem.cc b/storage/innobase/dict/dict0mem.cc index 3d373bf37d8..94070aeefdd 100644 --- a/storage/innobase/dict/dict0mem.cc +++ b/storage/innobase/dict/dict0mem.cc @@ -714,6 +714,7 @@ dict_mem_fill_column_struct( dtype_get_mblen(mtype, prtype, &mbminlen, &mbmaxlen); column->mbminlen = mbminlen; column->mbmaxlen = mbmaxlen; + column->unsigned_len = 0; column->def_val.data = NULL; column->def_val.len = UNIV_SQL_DEFAULT; ut_ad(!column->is_dropped()); @@ -1217,6 +1218,7 @@ inline void dict_index_t::reconstruct_fields() n_nullable++; n_core_null += i <= n_core_fields; } + f.col->unsigned_len = c.unsigned_len(); } fields = tfields; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 30ca3975bf6..788c204c84c 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -184,12 +184,16 @@ inline void dict_table_t::init_instant(const dict_table_t& table) ut_d(unsigned n_nullable = 0); for (unsigned i = u; i < index.n_fields; i++) { auto& f = index.fields[i]; + /* FIXME: CHAR of arbitrary size is now allowed. */ DBUG_ASSERT(dict_col_get_fixed_size(f.col, not_redundant()) - <= DICT_MAX_FIXED_COL_LEN); + <= DICT_MAX_FIXED_COL_LEN + || !not_redundant()); ut_d(n_nullable += f.col->is_nullable()); if (!f.col->is_dropped()) { - (*field_map_it++).set_ind(f.col->ind); + field_map_it->set_ind(f.col->ind); + field_map_it->or_unsigned_len(f.col->unsigned_len); + field_map_it++; continue; } @@ -541,6 +545,16 @@ inline bool dict_table_t::instant_column(const dict_table_t& table, c.def_val = o->def_val; ut_ad(c.same_format(*o, !not_redundant())); + if (o->mtype == DATA_INT && !(c.prtype & DATA_UNSIGNED)) { + DBUG_ASSERT(c.mtype == o->mtype); + if (o->prtype & DATA_UNSIGNED) { + DBUG_ASSERT(c.len > o->len); + c.unsigned_len = o->len; + } else { + c.unsigned_len = o->unsigned_len; + } + } + if (o->vers_sys_start()) { ut_ad(o->ind == vers_start); vers_start = i; @@ -2960,7 +2974,7 @@ innobase_col_to_mysql( *--ptr = *data++; } - if (!(col->prtype & DATA_UNSIGNED)) { + if (!(col->prtype & DATA_UNSIGNED) && len > col->unsigned_len) { ((byte*) dest)[len - 1] ^= 0x80; } @@ -4244,6 +4258,8 @@ innobase_build_col_map( } col_map[old_i - num_old_v] = i; + new_table->cols[i].unsigned_len = + old_table->cols[old_i].unsigned_len; if (old_table->versioned() && altered_table->versioned()) { if (old_i == old_table->vers_start) { diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 3314b8e2459..2ad1c5fd8a5 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -581,6 +581,9 @@ struct dict_col_t{ this column. Our current max limit is 3072 (REC_VERSION_56_MAX_INDEX_COL_LEN) bytes. */ + unsigned unsigned_len:16; /*!< before ALTER to signed the field + was unsigned and was of this length. */ + private: /** Special value of ind for a dropped column */ static const unsigned DROPPED = 1023; @@ -1592,6 +1595,15 @@ class field_map_element_t /** Set if the column was dropped and originally declared NOT NULL */ static constexpr uint16_t NOT_NULL = 1U << (IND_BITS + 4); + /** The field was unsigned before ALTER to signed: + 0: never + 1: tinyint + 2: smallint + 3: mediumint + 4: int + */ + static constexpr uint16_t UNSIGNED_SIZE = 7U << (IND_BITS + 1); + /** Column index (if !(data & DROPPED)): table->cols[data & IND], or field length (if (data & DROPPED)): (data & IND) = 0 if variable-length with max_len < 256 bytes; @@ -1608,6 +1620,15 @@ class field_map_element_t void set_dropped() { data |= DROPPED; } bool is_not_null() const { return data & NOT_NULL; } void set_not_null() { ut_ad(is_dropped()); data |= NOT_NULL; } + uint16_t unsigned_len() const + { + return (data & UNSIGNED_SIZE) >> (IND_BITS + 1); + } + void or_unsigned_len(uint16_t unsigned_len) + { + ut_ad(unsigned_len <= 4); + data |= unsigned_len << (IND_BITS + 1); + } uint16_t ind() const { return data & IND; } void set_ind(uint16_t i) { @@ -1745,7 +1766,8 @@ struct dict_table_t { const ulint* col_map, unsigned& first_alter_pos); - /** Adjust table metadata for instant ADD/DROP/reorder COLUMN. + /** Adjust table metadata for instant ADD/DROP/reorder COLUMN, + unsigned -> bigger signed. @param[in] table table on which prepare_instant() was invoked @param[in] col_map mapping from cols[] and v_cols[] to table @return whether the metadata record must be updated */ diff --git a/storage/innobase/include/row0sel.h b/storage/innobase/include/row0sel.h index 1e58686dfcb..c14ac35598a 100644 --- a/storage/innobase/include/row0sel.h +++ b/storage/innobase/include/row0sel.h @@ -438,23 +438,12 @@ enum row_sel_match_mode { of a fixed length column) */ }; -#ifdef UNIV_DEBUG -/** Convert a non-SQL-NULL field from Innobase format to MySQL format. */ -# define row_sel_field_store_in_mysql_format(dest,templ,idx,field,src,len) \ - row_sel_field_store_in_mysql_format_func(dest,templ,idx,field,src,len) -#else /* UNIV_DEBUG */ -/** Convert a non-SQL-NULL field from Innobase format to MySQL format. */ -# define row_sel_field_store_in_mysql_format(dest,templ,idx,field,src,len) \ - row_sel_field_store_in_mysql_format_func(dest,templ,src,len) -#endif /* UNIV_DEBUG */ - /**************************************************************//** Stores a non-SQL-NULL field in the MySQL format. The counterpart of this function is row_mysql_store_col_in_innobase_format() in row0mysql.cc. */ void -row_sel_field_store_in_mysql_format_func( -/*=====================================*/ +row_sel_field_store_in_mysql_format( byte* dest, /*!< in/out: buffer where to store; NOTE that BLOBs are not in themselves stored here: the caller must allocate @@ -466,14 +455,12 @@ row_sel_field_store_in_mysql_format_func( Its following fields are referenced: type, is_unsigned, mysql_col_len, mbminlen, mbmaxlen */ -#ifdef UNIV_DEBUG const dict_index_t* index, /*!< in: InnoDB index */ ulint field_no, /*!< in: templ->rec_field_no or templ->clust_rec_field_no or templ->icp_rec_field_no */ -#endif /* UNIV_DEBUG */ const byte* data, /*!< in: data to store */ ulint len); /*!< in: length of the data */ diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index a03a4fa4acf..10735684abe 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -2698,22 +2698,18 @@ row_sel_convert_mysql_key_to_innobase( Stores a non-SQL-NULL field in the MySQL format. The counterpart of this function is row_mysql_store_col_in_innobase_format() in row0mysql.cc. */ void -row_sel_field_store_in_mysql_format_func( +row_sel_field_store_in_mysql_format( byte* dest, const mysql_row_templ_t* templ, -#ifdef UNIV_DEBUG const dict_index_t* index, ulint field_no, -#endif /* UNIV_DEBUG */ const byte* data, ulint len) { byte* ptr; -#ifdef UNIV_DEBUG - const dict_field_t* field - = templ->is_virtual - ? NULL : dict_index_get_nth_field(index, field_no); -#endif /* UNIV_DEBUG */ + const dict_field_t* field = templ->is_virtual + ? NULL : dict_index_get_nth_field(index, field_no); + uint16_t unsigned_len; ut_ad(len != UNIV_SQL_NULL); UNIV_MEM_ASSERT_RW(data, len); @@ -2738,7 +2734,9 @@ row_sel_field_store_in_mysql_format_func( data++; } - if (!templ->is_unsigned) { + unsigned_len = field ? field->col->unsigned_len : 0; + + if (!templ->is_unsigned && len > unsigned_len) { dest[len - 1] = (byte) (dest[len - 1] ^ 128); } -- 2.20.1