MDEV-18598: Wrong results after instant integer conversions

dr-m · dr-m · commit 790b6f5ae2b8 · 2019-02-17T01:05:31.000+02:00
Field_str::is_equal(): Do not allow instant conversions between
BIT (which is stored big-endian) and integer types (which can
be stored big-endian or little-endian, depending on storage engine).

row_sel_field_store_in_mysql_format_func(): Properly extend
narrower integer and DATA_FIXBINARY values to the current format.
DATA_FIXBINARY was incorrectly padded with 0x20 instead of 0.
diff --git a/mysql-test/suite/innodb/r/instant_alter.result b/mysql-test/suite/innodb/r/instant_alter.result
@@ -737,6 +737,52 @@ DROP TABLE t1;
 CREATE TABLE t1(t TEXT NOT NULL, FULLTEXT(t)) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
 ALTER TABLE t1 MODIFY COLUMN t TEXT;
 DROP TABLE t1;
+CREATE TABLE t1 (f TINYINT, g SMALLINT UNSIGNED) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+INSERT INTO t1 VALUES(127,6502),(-128,33101);
+ALTER TABLE t1 MODIFY f SMALLINT DEFAULT 12345,
+MODIFY g BIGINT UNSIGNED DEFAULT 1234567;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f	g
+127	6502
+-128	33101
+DROP TABLE t1;
+CREATE TABLE t1 (f BIT(8)) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+INSERT INTO t1 VALUES (b'10000000'),(b'00000001');
+ALTER TABLE t1 MODIFY f BIT(16);
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+INSERT INTO t1 VALUES (b'1000000010101111'),(b'10000000');
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+1
+80AF
+80
+ALTER TABLE t1 MODIFY f SMALLINT;
+ERROR 22003: Out of range value for column 'f' at row 3
+ALTER TABLE t1 MODIFY f SMALLINT UNSIGNED;
+affected rows: 4
+info: Records: 4  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f
+128
+1
+32943
+128
+ALTER TABLE t1 MODIFY f BIT;
+ERROR 22001: Data too long for column 'f' at row 1
+ALTER TABLE t1 MODIFY f BIT(15);
+ERROR 22001: Data too long for column 'f' at row 3
+DELETE FROM t1 LIMIT 3;
+ALTER TABLE t1 MODIFY f BIT(15);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -1420,6 +1466,52 @@ DROP TABLE t1;
 CREATE TABLE t1(t TEXT NOT NULL, FULLTEXT(t)) ENGINE=InnoDB ROW_FORMAT=COMPACT;
 ALTER TABLE t1 MODIFY COLUMN t TEXT;
 DROP TABLE t1;
+CREATE TABLE t1 (f TINYINT, g SMALLINT UNSIGNED) ENGINE=InnoDB ROW_FORMAT=COMPACT;
+INSERT INTO t1 VALUES(127,6502),(-128,33101);
+ALTER TABLE t1 MODIFY f SMALLINT DEFAULT 12345,
+MODIFY g BIGINT UNSIGNED DEFAULT 1234567;
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f	g
+127	6502
+-128	33101
+DROP TABLE t1;
+CREATE TABLE t1 (f BIT(8)) ENGINE=InnoDB ROW_FORMAT=COMPACT;
+INSERT INTO t1 VALUES (b'10000000'),(b'00000001');
+ALTER TABLE t1 MODIFY f BIT(16);
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+INSERT INTO t1 VALUES (b'1000000010101111'),(b'10000000');
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+1
+80AF
+80
+ALTER TABLE t1 MODIFY f SMALLINT;
+ERROR 22003: Out of range value for column 'f' at row 3
+ALTER TABLE t1 MODIFY f SMALLINT UNSIGNED;
+affected rows: 4
+info: Records: 4  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f
+128
+1
+32943
+128
+ALTER TABLE t1 MODIFY f BIT;
+ERROR 22001: Data too long for column 'f' at row 1
+ALTER TABLE t1 MODIFY f BIT(15);
+ERROR 22001: Data too long for column 'f' at row 3
+DELETE FROM t1 LIMIT 3;
+ALTER TABLE t1 MODIFY f BIT(15);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -2103,6 +2195,52 @@ DROP TABLE t1;
 CREATE TABLE t1(t TEXT NOT NULL, FULLTEXT(t)) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
 ALTER TABLE t1 MODIFY COLUMN t TEXT;
 DROP TABLE t1;
