[MDEV-8565] [PATCH] COLUMN_CHECK fails on valid data Created: 2015-07-31 Updated: 2015-11-23 Resolved: 2015-10-19 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Dynamic Columns |
| Affects Version/s: | 10.0.20, 10.1.7, 10.1.8, 10.0.21-galera |
| Fix Version/s: | 10.0.22, 10.1.9 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Vincent Milum Jr | Assignee: | Oleksandr Byelkin |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Environment: |
Server version: 10.0.20-MariaDB-1~wheezy-wsrep-log |
||
| Issue Links: |
|
||||||||
| Description |
|
COLUMN_CHECK fails on valid data
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. |
| Comments |
| Comment by Elena Stepanova [ 2015-07-31 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
Thanks for the report. | ||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vincent Milum Jr [ 2015-08-09 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
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:
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)
1) column 1 header is bypassed (first), and then column 2 header is checked. column 1 and 2 data offsets compared = 0 length, fails. 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:
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. | ||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Golubchik [ 2015-08-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
pull request with a fix: https://github.com/MariaDB/server/pull/90 | ||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Vincent Milum Jr [ 2015-10-17 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
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. | ||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Oleksandr Byelkin [ 2015-10-19 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
I put a bit different check: zero size can be int and uint and not other (except NULL, but it is processed). | ||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Oleksandr Byelkin [ 2015-10-19 ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
Thank you a lot! I pushed it with changed check (more strict and takes into account almost impossible case of uint) and test suite. |