Skip to content

Commit

Permalink
MDEV-31154 Fatal InnoDB error or assertion `!is_v' failure upon multi…
Browse files Browse the repository at this point in the history 
…-update with indexed virtual column

MDEV-33558 Fatal error InnoDB: Clustered record field for column x not found

This is issue is about row ID filtering used with index on virtual
column(s). We hit debug assert and crash while building the record
template in Innodb. The primary reason is that we try to force the code
path to use the ICP path. With ICP, we don't support index with virtual
column and we validate it while index condition is pushed.

Simplify the code for building template to handle both ICP and Row ID
filtering by skipping virtual columns.
  • Loading branch information
@mariadb-DebarunBanerjee
mariadb-DebarunBanerjee committed Mar 15, 2024
1 parent f5df448 commit d912a63
Show file tree
Hide file tree
Showing 3 changed files with 228 additions and 29 deletions.
99 changes: 99 additions & 0 deletions mysql-test/main/rowid_filter_innodb.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3769,4 +3769,103 @@ Warnings:
Note 1276 Field or reference 'test.t1.pk' of SELECT #2 was resolved in SELECT #1
Note 1003 /* select#1 */ select `test`.`t1`.`pk` AS `pk`,`test`.`t1`.`c1` AS `c1` from `test`.`t1` where !<expr_cache><`test`.`t1`.`c1`,`test`.`t1`.`pk`>(<in_optimizer>(`test`.`t1`.`c1`,<exists>(/* select#2 */ select `test`.`t2`.`c1` from `test`.`t2` join `test`.`t1` `a1` where `test`.`t2`.`i1` = `test`.`t1`.`pk` and `test`.`t2`.`i1` between 3 and 5 and trigcond(<cache>(`test`.`t1`.`c1`) = `test`.`t2`.`c1`))))
DROP TABLE t1,t2;
#
# MDEV-31154: Fatal InnoDB error or assertion `!is_v' failure upon multi-update with indexed virtual column
#
# Test with auto generated Primary Key
#
SET @save_optimizer_switch= @@optimizer_switch;
SET optimizer_switch='rowid_filter=on';
CREATE TABLE t0(a int);
INSERT INTO t0 SELECT seq FROM seq_1_to_20;
ANALYZE TABLE t0 PERSISTENT FOR ALL;
Table Op Msg_type Msg_text
test.t0 analyze status Engine-independent statistics collected
test.t0 analyze status OK
CREATE TABLE t1 (
a int,
b int as (a * 2) VIRTUAL,
f char(200), /* Filler */
key (b),
key (a)
) engine=innodb;
INSERT INTO t1 (a, f) SELECT seq, seq FROM seq_1_to_1000;
ANALYZE TABLE t1 PERSISTENT FOR ALL;
Table Op Msg_type Msg_text
test.t1 analyze status Engine-independent statistics collected
test.t1 analyze status OK
# Test for type 'ref|filter'
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t0 ALL NULL NULL NULL NULL 20 Using where
1 SIMPLE t1 ref|filter b,a b|a 5|5 test.t0.a 1 (2%) Using where; Using rowid filter
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
count(*)
10
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t0 ALL NULL NULL NULL NULL 20 Using where
1 SIMPLE t1 ref|filter b,a b|a 5|5 test.t0.a 1 (2%) Using where; Using rowid filter
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
count(*)
10
# Test for type 'range|filter'
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range|filter b,a b|a 5|5 NULL 49 (10%) Using where; Using rowid filter
SELECT count(*) FROM t1 WHERE a<100 and b <100;
count(*)
49
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range|filter b,a b|a 5|5 NULL 49 (10%) Using where; Using rowid filter
SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
count(*)
49
# Test with Primary Key
#
DROP TABLE t1;
CREATE TABLE t1 (
p int PRIMARY KEY AUTO_INCREMENT,
a int,
b int as (a * 2) VIRTUAL,
f char(200), /* Filler */
key (b),
key (a)
) engine=innodb;
INSERT INTO t1 (a, f) SELECT seq, seq FROM seq_1_to_1000;
ANALYZE TABLE t1 PERSISTENT FOR ALL;
Table Op Msg_type Msg_text
test.t1 analyze status Engine-independent statistics collected
test.t1 analyze status OK
# Test for type 'ref|filter'
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t0 ALL NULL NULL NULL NULL 20 Using where
1 SIMPLE t1 ref|filter b,a b|a 5|5 test.t0.a 1 (2%) Using where; Using rowid filter
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
count(*)
10
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t0 ALL NULL NULL NULL NULL 20 Using where
1 SIMPLE t1 ref|filter b,a b|a 5|5 test.t0.a 1 (2%) Using where; Using rowid filter
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
count(*)
10
# Test for type 'range|filter'
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range|filter b,a b|a 5|5 NULL 49 (10%) Using where; Using rowid filter
SELECT count(*) FROM t1 WHERE a<100 and b <100;
count(*)
49
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range|filter b,a b|a 5|5 NULL 49 (10%) Using where; Using rowid filter
SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
count(*)
49
SET optimizer_switch=@save_optimizer_switch;
DROP TABLE t0, t1;
# End of 10.4 tests
75 changes: 75 additions & 0 deletions mysql-test/main/rowid_filter_innodb.test
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
--source include/no_valgrind_without_big.inc
--source include/have_innodb.inc
--source include/have_debug.inc
--source include/have_sequence.inc
--source include/innodb_stable_estimates.inc

