Description:
In redundant row_format table, if we add a default null instant col, then update the null col to empty string, which will cause problem.
In debug build mysql version, I got a assert failure, which is expected in current mysql code.
However, in release build, mysqld will not crash, the result is more serious:
1. the update operation is successful, but the record is not changed.
2. mysqld access invalid memory in function rec_set_nth_field
The behavior is unexpected.
What worries me the most is can mysqld be restarted when kill mysqld during batch updating default NULL instant col to empty string?
By the way, will redundant format be deprecated in a future release?
How to repeat:
Run the following testcase:
cat suite/innodb/t/redundant_upd_instant_to_empty.test
--source include/have_debug.inc
--source include/count_sessions.inc
connection default;
CREATE TABLE t1 (a INT, b INT, KEY(b)) row_format=REDUNDANT;
INSERT INTO t1 VALUES(0,0) ,(1,1),(2,2);
ALTER TABLE t1 ADD COLUMN c varchar(25) DEFAULT NULL, ALGORITHM=INSTANT;
UPDATE t1 SET c="" WHERE a=0;
DROP TABLE t1;
--source include/wait_until_count_sessions.inc
Suggested fix:
In function row_upd_changes_field_size_or_external, just like compact row_format record, if the old field's value is NULL, we should update old_len to UNIV_SQL_NULL.
For example:
[lb@mysql-develop-0001 install]$ git diff
diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc
index 27eec28..b74086c 100644
--- a/storage/innobase/row/row0upd.cc
+++ b/storage/innobase/row/row0upd.cc
@@ -398,30 +398,22 @@ bool row_upd_changes_field_size_or_external(
old_len = rec_offs_nth_size(index, offsets, field_no);
- if (rec_offs_comp(offsets)) {
- if (rec_offs_nth_sql_null(index, offsets, field_no)) {
- /* Note that in the compact table format,
- for a variable length field, an SQL NULL
- will use zero bytes in the offset array
- at the start of the physical record, but
- a zero-length value (empty string) will
- use one byte! Thus, we cannot use
- update-in-place if we update an SQL NULL
- varchar to an empty string! */
-
- old_len = UNIV_SQL_NULL;
- } else if (rec_offs_nth_default(index, offsets, field_no)) {
- /* This will force to do pessimistic update,
- since the default value is not inlined,
- so any update to it will extend the record. */
- old_len = UNIV_SQL_ADD_COL_DEFAULT;
- }
- } else {
- /* REDUNDANT row format, if it updates the field with
- not inlined default value, do it in pessimistic way */
- if (rec_offs_nth_default(index, offsets, field_no)) {
- old_len = UNIV_SQL_ADD_COL_DEFAULT;
- }
+ if (rec_offs_nth_sql_null(index, offsets, field_no)) {
+ /* Note that in the compact table format,
+ for a variable length field, an SQL NULL
+ will use zero bytes in the offset array
+ at the start of the physical record, but
+ a zero-length value (empty string) will
+ use one byte! Thus, we cannot use
+ update-in-place if we update an SQL NULL
+ varchar to an empty string! */
+
+ old_len = UNIV_SQL_NULL;
+ } else if (rec_offs_nth_default(index, offsets, field_no)) {
+ /* This will force to do pessimistic update,
+ since the default value is not inlined,
+ so any update to it will extend the record. */
+ old_len = UNIV_SQL_ADD_COL_DEFAULT;
}
Description: In redundant row_format table, if we add a default null instant col, then update the null col to empty string, which will cause problem. In debug build mysql version, I got a assert failure, which is expected in current mysql code. However, in release build, mysqld will not crash, the result is more serious: 1. the update operation is successful, but the record is not changed. 2. mysqld access invalid memory in function rec_set_nth_field The behavior is unexpected. What worries me the most is can mysqld be restarted when kill mysqld during batch updating default NULL instant col to empty string? By the way, will redundant format be deprecated in a future release? How to repeat: Run the following testcase: cat suite/innodb/t/redundant_upd_instant_to_empty.test --source include/have_debug.inc --source include/count_sessions.inc connection default; CREATE TABLE t1 (a INT, b INT, KEY(b)) row_format=REDUNDANT; INSERT INTO t1 VALUES(0,0) ,(1,1),(2,2); ALTER TABLE t1 ADD COLUMN c varchar(25) DEFAULT NULL, ALGORITHM=INSTANT; UPDATE t1 SET c="" WHERE a=0; DROP TABLE t1; --source include/wait_until_count_sessions.inc Suggested fix: In function row_upd_changes_field_size_or_external, just like compact row_format record, if the old field's value is NULL, we should update old_len to UNIV_SQL_NULL. For example: [lb@mysql-develop-0001 install]$ git diff diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index 27eec28..b74086c 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -398,30 +398,22 @@ bool row_upd_changes_field_size_or_external( old_len = rec_offs_nth_size(index, offsets, field_no); - if (rec_offs_comp(offsets)) { - if (rec_offs_nth_sql_null(index, offsets, field_no)) { - /* Note that in the compact table format, - for a variable length field, an SQL NULL - will use zero bytes in the offset array - at the start of the physical record, but - a zero-length value (empty string) will - use one byte! Thus, we cannot use - update-in-place if we update an SQL NULL - varchar to an empty string! */ - - old_len = UNIV_SQL_NULL; - } else if (rec_offs_nth_default(index, offsets, field_no)) { - /* This will force to do pessimistic update, - since the default value is not inlined, - so any update to it will extend the record. */ - old_len = UNIV_SQL_ADD_COL_DEFAULT; - } - } else { - /* REDUNDANT row format, if it updates the field with - not inlined default value, do it in pessimistic way */ - if (rec_offs_nth_default(index, offsets, field_no)) { - old_len = UNIV_SQL_ADD_COL_DEFAULT; - } + if (rec_offs_nth_sql_null(index, offsets, field_no)) { + /* Note that in the compact table format, + for a variable length field, an SQL NULL + will use zero bytes in the offset array + at the start of the physical record, but + a zero-length value (empty string) will + use one byte! Thus, we cannot use + update-in-place if we update an SQL NULL + varchar to an empty string! */ + + old_len = UNIV_SQL_NULL; + } else if (rec_offs_nth_default(index, offsets, field_no)) { + /* This will force to do pessimistic update, + since the default value is not inlined, + so any update to it will extend the record. */ + old_len = UNIV_SQL_ADD_COL_DEFAULT; }