[MDEV-18732] InnoDB: Wrong results in ALTER IGNORE TABLE with ALGORITHM=INPLACE Created: 2019-02-25  Updated: 2019-03-05  Resolved: 2019-03-05

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.0, 10.2.2, 10.3.0, 10.4.0
Fix Version/s: 10.2.23, 10.1.39, 10.3.14, 10.4.4

Type: Bug Priority: Major
Reporter: Matthias Leich Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: upstream

Attachments: File t_s_3A.test    
Issue Links:
Problem/Incident
is caused by MDEV-18819 ALTER_COLUMN_VCOL is not set for gene... Closed
Relates
relates to MDEV-14168 Unconditionally allow ALGORITHM=INPLA... Closed
relates to MDEV-16365 Setting a column NOT NULL fails to re... Closed

 Description   

Problem found during RQG testing involving row based replication.

SET SESSION sql_mode=NO_ENGINE_SUBSTITUTION;
CREATE TABLE t_innodb_DEFAULT ( col2 INT, col_int INT, col_int_g INT AS (col_int) PERSISTENT ) ENGINE = InnoDB  ;
CREATE TABLE t_innodb_INPLACE ( col2 INT, col_int INT, col_int_g INT AS (col_int) PERSISTENT ) ENGINE = InnoDB  ;
CREATE TABLE t_innodb_COPY    ( col2 INT, col_int INT, col_int_g INT AS (col_int) PERSISTENT ) ENGINE = InnoDB  ;
CREATE TABLE t_myisam_DEFAULT ( col2 INT, col_int INT, col_int_g INT AS (col_int) PERSISTENT ) ENGINE = MyISAM  ;
INSERT INTO t_innodb_DEFAULT (col_int, col2) VALUES (NULL, 1);
INSERT INTO t_innodb_INPLACE (col_int, col2) VALUES (NULL, 1);
INSERT INTO t_innodb_COPY    (col_int, col2) VALUES (NULL, 1);
INSERT INTO t_myisam_DEFAULT (col_int, col2) VALUES (NULL, 1);
ALTER TABLE t_innodb_DEFAULT MODIFY COLUMN col_int INT NOT NULL;
Warnings:
Warning 1265    Data truncated for column 'col_int' at row 1
ALTER TABLE t_innodb_INPLACE MODIFY COLUMN col_int INT NOT NULL, ALGORITHM = INPLACE;
Warnings:
Warning 1265    Data truncated for column 'col_int' at row 1
ALTER TABLE t_innodb_COPY    MODIFY COLUMN col_int INT NOT NULL, ALGORITHM = COPY;
Warnings:
Warning 1265    Data truncated for column 'col_int' at row 1
ALTER TABLE t_myisam_DEFAULT MODIFY COLUMN col_int INT NOT NULL;
Warnings:
Warning 1265    Data truncated for column 'col_int' at row 1
INSERT INTO t_innodb_DEFAULT (col_int, col2) VALUES (NULL, 2);
ERROR 23000: Column 'col_int' cannot be null
INSERT INTO t_innodb_INPLACE (col_int, col2) VALUES (NULL, 2);
ERROR 23000: Column 'col_int' cannot be null
INSERT INTO t_innodb_COPY    (col_int, col2) VALUES (NULL, 2);
ERROR 23000: Column 'col_int' cannot be null
INSERT INTO t_myisam_DEFAULT (col_int, col2) VALUES (NULL, 2);
ERROR 23000: Column 'col_int' cannot be null
---- The behaviour of the different tables above is consistent ----
SELECT * FROM t_innodb_DEFAULT;
col2    col_int col_int_g
1   0   NULL                    <====== Why NULL?
SELECT * FROM t_innodb_INPLACE;
col2    col_int col_int_g
1   0   NULL                   <====== Why NULL?
SELECT * FROM t_innodb_COPY;
col2    col_int col_int_g
1   0   0
SELECT * FROM t_myisam_DEFAULT;
col2    col_int col_int_g
1   0   0
DROP TABLE t_innodb_DEFAULT;
DROP TABLE t_innodb_INPLACE;
DROP TABLE t_innodb_COPY;
DROP TABLE t_myisam_DEFAULT;
 
10.3 09bd2138522787a4e0b015695c462903f4a9e728 2019-02-22
10.4 5b4d6595d26aaaf0bf8bb3c9171cf1da96306a7c 2019-02-21
No replay on 10.2 af6fdc13072cc310cf17fd3b28c749515d9c563c 2019-02-19
        (ALTER ... COLUMN ...INPLACE is not supported and DEFAULT seems to be COPY).



 Comments   
Comment by Matthias Leich [ 2019-02-25 ]

How to replay the problem?
Please

  • copy t_s_3A.test to the directory main
  • run the test like
    ./mysql-test-run.pl --mem --suite=main t_s_3A | tee prt
  • just check the output in "prt"
    and do not become confused by the fact that MTR reports a "pass".
