summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNikita Malyavin <nikitamalyavin@gmail.com>2021-07-06 10:17:22 +0300
committerNikita Malyavin <nikitamalyavin@gmail.com>2021-07-28 15:50:01 +0300
commit122457d517dbf47a96f302a877d75f11ad9ced6a (patch)
tree8c0f8e5d776abee168724e22ef520001207f0a88
parentbeb401b25fa3e34ea431da990f1ead6bbc23a82f (diff)
downloadmariadb-git-122457d517dbf47a96f302a877d75f11ad9ced6a.tar.gz
MDEV-20154 Assertion `len <= col->len | ...` failed in row_merge_buf_add
10.6 version len was containing garbage, since vctempl->mysql_col_offset was containing old value while calling row_mysql_store_col_in_innobase_format from innobase_get_computed_value(). It was not updated after the first ALTER TABLE call, because it's INPLACE logic considered there's nothing to update, and exited immediately from ha_innobase::inplace_alter_table(). However, vcol metadata needs an update, since vcols structure is changed in mysql record. The regression was introduced by 12614af1fe. There, refcount==1 condition was removed, which turned out to be crucial, though racy. The idea was to update vc_templ after each (sequencing) ALTER TABLE. We should do the same another way, and there may be a plenty of solutions, but the simplest one is to add a following condition: if vcol structure is changed, drop vc_templ; it will be recreated on next ha_innobase::open() call. in prepare_inplace_alter_table. It is safe, since innodb inplace changes require at least HA_ALTER_INPLACE_SHARED_LOCK_AFTER_PREPARE, which guarantee MDL_EXCLUSIVE on this stage. alter_templ_needs_rebuild() also has to track the columns not indexed, to keep vc_templ correct. Note that vc_templ is always kept constructed and available after ha_innobase::open() call, even on INSERT, though no virtual columns are evaluated during that statement inside innodb. In the test case suplied, it will be recreated on the second ALTER TABLE.
-rw-r--r--mysql-test/suite/gcol/r/innodb_virtual_index.result14
-rw-r--r--mysql-test/suite/gcol/t/innodb_virtual_index.test20
-rw-r--r--storage/innobase/handler/handler0alter.cc65
3 files changed, 63 insertions, 36 deletions
diff --git a/mysql-test/suite/gcol/r/innodb_virtual_index.result b/mysql-test/suite/gcol/r/innodb_virtual_index.result
index 2e9b762500d..aafe65b7fa3 100644
--- a/mysql-test/suite/gcol/r/innodb_virtual_index.result
+++ b/mysql-test/suite/gcol/r/innodb_virtual_index.result
@@ -296,3 +296,17 @@ Table Op Msg_type Msg_text
test.t1 optimize note Table does not support optimize, doing recreate + analyze instead
test.t1 optimize status OK
DROP TABLE t1;
+#
+# MDEV-20154 Assertion `len <= col->len || ((col->mtype) == 5
+# || (col->mtype) == 14)' failed in row_merge_buf_add
+#
+CREATE TABLE t1 (
+a VARCHAR(2500),
+b VARCHAR(2499) AS (a) VIRTUAL
+) ENGINE=InnoDB;
+INSERT INTO t1 (a) VALUES ('foo');
+ALTER TABLE t1 MODIFY a VARCHAR(2600), ALGORITHM=INPLACE;
+ALTER TABLE t1 ADD KEY (b), ALGORITHM=INPLACE;
+# Cleanup
+DROP TABLE t1;
+# End of 10.2 tests
diff --git a/mysql-test/suite/gcol/t/innodb_virtual_index.test b/mysql-test/suite/gcol/t/innodb_virtual_index.test
index bb47379f3b2..4a41623264e 100644
--- a/mysql-test/suite/gcol/t/innodb_virtual_index.test
+++ b/mysql-test/suite/gcol/t/innodb_virtual_index.test
@@ -321,3 +321,23 @@ CREATE TABLE t1 (id INT PRIMARY KEY, a CHAR(3),
INSERT INTO t1 (id,a) VALUES (1,'foo');
OPTIMIZE TABLE t1;
DROP TABLE t1;
+
+--echo #
+--echo # MDEV-20154 Assertion `len <= col->len || ((col->mtype) == 5
+--echo # || (col->mtype) == 14)' failed in row_merge_buf_add
+--echo #
+
+CREATE TABLE t1 (
+ a VARCHAR(2500),
+ b VARCHAR(2499) AS (a) VIRTUAL
+) ENGINE=InnoDB;
+INSERT INTO t1 (a) VALUES ('foo');
+
+ALTER TABLE t1 MODIFY a VARCHAR(2600), ALGORITHM=INPLACE;
+ALTER TABLE t1 ADD KEY (b), ALGORITHM=INPLACE;
+
+--echo # Cleanup
+DROP TABLE t1;
+
+--echo # End of 10.2 tests
+
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index 044b760bf04..aad1d93a155 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -7482,6 +7482,10 @@ alter_fill_stored_column(
}
}
+static bool alter_templ_needs_rebuild(const TABLE* altered_table,
+ const Alter_inplace_info* ha_alter_info,
+ const dict_table_t* table);
+
/** Allows InnoDB to update internal structures with concurrent
writes blocked (provided that check_if_supported_inplace_alter()
@@ -8096,9 +8100,9 @@ err_exit:
m_user_thd);
}
+ ha_innobase_inplace_ctx *ctx = NULL;
if (heap) {
- ha_alter_info->handler_ctx
- = new ha_innobase_inplace_ctx(
+ ctx = new ha_innobase_inplace_ctx(
m_prebuilt,
drop_index, n_drop_index,
drop_fk, n_drop_fk,
@@ -8110,6 +8114,7 @@ err_exit:
|| !thd_is_strict_mode(m_user_thd)),
alt_opt.page_compressed,
alt_opt.page_compression_level);
+ ha_alter_info->handler_ctx = ctx;
}
if ((ha_alter_info->handler_flags
@@ -8125,6 +8130,24 @@ err_exit:
DBUG_RETURN(true);
}
+ if (!(ha_alter_info->handler_flags & INNOBASE_ALTER_DATA)
+ && alter_templ_needs_rebuild(altered_table, ha_alter_info,
+ ctx->new_table)
+ && ctx->new_table->n_v_cols > 0) {
+ /* Changing maria record structure may end up here only
+ if virtual columns were altered. In this case, however,
+ vc_templ should be rebuilt. Since we don't actually
+ change any stored data, we can just dispose vc_templ;
+ it will be recreated on next ha_innobase::open(). */
+
+ DBUG_ASSERT(ctx->new_table == ctx->old_table);
+
+ dict_free_vc_templ(ctx->new_table->vc_templ);
+ UT_DELETE(ctx->new_table->vc_templ);
+
+ ctx->new_table->vc_templ = NULL;
+ }
+
success:
/* Memorize the future transaction ID for committing
the data dictionary change, to be reported by
@@ -8249,35 +8272,6 @@ found_col:
DBUG_RETURN(true);
}
-/** Check that the column is part of a virtual index(index contains
-virtual column) in the table
-@param[in] table Table containing column
-@param[in] col column to be checked
-@return true if this column is indexed with other virtual columns */
-static
-bool
-dict_col_in_v_indexes(
- dict_table_t* table,
- dict_col_t* col)
-{
- for (dict_index_t* index = dict_table_get_next_index(
- dict_table_get_first_index(table)); index != NULL;
- index = dict_table_get_next_index(index)) {
- if (!dict_index_has_virtual(index)) {
- continue;
- }
- for (ulint k = 0; k < index->n_fields; k++) {
- dict_field_t* field
- = dict_index_get_nth_field(index, k);
- if (field->col->ind == col->ind) {
- return(true);
- }
- }
- }
-
- return(false);
-}
-
/* Check whether a columnn length change alter operation requires
to rebuild the template.
@param[in] altered_table TABLE object for new version of table.
@@ -8289,9 +8283,9 @@ to rebuild the template.
static
bool
alter_templ_needs_rebuild(
- TABLE* altered_table,
- Alter_inplace_info* ha_alter_info,
- dict_table_t* table)
+ const TABLE* altered_table,
+ const Alter_inplace_info* ha_alter_info,
+ const dict_table_t* table)
{
ulint i = 0;
@@ -8301,8 +8295,7 @@ alter_templ_needs_rebuild(
for (ulint j=0; j < table->n_cols; j++) {
dict_col_t* cols
= dict_table_get_nth_col(table, j);
- if (cf.length > cols->len
- && dict_col_in_v_indexes(table, cols)) {
+ if (cf.length > cols->len) {
return(true);
}
}