Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-6838

Using too big key for internal temp tables

Details

    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

      Attachments

        Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-5721 Using too big key for internal temp tables MariaDB 5.5.35

          • Major - Major loss of function.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-6745 Wrong result (NULLs instead of real values) with TEMPTABLE view, LEFT JOIN

          • Major - Major loss of function.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            View 4 older comments
            cvicentiu Vicențiu Ciorbaru added a comment - - edited

            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.
            cvicentiu Vicențiu Ciorbaru added a comment - - edited 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.
            psergei Sergei Petrunia added a comment -

            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.
            psergei Sergei Petrunia added a comment - 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.
            psergei Sergei Petrunia added a comment -

            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.

            psergei Sergei Petrunia added a comment - 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.
            psergei Sergei Petrunia added a comment -

            Ok to push.

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

            psergei Sergei Petrunia added a comment - Ok to push. I think, the patch is sufficiently simple so that approval of one reviewer is sufficient.
            cvicentiu Vicențiu Ciorbaru added a comment -

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

            cvicentiu Vicențiu Ciorbaru added a comment - Fixed with commit: https://github.com/MariaDB/server/commit/45b6edb158f8101d641f550179ee15df363f686f

            People

              cvicentiu Vicențiu Ciorbaru
              pomyk Patryk Pomykalski
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.