Bug #40369 dtype_get_sql_null_size() returns 0 or 1, not the size
Submitted: 28 Oct 2008 10:17 Modified: 12 Mar 23:13
Reporter: Marko Mäkelä
Status: Closed
Category:Server: InnoDB Severity:S3 (Non-critical)
Version:5.1.12 OS:Any
Assigned to: Marko Mäkelä Target Version:5.1+
Triage: Triaged: D4 (Minor) / R2 (Low)

[28 Oct 2008 10:17] Marko Mäkelä
Description:
The function dtype_get_sql_null_size() should return the storage size of a column that is
SQL NULL. For fixed-length columns in ROW_FORMAT=REDUNDANT, the size should be equal to
the fixed length. However, this function would return 1.

In newer InnoDB ROW_FORMATs (COMPACT is the default since 5.0.3, and the InnoDB plugin
introduced DYNAMIC and COMPRESSED), SQL NULL columns occupy zero space. In this case, the
function correctly returns 0.

How to repeat:
Unsure, but something along these lines:

CREATE TABLE t(a char(5)) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
INSERT INTO t VALUES(NULL);

Suggested fix:
Remove the > 0 in the function body of dtype_get_sql_null_size().
[28 Oct 2008 10:19] Marko Mäkelä
This bug was introduced when fixing Bug #20877.
[28 Oct 2008 11:01] Marko Mäkelä
Verified with this test case:

CREATE TABLE t1(a CHAR(5)) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;

INSERT INTO t1 VALUES(NULL),('abcde');

The record format is (DB_ROW_ID, DB_TRX_ID, DB_ROLL_PTR, a). The two inserted user
records on the index page will be as follows:

94 13 0c 06 - offsets to column ends: 6, 12, 19, 20 + NULL bit set
00 00 10 09 00 a5 - REC_N_OLD_EXTRA_BYTES
00 00 00 00 02 00 - DB_ROW_ID
00 00 00 00 03 02 - DB_TRX_ID
80 00 00 00 2d 01 10 - DB_ROLL_PTR
00 - a=NULL (should be 5 bytes instead of 1)

18 13 0c 06 - offsets to column ends: 6, 12, 19, 24
00 00 18 09 00 74 - REC_N_OLD_EXTRA_BYTES
00 00 00 00 02 01 - DB_ROW_ID
00 00 00 00 03 02 - DB_TRX_ID
80 00 00 00 2d 01 1e - DB_ROLL_PTR
61 62 63 64 65 - a='abcde'

The impact of this bug seems low: in-place updates of columns from NULL to non-NULL will
fail. Such updates will be mapped to delete+insert.
[28 Oct 2008 15:52] Marko Mäkelä
After applying this patch, both records are of the same length, as they should be:

Index: include/data0type.ic
===================================================================
--- include/data0type.ic	(revision 2875)
+++ include/data0type.ic	(working copy)
@@ -558,5 +558,5 @@ dtype_get_sql_null_size(
 	const dtype_t*	type)	/* in: type */
 {
 	return(dtype_get_fixed_size_low(type->mtype, type->prtype, type->len,
-					type->mbminlen, type->mbmaxlen) > 0);
+					type->mbminlen, type->mbmaxlen));
 }

Here is an excerpt from the clustered index B-tree page:

98 13 0c 06 - offsets to column ends: 6, 12, 19, 24 + NULL
00 00 10 09 00 a9
00 00 00 00 02 00 DB_ROW_ID
00 00 00 00 03 02 DB_TRX_ID
80 00 00 00 2d 01 10 DB_ROLL_PTR
00 00 00 00 00 a=NULL

18 13 0c 06 - offsets to column ends: 6, 12, 19, 24
00 00 18 09 00 74
00 00 00 00 02 01 DB_ROW_ID
00 00 00 00 03 02 DB_TRX_ID
80 00 00 00 2d 01 1e DB_ROLL_PTR
61 62 63 64 65 a='abcde'

The only impact of this bug seems to be that fixed-length columns can't be updated in
place from or to NULL even in ROW_FORMAT=REDUNDANT. (SQL NULL always is 0 bytes in
ROW_FORMAT=COMPACT and later.)
[29 Oct 2008 8:03] Marko Mäkelä
An important question is if this bug has broken compatibility of ROW_FORMAT=REDUNDANT
tables. In other words, can buggy versions of InnoDB (5.1.12 and later) corrupt tables
created with non-buggy versions, and vice versa?

Luckily, it seems that the problematic function btr_cur_update_in_place() is only called
in three places. In btr_cur_optimistic_update(), it is called if
row_upd_changes_field_size_or_external() returns FALSE, which it should not, because the
size of NULL vs. non-NULL would differ due to this bug.

The functions row_upd_clust_rec() and row_upd_in_place_in_select() call
btr_cur_update_in_place() without checking row_upd_changes_field_size_or_external().
However, these code paths can only be reached when the internal SQL parser of InnoDB is
used, that is, when updating InnoDB data dictionary tables, which are in
ROW_FORMAT=REDUNDANT. InnoDB data dictionary columns are apparently never set NULL or
non-NULL. If they were, anyone who had upgraded old databases to 5.1.12 or later should
have noticed problems.

I will run some tests to confirm this analysis.
[12 Mar 23:13] Paul DuBois
Noted in 5.1.32, 6.0.10 changelogs.

For InnoDB tables that used ROW_FORMAT=REDUNDANT, storage size of
NULL columns could be determined incorrectly.