[MDEV-23632] ALTER TABLE...ADD KEY creates corrupted index on virtual column Created: 2020-08-31  Updated: 2021-04-25  Resolved: 2021-01-05

Status: Closed
Project: MariaDB Server
Component/s: Virtual Columns
Affects Version/s: 10.3, 10.4, 10.5
Fix Version/s: 10.3.28, 10.4.18, 10.5.9

Type: Bug Priority: Critical
Reporter: Marko Mäkelä Assignee: Nikita Malyavin
Resolution: Fixed Votes: 0
Labels: MSAN, affects-tests, corruption, not-10.2, regression, valgrind

Issue Links:
Relates
relates to MDEV-24564 Statistics are lost after ALTER TABLE Closed
relates to MDEV-25509 Atomic DDL: Assertion `err != DB_DUPL... Closed
relates to MDEV-5800 indexes on virtual (not materialized)... Closed
relates to MDEV-11369 Instant add column for InnoDB Closed
relates to MDEV-22924 Warning InnoDB: Index 'Marvão_idx3' c... Closed

 Description   

Based on a trace that I analyzed in MDEV-22924, I created the following test case:

--source include/have_innodb.inc
 
CREATE TABLE t1(a INT PRIMARY KEY, b INT, g INT GENERATED ALWAYS AS(b)VIRTUAL)
 
ENGINE=InnoDB;
 
INSERT INTO t1 VALUES (1,1,default);
 
ALTER TABLE t1 ADD COLUMN c INT;
 
ALTER TABLE t1 ADD KEY(g);
 
CHECK TABLE t1;
 
SELECT g FROM t1 FORCE INDEX (g);
 
DROP TABLE t1;

This test will fail in 10.3 Valgrind as well as 10.5 MSAN:

10.3 6a042281bdbfe91cc39e1f6e02295bfe7eaa9d43

 
==469741== Uninitialised byte(s) found during client check request
 
==469741==    at 0xD23BEE: dfield_dup (data0data.ic:172)
 
==469741==    by 0xD23BEE: row_merge_buf_add(row_merge_buf_t*, dict_index_t*, dict_table_t const*, dict_table_t const*, fts_psort_t*, dtuple_t*, row_ext_t const*, unsigned long*, mem_block_info_t*, dberr_t*, mem_block_info_t**, TABLE*, trx_t*) (row0merge.cc:841)
 
==469741==    by 0xD26DC2: row_merge_read_clustered_index(trx_t*, TABLE*, dict_table_t const*, dict_table_t*, bool, dict_index_t**, dict_index_t*, fts_psort_t*, merge_file_t*, unsigned long const*, unsigned long, dtuple_t const*, dict_add_v_col_t const*, unsigned long const*, unsigned long, ib_sequence_t&, unsigned char*, bool, pfs_os_file_t*, ut_stage_alter_t*, double, unsigned char*, TABLE*, bool) (row0merge.cc:2358)

10.5 31e6c96b0449761dc15f548c28ded671d1b7219b

 
Uninitialized bytes in __msan_check_mem_is_initialized at offset 0 inside [0x7290000b90f0, 4)
 
==457772==WARNING: MemorySanitizer: use-of-uninitialized-value
 
    #0 0x55a19150c8f0 in dfield_dup(dfield_t*, mem_block_info_t*) /mariadb/10.5m/storage/innobase/include/data0data.ic:169:3
 
    #1 0x55a19150c8f0 in row_merge_buf_add(row_merge_buf_t*, dict_index_t*, dict_table_t const*, dict_table_t const*, fts_psort_t*, dtuple_t*, row_ext_t const*, unsigned long*, mem_block_info_t*, dberr_t*, mem_block_info_t**, TABLE*, trx_t*) /mariadb/10.5m/storage/innobase/row/row0merge.cc:835:3

If the ADD COLUMN is removed or ,FORCE is added to the statement, the test will not fail. Instant ADD COLUMN was introduced in MDEV-11369.

 Comments   
Comment by Elena Stepanova [ 2020-08-31 ]

Looks like it could be related to MDEV-20154

Comment by Marko Mäkelä [ 2020-09-03 ]

The test case in the Description triggers the following errors after I merged the fixes of MDEV-18366 and MDEV-20618 to 10.3:

10.3 c3752cef3c34589ea95a7ab66cff3dd5b11290d5

 
==1318149== Thread 27:
 
==1318149== Uninitialised byte(s) found during client check request
 
==1318149==    at 0xD23F45: dfield_dup (data0data.ic:172)
 
==1318149==    by 0xD23F45: row_merge_buf_add(row_merge_buf_t*, dict_index_t*, dict_table_t const*, dict_table_t const*, fts_psort_t*, dtuple_t*, row_ext_t const*, unsigned long*, mem_block_info_t*, dberr_t*, mem_block_info_t**, TABLE*, trx_t*) (row0merge.cc:837)
 
==1318149==    by 0xD27171: row_merge_read_clustered_index(trx_t*, TABLE*, dict_table_t const*, dict_table_t*, bool, dict_index_t**, dict_index_t*, fts_psort_t*, merge_file_t*, unsigned long const*, unsigned long, dtuple_t const*, dict_add_v_col_t const*, unsigned long const*, unsigned long, ib_sequence_t&, unsigned char*, bool, pfs_os_file_t*, ut_stage_alter_t*, double, unsigned char*, TABLE*, bool) (row0merge.cc:2354)
 
==1318149==    by 0xD29231: row_merge_build_indexes(trx_t*, dict_table_t*, dict_table_t*, bool, dict_index_t**, unsigned long const*, unsigned long, TABLE*, dtuple_t const*, unsigned long const*, unsigned long, ib_sequence_t&, bool, ut_stage_alter_t*, dict_add_v_col_t const*, TABLE*, bool) (row0merge.cc:4709)
 
==1318149==    by 0xBF2F32: ha_innobase::inplace_alter_table(TABLE*, Alter_inplace_info*) (handler0alter.cc:7148)
 
…

Note: This is with GCC and WITH_VALGRIND=ON. If I compile with Clang, there will be many bogus warnings. You may have to avoid optimizations as well. In GCC 10.2.0, -Og does not seem to trigger too many bogus Valgrind warnings.

Comment by Nikita Malyavin [ 2020-12-21 ]

Seems that mysql_col_offset is not updated (correctly) after new column added:

(gdb) l
 
21151			dfield_set_null(field);
 
21152			field->type.prtype |= DATA_VIRTUAL;
 
21153			DBUG_RETURN(field);
 
21154		}
 
21155	
21156		row_mysql_store_col_in_innobase_format(
 
21157			field, buf,
 
21158			TRUE, mysql_rec + vctempl->mysql_col_offset,
 
21159			vctempl->mysql_col_len, dict_table_is_comp(index->table));
 
21160		field->type.prtype |= DATA_VIRTUAL;
 
(gdb) p vctempl->mysql_col_offset
 
$5 = 9

virtual fields are always added in the end, so the column should be 4th, and mysql_col_offset should be 1 + 4*3 = 13

Comment by Nikita Malyavin [ 2020-12-24 ]

marko Please review the bugfix and two other related commits on bb-10.3-vcol-instant branch

Comment by Marko Mäkelä [ 2021-01-04 ]

Thank you! I provided some feedback on the first commit. It is OK to push after addressing those.

Generated at Thu Feb 08 09:23:52 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.