maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Vicentiu,

Please find some cosmetic feedback below. Ok to push after it is addressed.


On Tue, Feb 09, 2016 at 04:02:58PM +0200, Vicentiu Ciorbaru wrote:
> revision-id: 841c317d237453f3e6c253e2a884cad73622b59a (mariadb-5.5.47-33-g841c317)
> parent(s): ad94790f468bacb2cd62d02d3a6e7f012ca72e65
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2016-02-09 16:00:18 +0200
> message:
> 
> MDEV-7122: Assertion `0' failed in subselect_hash_sj_engine::init
> 
> The select mentioned in the bug attempted to create a temporary table
> using the maria storage engine. The table needs to have primary keys such that
> duplicates can be removed. Unfortunately this use case has a longer
> than allowed key and the tmp table got created without a temporary key.
> We must not allow materialization for the subquery if the total key
> length and key parts is greater than what the storage engine supports.
> 
> ---
>  mysql-test/r/subselect.result             | 12 ++++++++++++
>  mysql-test/r/subselect_no_mat.result      | 12 ++++++++++++
>  mysql-test/r/subselect_no_opts.result     | 12 ++++++++++++
>  mysql-test/r/subselect_no_scache.result   | 12 ++++++++++++
>  mysql-test/r/subselect_no_semijoin.result | 12 ++++++++++++
>  mysql-test/t/subselect.test               |  9 +++++++++
>  sql/opt_subselect.cc                      | 10 ++++++++++
>  sql/sql_select.cc                         | 25 +++++++++++++++++++++++++
>  8 files changed, 104 insertions(+)
> 
> diff --git a/mysql-test/r/subselect.result b/mysql-test/r/subselect.result
> index ba52fd9..27e8eaa 100644
> --- a/mysql-test/r/subselect.result
> +++ b/mysql-test/r/subselect.result
> @@ -7081,3 +7081,15 @@ sq
>  NULL
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +# MDEV-7122
> +# Assertion `0' failed in subselect_hash_sj_engine::init
> +#
> +SET SESSION big_tables=1;

Please unset the variable back at the end of this testcase. We don't want the
rest of the test to run with non-standard settings.

> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +a
> +0
> +0
> +0
> +DROP TABLE t1;
> diff --git a/mysql-test/r/subselect_no_mat.result b/mysql-test/r/subselect_no_mat.result
> index d8d5c4e..4727f9f 100644
> --- a/mysql-test/r/subselect_no_mat.result
> +++ b/mysql-test/r/subselect_no_mat.result
> @@ -7078,6 +7078,18 @@ sq
>  NULL
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +# MDEV-7122
> +# Assertion `0' failed in subselect_hash_sj_engine::init
> +#
> +SET SESSION big_tables=1;
> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +a
> +0
> +0
> +0
> +DROP TABLE t1;
>  set optimizer_switch=default;
>  select @@optimizer_switch like '%materialization=on%';
>  @@optimizer_switch like '%materialization=on%'
> diff --git a/mysql-test/r/subselect_no_opts.result b/mysql-test/r/subselect_no_opts.result
> index 78908bc..7c542dd 100644
> --- a/mysql-test/r/subselect_no_opts.result
> +++ b/mysql-test/r/subselect_no_opts.result
> @@ -7076,4 +7076,16 @@ sq
>  NULL
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +# MDEV-7122
> +# Assertion `0' failed in subselect_hash_sj_engine::init
> +#
> +SET SESSION big_tables=1;
> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +a
> +0
> +0
> +0
> +DROP TABLE t1;
>  set @optimizer_switch_for_subselect_test=null;
> diff --git a/mysql-test/r/subselect_no_scache.result b/mysql-test/r/subselect_no_scache.result
> index d1de4b8..3b58b3c 100644
> --- a/mysql-test/r/subselect_no_scache.result
> +++ b/mysql-test/r/subselect_no_scache.result
> @@ -7087,6 +7087,18 @@ sq
>  NULL
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +# MDEV-7122
> +# Assertion `0' failed in subselect_hash_sj_engine::init
> +#
> +SET SESSION big_tables=1;
> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +a
> +0
> +0
> +0
> +DROP TABLE t1;
>  set optimizer_switch=default;
>  select @@optimizer_switch like '%subquery_cache=on%';
>  @@optimizer_switch like '%subquery_cache=on%'
> diff --git a/mysql-test/r/subselect_no_semijoin.result b/mysql-test/r/subselect_no_semijoin.result
> index 524a5dd..9246ffb 100644
> --- a/mysql-test/r/subselect_no_semijoin.result
> +++ b/mysql-test/r/subselect_no_semijoin.result
> @@ -7076,5 +7076,17 @@ sq
>  NULL
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +# MDEV-7122
> +# Assertion `0' failed in subselect_hash_sj_engine::init
> +#
> +SET SESSION big_tables=1;
> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +a
> +0
> +0
> +0
> +DROP TABLE t1;
>  set @optimizer_switch_for_subselect_test=null;
>  set @join_cache_level_for_subselect_test=NULL;
> diff --git a/mysql-test/t/subselect.test b/mysql-test/t/subselect.test
> index 67eac0d..5ca21ea 100644
> --- a/mysql-test/t/subselect.test
> +++ b/mysql-test/t/subselect.test
> @@ -5964,3 +5964,12 @@ EXECUTE stmt;
>  
>  deallocate prepare stmt;
>  drop table t1,t2,t3,t4;
> +
> +--echo # MDEV-7122
> +--echo # Assertion `0' failed in subselect_hash_sj_engine::init
> +--echo #
> +SET SESSION big_tables=1;
> +CREATE TABLE t1(a char(255) DEFAULT '', KEY(a(10))) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
> +INSERT INTO t1 VALUES(0),(0),(0);
> +SELECT * FROM t1 WHERE a IN(SELECT MIN(a) FROM t1);
> +DROP TABLE t1;
> diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc
> index 827290f..a39e04b 100644
> --- a/sql/opt_subselect.cc
> +++ b/sql/opt_subselect.cc
> @@ -830,12 +830,14 @@ bool subquery_types_allow_materialization(Item_in_subselect *in_subs)
>    in_subs->sjm_scan_allowed= FALSE;
>    
>    bool all_are_fields= TRUE;
> +  uint32 total_key_length = 0;
>    for (uint i= 0; i < elements; i++)
>    {
>      Item *outer= in_subs->left_expr->element_index(i);
>      Item *inner= it++;
>      all_are_fields &= (outer->real_item()->type() == Item::FIELD_ITEM && 
>                         inner->real_item()->type() == Item::FIELD_ITEM);
> +    total_key_length += inner->max_length;
>      if (outer->cmp_type() != inner->cmp_type())
>        DBUG_RETURN(FALSE);
>      switch (outer->cmp_type()) {
> @@ -866,6 +868,14 @@ bool subquery_types_allow_materialization(Item_in_subselect *in_subs)
>      }
>    }
>  
> +  /*
> +     Make sure that create_tmp_table will not fail due to too long keys.
> +     See MDEV-7122. This check is performed inside create_tmp_table also and
> +     we must do it so that we know the table has keys created.
> +  */
> +  if (total_key_length > HA_MAX_KEY_LENGTH || elements > HA_MAX_KEY_SEG)
> +    DBUG_RETURN(FALSE);
> +
>    in_subs->types_allow_materialization= TRUE;
>    in_subs->sjm_scan_allowed= all_are_fields;
>    DBUG_PRINT("info",("subquery_types_allow_materialization: ok, allowed"));
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index f43f506..218a4ce 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -15307,6 +15307,13 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List<Item> &fields,
>    if (!table->file)
>      goto err;
>  
> +  /*
> +    Temporary table storage engines must allow keys of at least
> +    HA_MAX_KEY_LENGT and at least HA_MAX_KEY_SEG key parts.
psergey: typo. ^^^^^^^^^^^

> +  */
> +  DBUG_ASSERT(table->file->max_key_length() >= HA_MAX_KEY_LENGTH &&
> +              table->file->max_key_parts() >= HA_MAX_KEY_SEG);
> +
>    if (!using_unique_constraint)
>      reclength+= group_null_items;	// null flag is stored separately
>  
> @@ -15940,6 +15947,12 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
>        goto err;
>  
>      bzero(seg, sizeof(*seg) * keyinfo->key_parts);
> +    /*
> +       Note that a simillar check is performed during
typo                   ^^^^^^^^^
> +       subquery_types_allow_materialization. See MDEV-7122 for more details as
> +       to why. Whenever this changes, it must be updated there as well, for
> +       all tmp_table engines.
> +    */
>      if (keyinfo->key_length > table->file->max_key_length() ||
>  	keyinfo->key_parts > table->file->max_key_parts() ||
>  	share->uniques)
> @@ -16126,6 +16139,12 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
>        goto err;
>  
>      bzero(seg, sizeof(*seg) * keyinfo->key_parts);
> +    /*
> +       Note that a simillar check is performed during
typo                   ^^^^^^^^^
> +       subquery_types_allow_materialization. See MDEV-7122 for more details as
> +       to why. Whenever this changes, it must be updated there as well, for
> +       all tmp_table engines.
> +    */
>      if (keyinfo->key_length > table->file->max_key_length() ||
>  	keyinfo->key_parts > table->file->max_key_parts() ||
>  	share->uniques)
> @@ -16286,6 +16305,12 @@ create_internal_tmp_table_from_heap2(THD *thd, TABLE *table,
>    if (!(new_table.file= get_new_handler(&share, &new_table.mem_root,
>                                          new_table.s->db_type())))
>      DBUG_RETURN(1);				// End of memory
> +  /*
> +    Temporary table storage engines must allow keys of at least
> +    HA_MAX_KEY_LENGTH and at least HA_MAX_KEY_SEG key parts.
> +  */
> +  DBUG_ASSERT(new_table.file->max_key_length() >= HA_MAX_KEY_LENGTH &&
> +              new_table.file->max_key_parts() >= HA_MAX_KEY_SEG);
>  
>    save_proc_info=thd->proc_info;
>    thd_proc_info(thd, proc_info);
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog