Skip to content

Commit

Permalink
MDEV-20048 Assertion 'n < tuple->n_fields on ROLLBACK after DROP COLUMN
Browse files Browse the repository at this point in the history 
btr_push_update_extern_fields(): Add a parameter for the original number
of fields in the record before btr_cur_trim(). Assume that this function
will only be called for the clustered index, which is the only index
that can contain off-page columns.

trx_undo_prev_version_build(), btr_cur_pessimistic_update():
Only invoke btr_push_update_extern_fields() for the clustered index.
  • Loading branch information
@dr-m
dr-m committed Jul 19, 2019
1 parent 90600e8 commit 09e9f88
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 25 deletions.
32 changes: 32 additions & 0 deletions mysql-test/suite/innodb/r/instant_alter_bugs.result
View file
Expand Up @@ -203,3 +203,35 @@ ALTER TABLE t1 MODIFY c TEXT NULL, ALGORITHM=INSTANT;
ERROR 0A000: ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=INPLACE ERROR 0A000: ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=INPLACE
ALTER TABLE t1 MODIFY c TEXT NULL; ALTER TABLE t1 MODIFY c TEXT NULL;
DROP TABLE t1; DROP TABLE t1;
#
# MDEV-20048 dtuple_get_nth_field(): Assertion 'n < tuple->n_fields'
# failed on ROLLBACK after instant DROP COLUMN
#
CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1);
ALTER TABLE t1 ADD b TEXT, ALGORITHM=INSTANT;
SET @b = REPEAT('b', @@innodb_page_size / 2 + 1);
INSERT INTO t1 VALUES(2, @b), (3, @b);
BEGIN;
DELETE FROM t1 WHERE a=2;
connect purge_control,localhost,root;
START TRANSACTION WITH CONSISTENT SNAPSHOT;
connection default;
COMMIT;
ALTER TABLE t1 DROP b, ALGORITHM=INSTANT;
BEGIN;
INSERT INTO t1 VALUES (2);
connection purge_control;
SELECT * FROM t1;
a
1
2
3
disconnect purge_control;
connection default;
ROLLBACK;
SELECT * FROM t1;
a
1
3
DROP TABLE t1;
32 changes: 32 additions & 0 deletions mysql-test/suite/innodb/t/instant_alter_bugs.test
View file
Expand Up @@ -216,3 +216,35 @@ ALTER TABLE t1 DROP INDEX ftidx;
ALTER TABLE t1 MODIFY c TEXT NULL, ALGORITHM=INSTANT; ALTER TABLE t1 MODIFY c TEXT NULL, ALGORITHM=INSTANT;
ALTER TABLE t1 MODIFY c TEXT NULL; ALTER TABLE t1 MODIFY c TEXT NULL;
DROP TABLE t1; DROP TABLE t1;

--echo #
--echo # MDEV-20048 dtuple_get_nth_field(): Assertion 'n < tuple->n_fields'
--echo # failed on ROLLBACK after instant DROP COLUMN
--echo #
CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1);
ALTER TABLE t1 ADD b TEXT, ALGORITHM=INSTANT;
SET @b = REPEAT('b', @@innodb_page_size / 2 + 1);
INSERT INTO t1 VALUES(2, @b), (3, @b);
BEGIN;
DELETE FROM t1 WHERE a=2;

# Stop purge so that it doesn't remove the delete-marked entry.
connect (purge_control,localhost,root);
START TRANSACTION WITH CONSISTENT SNAPSHOT;
connection default;

COMMIT;

ALTER TABLE t1 DROP b, ALGORITHM=INSTANT;
BEGIN;
INSERT INTO t1 VALUES (2);

connection purge_control;
SELECT * FROM t1;
disconnect purge_control;
connection default;
ROLLBACK;

SELECT * FROM t1;
DROP TABLE t1;
42 changes: 25 additions & 17 deletions storage/innobase/btr/btr0cur.cc
View file
Expand Up @@ -4904,14 +4904,18 @@ btr_cur_pessimistic_update(
itself. Thus the following call is safe. */ itself. Thus the following call is safe. */
row_upd_index_replace_new_col_vals_index_pos(new_entry, index, update, row_upd_index_replace_new_col_vals_index_pos(new_entry, index, update,
entry_heap); entry_heap);
const ulint n = new_entry->n_fields;
btr_cur_trim(new_entry, index, update, thr); btr_cur_trim(new_entry, index, update, thr);


/* We have to set appropriate extern storage bits in the new /* We have to set appropriate extern storage bits in the new
record to be inserted: we have to remember which fields were such */ record to be inserted: we have to remember which fields were such */


ut_ad(!page_is_comp(page) || !rec_get_node_ptr_flag(rec)); ut_ad(!page_is_comp(page) || !rec_get_node_ptr_flag(rec));
ut_ad(rec_offs_validate(rec, index, *offsets)); ut_ad(rec_offs_validate(rec, index, *offsets));
n_ext += btr_push_update_extern_fields(new_entry, update, entry_heap); if (index->is_primary()) {
n_ext += btr_push_update_extern_fields(
new_entry, n, update, entry_heap);
}


if ((flags & BTR_NO_UNDO_LOG_FLAG) if ((flags & BTR_NO_UNDO_LOG_FLAG)
&& rec_offs_any_extern(*offsets)) { && rec_offs_any_extern(*offsets)) {
Expand Down Expand Up @@ -7246,29 +7250,33 @@ btr_cur_unmark_extern_fields(
} }
} }


/*******************************************************************//** /** Flag the data tuple fields that are marked as extern storage in the
Flags the data tuple fields that are marked as extern storage in the
update vector. We use this function to remember which fields we must update vector. We use this function to remember which fields we must
mark as extern storage in a record inserted for an update. mark as extern storage in a record inserted for an update.
@param[in,out] tuple clustered index record
@param[in] n number of fields in tuple, before any btr_cur_trim()
@param[in] update update vector
@param[in,out] heap memory heap
@return number of flagged external columns */ @return number of flagged external columns */
ulint ulint
btr_push_update_extern_fields( btr_push_update_extern_fields(dtuple_t* tuple, ulint n, const upd_t* update,
/*==========================*/ mem_heap_t* heap)
dtuple_t* tuple, /*!< in/out: data tuple */
const upd_t* update, /*!< in: update vector */
mem_heap_t* heap) /*!< in: memory heap */
{ {
ulint n_pushed = 0; ulint n_pushed = 0;
ulint n; const upd_field_t* uf = update->fields;
const upd_field_t* uf;
ut_ad(n >= tuple->n_fields);
/* The clustered index record must always contain a
PRIMARY KEY and the system columns DB_TRX_ID,DB_ROLL_PTR. */
ut_ad(tuple->n_fields > DATA_ROLL_PTR);
compile_time_assert(DATA_ROLL_PTR == 2);


uf = update->fields; for (ulint un = upd_get_n_fields(update); un--; uf++) {
n = upd_get_n_fields(update); ut_ad(uf->field_no < n);


for (; n--; uf++) { if (dfield_is_ext(&uf->new_val)
if (dfield_is_ext(&uf->new_val)) { && uf->field_no < tuple->n_fields) {
dfield_t* field dfield_t* field = &tuple->fields[uf->field_no];
= dtuple_get_nth_field(tuple, uf->field_no);


if (!dfield_is_ext(field)) { if (!dfield_is_ext(field)) {
dfield_set_ext(field); dfield_set_ext(field);
Expand Down
14 changes: 7 additions & 7 deletions storage/innobase/include/btr0cur.h
View file
Expand Up @@ -784,17 +784,17 @@ btr_rec_copy_externally_stored_field(
ulint* len, ulint* len,
mem_heap_t* heap); mem_heap_t* heap);


/*******************************************************************//** /** Flag the data tuple fields that are marked as extern storage in the
Flags the data tuple fields that are marked as extern storage in the
update vector. We use this function to remember which fields we must update vector. We use this function to remember which fields we must
mark as extern storage in a record inserted for an update. mark as extern storage in a record inserted for an update.
@param[in,out] tuple clustered index record
@param[in] n number of fields in tuple, before any btr_cur_trim()
@param[in] update update vector
@param[in,out] heap memory heap
@return number of flagged external columns */ @return number of flagged external columns */
ulint ulint
btr_push_update_extern_fields( btr_push_update_extern_fields(dtuple_t* tuple, ulint n, const upd_t* update,
/*==========================*/ mem_heap_t* heap)
dtuple_t* tuple, /*!< in/out: data tuple */
const upd_t* update, /*!< in: update vector */
mem_heap_t* heap) /*!< in: memory heap */
MY_ATTRIBUTE((nonnull)); MY_ATTRIBUTE((nonnull));
/***********************************************************//** /***********************************************************//**
Sets a secondary index record's delete mark to the given value. This Sets a secondary index record's delete mark to the given value. This
Expand Down
5 changes: 4 additions & 1 deletion storage/innobase/trx/trx0rec.cc
View file
Expand Up @@ -2473,7 +2473,10 @@ trx_undo_prev_version_build(


entry = row_rec_to_index_entry( entry = row_rec_to_index_entry(
rec, index, offsets, &n_ext, heap); rec, index, offsets, &n_ext, heap);
n_ext += btr_push_update_extern_fields(entry, update, heap); if (index->is_primary()) {
n_ext += btr_push_update_extern_fields(
entry, entry->n_fields, update, heap);
}
/* The page containing the clustered index record /* The page containing the clustered index record
corresponding to entry is latched in mtr. Thus the corresponding to entry is latched in mtr. Thus the
following call is safe. */ following call is safe. */
Expand Down

0 comments on commit 09e9f88

Please sign in to comment.