Details
-
Bug
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
-
10.0.20, 10.1.7, 10.1.8, 10.0.21-galera
-
None
-
Server version: 10.0.20-MariaDB-1~wheezy-wsrep-log
Description
COLUMN_CHECK fails on valid data
SELECT COLUMN_CHECK(COLUMN_CREATE('a',0,'b','1'))
|
Result: 0 (failed)
|
|
|
SELECT COLUMN_CHECK(COLUMN_CREATE('a',1,'b','1'))
|
Result: 1 (correct)
|
|
|
SELECT COLUMN_JSON(COLUMN_CREATE('a',0,'b','1'))
|
Result: {"a":0,"b":"1"} (correct)
|
|
|
SELECT COLUMN_JSON(COLUMN_CREATE('a',1,'b','1'))
|
Result: {"a":1,"b":"1"} (correct)
|
It appears that COLUMN_CHECK fails when one of the dynamic column values is set to INTEGER 0, and only when there is more than 1 dynamic column present. COLUMN_JSON is able to parse the results properly, so the returned values from COLUMN_CREATE appears to be correct.
Attachments
Issue Links
- relates to
-
-
- Closed
-
- links to
Activity
SELECT
|
COLUMN_CHECK(COLUMN_CREATE(1,0)) AS 'r0',
|
COLUMN_CHECK(COLUMN_CREATE(1,1)) AS 'r1',
|
COLUMN_CHECK(COLUMN_CREATE(1,0,2,1)) AS 'r2',
|
COLUMN_CHECK(COLUMN_CREATE(1,1,2,0)) AS 'r3',
|
COLUMN_CHECK(COLUMN_CREATE(1,'',2,'x')) AS 'r4',
|
COLUMN_CHECK(COLUMN_CREATE(1,'x',2,'')) AS 'r5',
|
COLUMN_CHECK(COLUMN_CREATE(1,0,2,1,3,1,4,1)) AS 'r6',
|
COLUMN_CHECK(COLUMN_CREATE(1,1,2,0,3,1,4,1)) AS 'r7',
|
COLUMN_CHECK(COLUMN_CREATE(1,1,2,1,3,0,4,1)) AS 'r8',
|
COLUMN_CHECK(COLUMN_CREATE(1,1,2,1,3,1,4,0)) AS 'r9';
|
|
|
+------+------+------+------+------+------+------+------+------+------+
|
| r0 | r1 | r2 | r3 | r4 | r5 | r6 | r7 | r8 | r9 |
|
+------+------+------+------+------+------+------+------+------+------+
|
| 1 | 1 | 0 | 1 | 1 | 1 | 0 | 0 | 0 | 1 |
|
+------+------+------+------+------+------+------+------+------+------+
|
The error only occurs when the ZERO value is not in the first column. I think I found where this is occurring, too. I've only read over the code, but I don't have an active compile environment to this out though.
https://github.com/MariaDB/server/blob/10.1/mysys/ma_dyncol.c - Line 3722:
if (prev_type != DYN_COL_NULL)
|
{
|
/* It is not first entry */
|
if (prev_data_offset >= data_offset)
|
{
|
DBUG_PRINT("info", ("Field order: %u Previous data offset: %u"
|
" >= Current data offset: %u",
|
(uint)i,
|
(uint)prev_data_offset,
|
(uint)data_offset));
|
goto end;
|
}
|
Looking at just the raw binary data from a few of these, a couple points instantly stand out. First, when integer ZERO is saved, it is saved as zero-length in the HEADER section and no data written at all in the DATA section. Then when COLUMN_CHECK is performed, is specifically checks to ensure that the length in the HEADER section is greater than zero bytes. Here is the raw HEX output of the last four items tested above. (note: added spaces to separate out sections)
000400 010000 020000 030008 040010 02 02 02
|
000400 010000 020008 030008 040010 02 02 02
|
000400 010000 020008 030010 040010 02 02 02
|
000400 010000 020008 030010 040018 02 02 02
|
1) column 1 header is bypassed (first), and then column 2 header is checked. column 1 and 2 data offsets compared = 0 length, fails.
2) column 2 header is checked. then column 3 header is checked. column 2 and 3 data offsets compared = 0 length, fails.
3) column 3 header is checked. then column 4 header is checked. column 3 and 4 data offsets compared = 0 length, fails.
4) column 4 header is checked. it has a data offset at the very end of the data section (beyond last byte of data), so it succeeds.
In the 4th condition, the data is then passed along to dynamic_column_uint_read() with a ZERO byte length. This particular function doesn't have a failure case at all, and will always return ER_DYNCOL_OK. This function also defaults to storing a value of ZERO before attempting to read from the data section. With a zero byte length, nothing is read, so this ZERO value is returned (and why the function still works with COLUMN_JSON and other functions that attempt to read the column data)
https://github.com/MariaDB/server/blob/10.1/mysys/ma_dyncol.c - Line 838:
static enum enum_dyncol_func_result
|
dynamic_column_uint_read(DYNAMIC_COLUMN_VALUE *store_it_here,
|
uchar *data, size_t length)
|
{
|
ulonglong value= 0;
|
size_t i;
|
|
|
for (i= 0; i < length; i++)
|
value+= ((ulonglong)data[i]) << (i*8);
|
|
|
store_it_here->x.ulong_value= value;
|
return ER_DYNCOL_OK;
|
}
|
So one of two solutions should happen here. Either
1) When integer values of ZERO are stored, they should actually be stored as 0x00 within the data section, with an appropriate length stored in the header. (looking at the code, it may be intentional to NOT store integer ZERO for space saving reasons tho)
OR
2) Update the check for zero-length data section to specifically support type integer (signed or unsigned) to not error out, and treat this as an intended condition.
pull request with a fix: https://github.com/MariaDB/server/pull/90
As a note: This bug is STILL present in the 10.1 "Stable" release, despite there being a pull request which fixes this bug for a couple months now.
I put a bit different check: zero size can be int and uint and not other (except NULL, but it is processed).
Thank you a lot! I pushed it with changed check (more strict and takes into account almost impossible case of uint) and test suite.
Thanks for the report.