Comment by Marko Mäkelä [ 2019-02-27 ]

Here is a more concise test case that demonstrates the issue on 10.2. I suspect that it could be present on 10.0 and 10.1 as well:

--source include/have_innodb.inc
 
CREATE TABLE t_innodb_INPLACE(c INT) ENGINE = InnoDB;
CREATE TABLE t_innodb_COPY(c INT) ENGINE = InnoDB;
 
INSERT INTO t_innodb_INPLACE VALUES (NULL);
INSERT INTO t_innodb_COPY VALUES (NULL);
 
ALTER IGNORE TABLE t_innodb_COPY    MODIFY COLUMN c INT NOT NULL, ALGORITHM = COPY;
--error ER_INVALID_USE_OF_NULL
ALTER IGNORE TABLE t_innodb_INPLACE MODIFY COLUMN c INT NOT NULL, ALGORITHM = INPLACE;
 
SELECT * FROM t_innodb_INPLACE;
SELECT * FROM t_innodb_COPY;
 
DROP TABLE t_innodb_INPLACE, t_innodb_COPY;

The problem is that ALGORITHM=INPLACE is throwing an error, while ALGORITHM=COPY allows the operation to go through (the NULL is being updated to 0).
Because the attached test case t_s_3A.test was ignoring all errors, the errors were propagated.

Comment by Marko Mäkelä [ 2019-02-27 ]

On MariaDB 10.3, the ALTER IGNORE TABLE will not fail, but it will actually produce the warning. So, we must fix different things in this area.

Starting with MariaDB 10.2, SQL_MODE includes STRICT_TRANS_TABLES by default. In older versions, you would have to set that in order to enable ALGORITHM=INPLACE for introducing a NOT NULL attribute to a column.

Comment by Matthias Leich [ 2019-02-27 ]

When adding a   SET SESSION sql_mode=strict_trans_tables;      at test begin
- 10.0 commit d9d83f1d92b696ef56f4944df036b8a78364ebb4 2019-01-31
and
- 10.1 commit 243f829c1c772c1b8e9e0717fbf5839e84244559 2019-02-15
deliver
ALTER IGNORE TABLE t_innodb_COPY    MODIFY COLUMN c INT NOT NULL, ALGORITHM = COPY;
Warnings:
Warning 1265    Data truncated for column 'c' at row 1
ALTER IGNORE TABLE t_innodb_INPLACE MODIFY COLUMN c INT NOT NULL, ALGORITHM = INPLACE;
ERROR 22004: Invalid use of NULL value

Comment by Marko Mäkelä [ 2019-03-04 ]

The sql_mode in the test effectively turns off STRICT_ALL_TABLES or STRICT_TRANS_TABLES. Starting with MDEV-16365 (and MDEV-14168) in MariaDB 10.3.8, this should be equivalent to executing ALTER IGNORE TABLE, which should update the NULL column values in a similar way as ALGORITHM=COPY would.

In MariaDB 10.0, 10.1 and 10.2, we should have refused ALTER IGNORE TABLE when introducing a NOT NULL attribute. (This is available when one of strict_all_tables or strict_trans_tables is set in sql_mode.) ALGORITHM=INPLACE allowed the ALTER IGNORE TABLE but returned an error for the NULL value. With the fix in place, 10.1 and 10.2 will refuse ALTER IGNORE TABLE for introducing NOT NULL attribute, and ALGORITHM=COPY will do the right thing.

While merging the fix from 10.1 to 10.2, I noticed that a regression had been introduced in MariaDB 10.2.2: ALTER IGNORE TABLE would fail to refuse ALGORITHM=INPLACE for ADD PRIMARY KEY or ADD UNIQUE KEY. I restored that code and added a test case.

For MariaDB Server 10.3, we need a bigger fix. Either we will have to refuse the ALTER IGNORE TABLE if indexed virtual columns depend on the column, or we will have to compute the dependent indexed virtual column values when updating the NULL column values.

Comment by Marko Mäkelä [ 2019-03-05 ]

The bug in MariaDB 10.3 only affects generated stored columns. For indexed virtual columns, we would use ALGORITHM=COPY because the ALTER_COLUMN_VCOL flag would be set in ha_alter_info->handler_flags.

The bug in MariaDB 10.3 is not limited to ALTER IGNORE TABLE. It also occurs when sql_mode does not include either of STRICT_ALL_TABLES or STRICT_TRANS_TABLES. The bug is that the generated stored columns are not being recomputed when a NULL value in a base column is being replaced with something else. I believe that it is easiest to fall back to ALGORITHM=COPY in such cases.

Comment by Marko Mäkelä [ 2019-03-05 ]

I merged the fixes up to 10.3 so far. Part of the problem in 10.3 and 10.4 remains to be fixed by MDEV-18819.

Generated at Thu Feb 08 08:46:19 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.