Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 5.5.39, 10.0.14
-
Fix Version/s: 5.5.43
-
Component/s: Data Manipulation - Subquery, Optimizer
-
Labels:None
Description
MDEV-5721 is not fixed properly. This test case still fails:
CREATE TABLE t1 (i INT, state VARCHAR(995)) ENGINE=MyISAM;
INSERT INTO t1 VALUES (2,'Louisiana'),(9,'Maine');
CREATE TABLE t2 (state VARCHAR(995), j INT) ENGINE=MyISAM;
INSERT INTO t2 VALUES ('Louisiana',9),('Alaska',5);
INSERT INTO t2 SELECT t2.* FROM t2 JOIN t2 AS t3 JOIN t2 AS t4 JOIN t2 AS t5;
SET @@max_heap_table_size= 16384;
set @@optimizer_switch='derived_merge=OFF';
SELECT * FROM t1 AS t1_1 LEFT JOIN ( t1 AS t1_2 INNER JOIN (SELECT * FROM t2) v2 ON t1_2.i = j ) ON t1_1.state = v2.state;
DROP TABLE t1, t2;
Due to differences in:
1. key length computation: key_length() in check_tmp_key vs pack_length() in create_internal_tmp_table
2. key length comparing:
"key_len <= MI_MAX_KEY_LENGTH" in check_tmp_table
vs
"keyinfo->key_length >= table->file->max_key_length()" in create_internal_tmp_table
Gliffy Diagrams
Attachments
Issue Links
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
with CHAR(998), in create_internal_tmp_table() we have:
(gdb) p keyinfo->key_length $24 = 5 (gdb) p table->file->max_key_length() $25 = 1000
with CHAR(995), in create_internal_tmp_table() we have:
(gdb) p keyinfo->key_length $10 = 1000 (gdb) p share->keys $11 = 1 (gdb) p table->file->max_key_length() $12 = 1000
The suspect is this line:
bzero(seg, sizeof(*seg) * keyinfo->user_defined_key_parts);
if (keyinfo->key_length >= table->file->max_key_length() ||
keyinfo->user_defined_key_parts > table->file->max_key_parts() ||
share->uniques)
why do we have >= in
if (keyinfo->key_length >= table->file->max_key_length() ||
So, this basically confirms Patryk Pomykalski's initial suggestion that instead of >= we should have > here.
This test case continues to fail despite the changes discussed previously:
CREATE TABLE t1 (i INT, state VARCHAR(996)) ENGINE=MyISAM;
INSERT INTO t1 VALUES (2,'Louisiana'),(9,'Maine');
CREATE TABLE t2 (state VARCHAR(996), j INT) ENGINE=MyISAM;
INSERT INTO t2 VALUES ('Louisiana',9),('Alaska',5);
INSERT INTO t2 SELECT t2.* FROM t2 JOIN t2 AS t3 JOIN t2 AS t4 JOIN t2 AS t5 JOIN t2 AS t6;
SET @@max_heap_table_size= 16384;
set @@optimizer_switch='derived_merge=OFF';
set @@optimizer_switch='extended_keys=ON';
SELECT * FROM t1 AS t1_1 LEFT JOIN ( t1 AS t1_2 INNER JOIN (SELECT * FROM t2) v2 ON t1_2.i = j ) ON t1_1.state = v2.state LIMIT 1;
DROP TABLE t1, t2;
The test case with the VARCHAR length set at 996 causes the check
keyinfo->key_length > table->file->max_key_length()
to fail, with the key_length == 1001.
It happens similarly with the length of 997, except the key_length in this case is 1002.
For 998 this passes, as the key length is set to 5.
Discovered thus far:
The table (v2), before being converted to aria, has a max_key_length() of 3072 and a key_length of either 1001 or 1002 (for 996 or 997 varchar length field respectively)
After it gets converted, the key_length remains the same, but the max_key_length supported by the ARIA engine is below the threshold.
Preliminary ideas for a fix:
- Check key size before converting to aria and shrink the key.
- Or choose an engine that supports the required key size.
Debugging a case with VARCHAR(997).
The sequence of calls is as follows:
1. create_tmp_table ( do_not_open=true ...)
https://gist.github.com/spetrunia/7e5973de60050f5b2cdb
2. TABLE::check_tmp_key()
https://gist.github.com/spetrunia/98ca6e80011681f271aa
3. TABLE::add_tmp_key()
https://gist.github.com/spetrunia/30b03b4f1b30da83001a
The calculations at #2 make sense. It takes 997 as char length, then it adds 1 byte for NULL-flag, then it adds two bytes for length.
Calculations in #3 do not make sense:
- it takes field->pack_length() = 999 // I think this is 997 + 2 bytes for key length
- then it adds HA_KEY_NULL_LENGTH=1 for NULL // this is ok
- then it adds HA_KEY_BLOB_LENGTH=2 // this is wrong, because 2 bytes have already been added.
Looking at field.h
uint32 pack_length() const { return (uint32) field_length+length_bytes; }
uint32 key_length() const { return (uint32) field_length; }
but TABLE::create_key_part_by_field does this:
key_part_info->length= (uint16) field->pack_length();
...
if (field->type() == MYSQL_TYPE_BLOB ||
field->type() == MYSQL_TYPE_GEOMETRY ||
field->real_type() == MYSQL_TYPE_VARCHAR)
{
key_part_info->store_length+= HA_KEY_BLOB_LENGTH;
keyinfo->key_length+= HA_KEY_BLOB_LENGTH; // ???
apparently TABLE::create_key_part_by_field adds the length twice.
Ok to push.
I think, the patch is sufficiently simple so that approval of one reviewer is sufficient.
If I replace 995 with 998 in the example, the error goes away:
CREATE TABLE t1 (i INT, state VARCHAR(998)) ENGINE=MyISAM; INSERT INTO t1 VALUES (2,'Louisiana'),(9,'Maine'); CREATE TABLE t2 (state VARCHAR(998), j INT) ENGINE=MyISAM; INSERT INTO t2 VALUES ('Louisiana',9),('Alaska',5); INSERT INTO t2 SELECT t2.* FROM t2 JOIN t2 AS t3 JOIN t2 AS t4 JOIN t2 AS t5; SET @@max_heap_table_size= 16384; set @@optimizer_switch='derived_merge=OFF'; SELECT * FROM t1 AS t1_1 LEFT JOIN ( t1 AS t1_2 INNER JOIN (SELECT * FROM t2) v2 ON t1_2.i = j ) ON t1_1.state = v2.state; DROP TABLE t1, t2;