[MDEV-6838] Using too big key for internal temp tables Created: 2014-10-05  Updated: 2015-03-06  Resolved: 2015-03-04

Status: Closed
Project: MariaDB Server
Component/s: Data Manipulation - Subquery, Optimizer
Affects Version/s: 5.5.39, 10.0.14
Fix Version/s: 5.5.43

Type: Bug Priority: Major
Reporter: Patryk Pomykalski Assignee: Vicențiu Ciorbaru
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Relates
relates to MDEV-5721 Using too big key for internal temp t... Closed
relates to MDEV-6745 Wrong result (NULLs instead of real v... Closed

 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

 Comments   
Comment by Sergei Petrunia [ 2015-02-05 ]

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;

Comment by Sergei Petrunia [ 2015-02-05 ]

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() ||

Comment by Sergei Petrunia [ 2015-02-05 ]

So, this basically confirms pomyk's initial suggestion that instead of >= we should have > here.

Comment by Vicențiu Ciorbaru [ 2015-02-16 ]

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.

Comment by Vicențiu Ciorbaru [ 2015-02-16 ]

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.
Comment by Sergei Petrunia [ 2015-02-16 ]

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.
Comment by Sergei Petrunia [ 2015-02-16 ]

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.

Comment by Sergei Petrunia [ 2015-03-04 ]

Ok to push.

I think, the patch is sufficiently simple so that approval of one reviewer is sufficient.

Comment by Vicențiu Ciorbaru [ 2015-03-04 ]

Fixed with commit:
https://github.com/MariaDB/server/commit/45b6edb158f8101d641f550179ee15df363f686f

Generated at Thu Feb 08 07:14:58 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.