SET SESSION STORAGE_ENGINE='InnoDB';

Expand Down Expand Up @@ -677,4 +679,77 @@ eval EXPLAIN EXTENDED $q;

DROP TABLE t1,t2;

--echo #
--echo # MDEV-31154: Fatal InnoDB error or assertion `!is_v' failure upon multi-update with indexed virtual column
--echo #

--echo # Test with auto generated Primary Key
--echo #

SET @save_optimizer_switch= @@optimizer_switch;
SET optimizer_switch='rowid_filter=on';

CREATE TABLE t0(a int);
INSERT INTO t0 SELECT seq FROM seq_1_to_20;
ANALYZE TABLE t0 PERSISTENT FOR ALL;

CREATE TABLE t1 (
a int,
b int as (a * 2) VIRTUAL,
f char(200), /* Filler */
key (b),
key (a)
) engine=innodb;

INSERT INTO t1 (a, f) SELECT seq, seq FROM seq_1_to_1000;
ANALYZE TABLE t1 PERSISTENT FOR ALL;

--echo # Test for type 'ref|filter'
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;

EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;

--echo # Test for type 'range|filter'
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100;
SELECT count(*) FROM t1 WHERE a<100 and b <100;

EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;

--echo # Test with Primary Key
--echo #

DROP TABLE t1;
CREATE TABLE t1 (
p int PRIMARY KEY AUTO_INCREMENT,
a int,
b int as (a * 2) VIRTUAL,
f char(200), /* Filler */
key (b),
key (a)
) engine=innodb;

INSERT INTO t1 (a, f) SELECT seq, seq FROM seq_1_to_1000;
ANALYZE TABLE t1 PERSISTENT FOR ALL;

--echo # Test for type 'ref|filter'
EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20;

EXPLAIN SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;
SELECT count(*) from t0,t1 WHERE t0.a=t1.b AND t1.a<20 FOR UPDATE;

--echo # Test for type 'range|filter'
EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100;
SELECT count(*) FROM t1 WHERE a<100 and b <100;

EXPLAIN SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;
SELECT count(*) FROM t1 WHERE a<100 and b <100 FOR UPDATE;

SET optimizer_switch=@save_optimizer_switch;

DROP TABLE t0, t1;