+CREATE TABLE t1 (f TINYINT, g SMALLINT UNSIGNED) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+INSERT INTO t1 VALUES(127,6502),(-128,33101);
+ALTER TABLE t1 MODIFY f SMALLINT DEFAULT 12345,
+MODIFY g BIGINT UNSIGNED DEFAULT 1234567;
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f	g
+127	6502
+-128	33101
+DROP TABLE t1;
+CREATE TABLE t1 (f BIT(8)) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+INSERT INTO t1 VALUES (b'10000000'),(b'00000001');
+ALTER TABLE t1 MODIFY f BIT(16);
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+INSERT INTO t1 VALUES (b'1000000010101111'),(b'10000000');
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+1
+80AF
+80
+ALTER TABLE t1 MODIFY f SMALLINT;
+ERROR 22003: Out of range value for column 'f' at row 3
+ALTER TABLE t1 MODIFY f SMALLINT UNSIGNED;
+affected rows: 4
+info: Records: 4  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+f
+128
+1
+32943
+128
+ALTER TABLE t1 MODIFY f BIT;
+ERROR 22001: Data too long for column 'f' at row 1
+ALTER TABLE t1 MODIFY f BIT(15);
+ERROR 22001: Data too long for column 'f' at row 3
+DELETE FROM t1 LIMIT 3;
+ALTER TABLE t1 MODIFY f BIT(15);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+SELECT HEX(f) FROM t1;
+HEX(f)
+80
+DROP TABLE t1;
 disconnect analyze;
 SELECT variable_value-@old_instant instants
 FROM information_schema.global_status
diff --git a/mysql-test/suite/innodb/r/instant_alter_extend.result b/mysql-test/suite/innodb/r/instant_alter_extend.result
diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test
@@ -638,6 +638,40 @@ eval CREATE TABLE t1(t TEXT NOT NULL, FULLTEXT(t)) $engine;
 ALTER TABLE t1 MODIFY COLUMN t TEXT;
 DROP TABLE t1;
 
+# MDEV-18598 Assertions and wrong results after MDEV-15563 extending INT
+eval CREATE TABLE t1 (f TINYINT, g SMALLINT UNSIGNED) $engine;
+INSERT INTO t1 VALUES(127,6502),(-128,33101);
+--enable_info
+ALTER TABLE t1 MODIFY f SMALLINT DEFAULT 12345,
+MODIFY g BIGINT UNSIGNED DEFAULT 1234567;
+--disable_info
+SELECT * FROM t1;
+DROP TABLE t1;
+
+eval CREATE TABLE t1 (f BIT(8)) $engine;
+INSERT INTO t1 VALUES (b'10000000'),(b'00000001');
+--enable_info
+ALTER TABLE t1 MODIFY f BIT(16);
+--disable_info
+INSERT INTO t1 VALUES (b'1000000010101111'),(b'10000000');
+SELECT HEX(f) FROM t1;
+--error ER_WARN_DATA_OUT_OF_RANGE
+ALTER TABLE t1 MODIFY f SMALLINT;
+--enable_info
+ALTER TABLE t1 MODIFY f SMALLINT UNSIGNED;
+--disable_info
+SELECT * FROM t1;
+--error ER_DATA_TOO_LONG
+ALTER TABLE t1 MODIFY f BIT;
+--error ER_DATA_TOO_LONG
+ALTER TABLE t1 MODIFY f BIT(15);
+DELETE FROM t1 LIMIT 3;
+--enable_info
+ALTER TABLE t1 MODIFY f BIT(15);
+--disable_info
+SELECT HEX(f) FROM t1;
+DROP TABLE t1;
+
 dec $format;
 }
 disconnect analyze;
diff --git a/sql/field.cc b/sql/field.cc
@@ -9550,9 +9550,35 @@ uint Field_num::is_equal(Create_field *new_field)
 
   if (th == new_th && new_field->pack_length == pack_length())
     return IS_EQUAL_YES;
