Thiru, Marko,
Here is the patch for MDEV-39081 that you requested to review.
I did some investigation inside InnoDB, and I think I understand what is
going on here, though I should say I don't have the full overview of what
happens around virtual columns inside InnoDB (but then, probably no-one
does).
This is a minimally intrusive patch that fixes the problems seen in the test
cases, and which has no performance impact for tables without virtual
columns. So I think this is a suitable fix for 11.4.
I see this in trx_undo_page_report_modify():
/*----------------------------------------*/
/* In the case of a delete marking, and also in the case of an update
where any ordering field of any index changes, store the values of all
columns which occur as ordering fields in any index. This info is used
...
for (col_no = 0; col_no < dict_table_get_n_v_cols(table);
col_no++) {
const dict_v_col_t* col
= dict_table_get_nth_v_col(table, col_no);
if (col->m_col.ord_part) {
IIUC, this code uses the value of all virtual columns that are part of an
index, including columns that are not used by the query in any way. This
requires that InnoDB requests for the server to compute those values,
otherwise they will not be available.
In the original MDEV-39081 testcase, the table has a unique index on a BLOB:
CREATE TABLE t (pk INT, a INT, b INT, c BLOB, KEY (a), PRIMARY KEY (pk),
UNIQUE (b,c))
The UNIQUE (b,c) makes the server create an extra virtual column
DB_ROW_HASH_1 that is computed from b and c.
Then in a query UPDATE t SET a=2, the read_set provided by the server is set
to 0x03, meaning that pk and a are in the read set, but _not_ b, c, or
DB_ROW_HASH_1.
It would be wrong for the server to include the extra columns, as that would
cause expensive reading of the blob and re-computing the hash also in
engines that have no need to do so. Therefore, InnoDB must request these
extra columns when needed.
There is already code in ha_innobase::column_bitmaps_signal() that handles a
similar case (inplace ALTER TABLE according to comments), where indexed
virtual columns are added to the read_set. Thus, it is natural to just add
the extra condition there, of any indexed column being in the write_set to
likewise trigger inclusion of these additional columns.
If you want, you are welcome to suggest a different fix inside InnoDB. But
the overall approach should be similar, IMO, that server puts in read_set
only columns directly related to the query, and InnoDB must be the one to
request additional columns, if/when it so requires.
- Kristian.
Kristian Nielsen (1):
MDEV-39081: InnoDB: tried to purge non-delete-marked record, assertion
fails
mysql-test/suite/innodb/r/vcol_index.result | 52 +++++++++++++++++
mysql-test/suite/innodb/t/vcol_index.test | 63 +++++++++++++++++++++
storage/innobase/handler/ha_innodb.cc | 37 ++++++++----
3 files changed, 140 insertions(+), 12 deletions(-)
create mode 100644 mysql-test/suite/innodb/r/vcol_index.result
create mode 100644 mysql-test/suite/innodb/t/vcol_index.test
--
2.47.3
_______________________________________________
commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