--echo # End of 10.4 tests
83 changes: 54 additions & 29 deletions storage/innobase/handler/ha_innodb.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7730,26 +7730,55 @@ ha_innobase::build_template(

ulint num_v = 0;

if (active_index != MAX_KEY
&& active_index == pushed_idx_cond_keyno) {
m_prebuilt->idx_cond = this;
goto icp;
} else if (pushed_rowid_filter && rowid_filter_is_active) {
icp:
/* Push down an index condition or an end_range check. */
/* MDEV-31154: For pushed down index condition we don't support virtual
column and idx_cond_push() does check for it. For row ID filtering we
don't need such restrictions but we get into trouble trying to use the
ICP path.

1. It should be fine to follow no_icp path if primary key is generated.
However, with user specified primary key(PK), the row is identified by
the PK and those columns need to be converted to mysql format in
row_search_idx_cond_check before doing the comparison. Since secondary
indexes always have PK appended in innodb, it works with current ICP
handling code when fetch_primary_key_cols is set to TRUE.

2. Although ICP comparison and Row ID comparison works on different
columns the current ICP code can be shared by both.

3. In most cases, it works today by jumping to goto no_icp when we
encounter a virtual column. This is hackish and already have some
issues as it cannot handle PK and all states are not reset properly,
for example, idx_cond_n_cols is not reset.

4. We already encountered MDEV-28747 m_prebuilt->idx_cond was being set.

Neither ICP nor row ID comparison needs virtual columns and the code is
simplified to handle both. It should handle the issues. */

const bool pushed_down = active_index != MAX_KEY
&& active_index == pushed_idx_cond_keyno;

m_prebuilt->idx_cond = pushed_down ? this : nullptr;

if (m_prebuilt->idx_cond || m_prebuilt->pk_filter) {
/* Push down an index condition, end_range check or row ID
filter */
for (ulint i = 0; i < n_fields; i++) {
const Field* field = table->field[i];
const bool is_v = !field->stored_in_db();
if (is_v && skip_virtual) {
num_v++;
continue;
}

bool index_contains = index->contains_col_or_prefix(
is_v ? num_v : i - num_v, is_v);
if (is_v && index_contains) {
m_prebuilt->n_template = 0;
num_v = 0;
goto no_icp;

if (is_v) {
if (index_contains) {
/* We want to ensure that ICP is not
used with virtual columns. */
ut_ad(!pushed_down);
m_prebuilt->idx_cond = nullptr;
}
num_v++;
continue;
}

/* Test if an end_range or an index condition
Expand All @@ -7769,7 +7798,7 @@ ha_innobase::build_template(
which would be acceptable if end_range==NULL. */
if (build_template_needs_field_in_icp(
index, m_prebuilt, index_contains,
is_v ? num_v : i - num_v, is_v)) {
i - num_v, false)) {
if (!whole_row) {
field = build_template_needs_field(
index_contains,
Expand All @@ -7778,15 +7807,10 @@ ha_innobase::build_template(
fetch_primary_key_cols,
index, table, i, num_v);
if (!field) {
if (is_v) {
num_v++;
}
continue;
}
}

ut_ad(!is_v);

mysql_row_templ_t* templ= build_template_field(
m_prebuilt, clust_index, index,
table, field, i - num_v, 0);
Expand Down Expand Up @@ -7863,15 +7887,16 @@ ha_innobase::build_template(
*/
}

if (is_v) {
num_v++;
}
}

ut_ad(m_prebuilt->idx_cond_n_cols > 0);
ut_ad(m_prebuilt->idx_cond_n_cols == m_prebuilt->n_template);

num_v = 0;
ut_ad(m_prebuilt->idx_cond_n_cols == m_prebuilt->n_template);
if (m_prebuilt->idx_cond_n_cols == 0) {
/* No columns to push down. It is safe to jump to np ICP
path. */
m_prebuilt->idx_cond = nullptr;
goto no_icp;
}

/* Include the fields that are not needed in index condition
pushdown. */
Expand All @@ -7886,7 +7911,7 @@ ha_innobase::build_template(
bool index_contains = index->contains_col_or_prefix(
is_v ? num_v : i - num_v, is_v);

if (!build_template_needs_field_in_icp(
if (is_v || !build_template_needs_field_in_icp(
index, m_prebuilt, index_contains,
is_v ? num_v : i - num_v, is_v)) {
/* Not needed in ICP */
Expand Down Expand Up @@ -7919,7 +7944,7 @@ ha_innobase::build_template(
} else {
no_icp:
/* No index condition pushdown */
m_prebuilt->idx_cond = NULL;
ut_ad(!m_prebuilt->idx_cond);
ut_ad(num_v == 0);

for (ulint i = 0; i < n_fields; i++) {
Expand Down

0 comments on commit d912a63

Please sign in to comment.