+  /* FIXME: Test and consider returning IS_EQUAL_YES for the following:
+  TINYINT UNSIGNED to BIT(8)
+  SMALLINT UNSIGNED to BIT(16)
+  MEDIUMINT UNSIGNED to BIT(24)
+  INT UNSIGNED to BIT(32)
+  BIGINT UNSIGNED to BIT(64)
+
+  BIT(1..7) to TINYINT, or BIT(1..8) to TINYINT UNSIGNED
+  BIT(9..15) to SMALLINT, or BIT(9..16) to SMALLINT UNSIGNED
+  BIT(17..23) to MEDIUMINT, or BIT(17..24) to MEDIUMINT UNSIGNED
+  BIT(25..31) to INT, or BIT(25..32) to INT UNSIGNED
+  BIT(57..63) to BIGINT, or BIT(57..64) to BIGINT UNSIGNED
+
+  Note: InnoDB stores integers in big-endian format, and BIT appears
+  to use big-endian format. For storage engines that use little-endian
+  format for integers, we can only return IS_EQUAL_YES for the TINYINT
+  conversion. */
 
   if (table->file->ha_table_flags() & HA_EXTENDED_TYPES_CONVERSION)
   {
+    /* For now, prohibit instant conversion between BIT and integers.
+    Note: pack_length(), which is compared below, is measured in
+    bytes, and for BIT the last byte may be partially occupied.  We
+    must not allow instant conversion to BIT such that the last byte
+    is partially occupied.
+    We could allow converting TINYINT UNSIGNED to BIT(8) or wider. */
+    if (th != new_th &&
+        (th == &type_handler_bit || new_th == &type_handler_bit))
+      return IS_EQUAL_NO;
     if (th->result_type() == new_th->result_type() &&
         new_field->pack_length >= pack_length())
       return IS_EQUAL_PACK_LENGTH_EXT;
diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
@@ -2708,7 +2708,6 @@ row_sel_field_store_in_mysql_format_func(
 	const byte*	data,
 	ulint		len)
 {
-	byte*			ptr;
 #ifdef UNIV_DEBUG
 	const dict_field_t*	field
 		= templ->is_virtual
@@ -2720,37 +2719,15 @@ row_sel_field_store_in_mysql_format_func(
 	UNIV_MEM_ASSERT_W(dest, templ->mysql_col_len);
 	UNIV_MEM_INVALID(dest, templ->mysql_col_len);
 
+	byte* pad = dest + len;
+
 	switch (templ->type) {
 		const byte*	field_end;
-		byte*		pad;
-	case DATA_INT:
-		/* Convert integer data from Innobase to a little-endian
-		format, sign bit restored to normal */
-
-		ptr = dest + len;
-
-		for (;;) {
-			ptr--;
-			*ptr = *data;
-			if (ptr == dest) {
-				break;
-			}
-			data++;
-		}
-
-		if (!templ->is_unsigned) {
-			dest[len - 1] = (byte) (dest[len - 1] ^ 128);
-		}
-
-		ut_ad(templ->mysql_col_len == len
-		      || !index->table->not_redundant());
-		break;
-
+	case DATA_CHAR:
+	case DATA_FIXBINARY:
 	case DATA_VARCHAR:
 	case DATA_VARMYSQL:
 	case DATA_BINARY:
-	case DATA_CHAR:
-	case DATA_FIXBINARY:
 		field_end = dest + templ->mysql_col_len;
 
 		if (templ->mysql_type == DATA_MYSQL_TRUE_VARCHAR) {
@@ -2771,7 +2748,14 @@ row_sel_field_store_in_mysql_format_func(
 
 		/* Pad with trailing spaces. */
 
-		pad = dest + len;
+		if (pad == field_end) {
+			break;
+		}
+
+		if (UNIV_UNLIKELY(templ->type == DATA_FIXBINARY)) {
+			memset(pad, 0, field_end - pad);
+			break;
+		}
 
 		ut_ad(templ->mbminlen <= templ->mbmaxlen);
 
@@ -2849,7 +2833,7 @@ row_sel_field_store_in_mysql_format_func(
 			done in row0mysql.cc, function
 			row_mysql_store_col_in_innobase_format(). */
 
-			memset(dest + len, 0x20, templ->mysql_col_len - len);
+			memset(pad, 0x20, templ->mysql_col_len - len);
 		}
 		break;
 
@@ -2864,13 +2848,29 @@ row_sel_field_store_in_mysql_format_func(
 	case DATA_FLOAT:
 	case DATA_DOUBLE:
 	case DATA_DECIMAL:
-		/* Above are the valid column types for MySQL data. */
 #endif /* UNIV_DEBUG */
 		ut_ad((templ->is_virtual && !field)
 		      || (field && field->prefix_len
 				? field->prefix_len == len
 				: templ->mysql_col_len == len));
 		memcpy(dest, data, len);
+		break;
+
+	case DATA_INT:
+		/* Convert InnoDB big-endian integer to little-endian
+		format, sign bit restored to 2's complement form */
+		DBUG_ASSERT(templ->mysql_col_len >= len);
+
+		byte* ptr = pad;
+		do *--ptr = *data++; while (ptr != dest);
+		byte b = templ->is_unsigned || !((pad[-1] ^= 0x80) & 0x80)
+			? 0 : 0xff;
+
+		if (ulint l = templ->mysql_col_len - len) {
+			DBUG_ASSERT(!index->table->not_redundant());
+			memset(pad, b, l);
+		}
+		break;
 	}
 }
 
