When removing the key build in column c1 in table t1, the error is gone.

Below is the explain plan obtained from version 10.5:

EXPLAIN EXTENDED 

SELECT f6 FROM (SELECT DISTINCT `c1` AS `f6` FROM `t1`) AS `dt1`

         JOIN (SELECT CHARSET(`c2`) AS `f5` FROM `t2`) AS `dt3`

              ON (EXISTS (SELECT `c2` FROM `t2`)) AND

(NOT ((CHARSET(`f5`)) NOT BETWEEN `f6` AND `f6`));

id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra

1	PRIMARY	<derived3>	system	NULL	NULL	NULL	NULL	1	100.00	

1	PRIMARY	<derived2>	ref	key0	key0	9	const	0	0.00	Using where

4	SUBQUERY	t2	system	NULL	NULL	NULL	NULL	1	100.00	

3	DERIVED	t2	system	NULL	NULL	NULL	NULL	1	100.00	

2	DERIVED	t1	range	c1	c1	9	NULL	2	50.00	Using where; Using index for group-by

Warnings:

Note	1003	/* select#1 */ select `dt1`.`f6` AS `f6` from (/* select#2 */ select distinct `test`.`t1`.`c1` AS `f6` from `test`.`t1` where <cache>(charset('binary')) between `test`.`t1`.`c1` and `test`.`t1`.`c1`) `dt1` where <cache>(charset('binary')) between `dt1`.`f6` and `dt1`.`f6`

Call stack of the crash:

1   optimize_keyuse             sql_select.cc    7189  0x555555f24f48 

2   make_join_statistics        sql_select.cc    5751  0x555555f2085e 

3   JOIN::optimize_inner        sql_select.cc    2386  0x555555f142f1 

4   JOIN::optimize              sql_select.cc    1740  0x555555f11c24 

5   mysql_derived_optimize      sql_derived.cc   1029  0x555555e60377 

6   mysql_handle_single_derived sql_derived.cc   200   0x555555e5dd99 

7   JOIN::optimize_inner        sql_select.cc    2204  0x555555f1387d 

8   JOIN::optimize              sql_select.cc    1740  0x555555f11c24 

9   mysql_select                sql_select.cc    4868  0x555555f1d66f 

10  mysql_explain_union         sql_select.cc    28125 0x555555f5efa7 

11  execute_sqlcom_select       sql_parse.cc     6356  0x555555ecada9 

12  mysql_execute_command       sql_parse.cc     4022  0x555555ec1f97 

... <More>                                                            

The crash happens because keyuse->used_tables == 2 when JOIN::table_count == 1. keyuse->used_tables == 2 due to keyuse->val->used_tables() == 2 and keyuse->val is Item_func_charset which is dbug_print'ed as "charset(t3.f5)".

As I can see, there are two JOIN objects involved here, the inner one is initialized at mysql_derived_optimize (frame 5 of the call stack) as

JOIN *join= derived->get_unit()->first_select()->join;

The outer join processed at frame 7 has table_count == 2, so keyuse->val->used_tables() == 2 makes sense for that join.

Simplified test case (crashes consistently, no need to run a cycle):

EXPLAIN EXTENDED 

     SELECT dt1_c1 FROM 

       (SELECT c1 AS dt1_c1 FROM t1) AS dt1

       JOIN 

       (SELECT 1 AS dt2_c2 FROM t2) AS dt2

          ON CHARSET(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;

Warnings:

Note	1003	

/* select#1 */ select `dt1`.`dt1_c1` AS `dt1_c1` 

   from 

   (/* select#2 */ select `test`.`t1`.`c1` AS `dt1_c1` from `test`.`t1` 

     where <cache>(charset(1)) between `test`.`t1`.`c1` and `test`.`t1`.`c1`) `dt1` 

   where <cache>(charset(1)) between `dt1`.`dt1_c1` and `dt1`.`dt1_c1`

• Note: rewritten query text in the warning is obtained by disabling the optimize_keyuse() call.

<cache>(charset(1)) between `test`.`t1`.`c1` and `test`.`t1`.`c1` is shown twice in the output, doesn't it look strange?

Observation: replacing CHARSET() with LENGTH() eliminates the crash, and the execution doesn't have duplicate call of the function octet_length():

EXPLAIN EXTENDED 

SELECT dt1_c1 FROM 

(SELECT c1 AS dt1_c1 FROM t1) AS dt1

JOIN 

(SELECT 1 AS dt2_c2 FROM t2) AS dt2

ON LENGTH(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;

id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra

1	PRIMARY	<derived3>	system	NULL	NULL	NULL	NULL	1	100.00	

1	PRIMARY	<derived2>	ref	key0	key0	9	const	0	0.00	Using where

3	DERIVED	t2	system	NULL	NULL	NULL	NULL	1	100.00	

2	DERIVED	t1	index	NULL	c1	9	NULL	1	100.00	Using index

Warnings:

Note	1003	

/* select#1 */ select `dt1`.`dt1_c1` AS `dt1_c1` 

  from 

  (/* select#2 */ select `test`.`t1`.`c1` AS `dt1_c1` from `test`.`t1`) `dt1` 

     where <cache>(octet_length(1)) between `dt1`.`dt1_c1` and `dt1`.`dt1_c1`

Looking at the result of rewrite of the problematic query

/* select#1 */ select `dt1`.`dt1_c1` AS `dt1_c1` 

   from 

   (/* select#2 */ select `test`.`t1`.`c1` AS `dt1_c1` from `test`.`t1` 

     where <cache>(charset(1)) between `test`.`t1`.`c1` and `test`.`t1`.`c1`) `dt1` 

   where <cache>(charset(1)) between `dt1`.`dt1_c1` and `dt1`.`dt1_c1`

I can see that condition CHARSET(dt2_c2) BETWEEN dt1_c1 AND dt1_c1 has been pushed down into the derived table `dt1`, which must not have happened. Only conditions depending solely on the derived table can be pushed into, but CHARSET(dt2_c2) is dependent on `dt2` and must not be pushed down.

The pushdown logic checks table dependencies but skips the check for const items. Const items are always considered as eligible for pushdown, and in this case Item_func_charset turns out to be erroneously const.

This happens because Item_func_charset inherits from Item_func_expr_str_metadata which overrides const_item() in the following way, forcing the const property:

bool const_item() const { return true; }

That was introduced with the commit

commit aa1002a35cefcc1851cbfb6b6b60463bda6f9aa3

Author: Alexander Barkov <bar@mariadb.org>, Wed Sep 2 08:20:49 2015 +0400 (9 years ago)

MDEV-8723 Wrong result for SELECT..WHERE COLLATION(a)='binary' AND a='a'

The same was made for Item_func_coercibility, so the crash is reproducible if CHARSET() is replaced with COERCIBILITY().

bar, psergei, please review the pull request.

Hi oleg.smirnov,

I think functions CHARSET() and COERCIBILITY should return const_item()==true. They are definitely constants during the query. They don't change their value per row.

They are very close to DEFAULT(field) in this sense. I checked this script:

create or replace table t1(a int default 1);

insert into t1 values (1),(2);

select * from t1 where a=default(a);

and Item_default_value::const_item() returns true during the select statement.

Can you please check the reported problem with Item_default_value? Does it also crash?

I filed a separate issue for DEFAULT() 'cause there is another call stack and it's reproducible not only on InnoDB but on MyISAM as well.

I agree with bar that CHARSET() and COERCIBILITY() should return const_item()==true. So this should be something related to the "condition pushdown into derived" optimization.

A little bit more simplified test case:

--source include/have_innodb.inc

CREATE TABLE t1 (c1 INT, KEY (c1)) ENGINE=InnoDB;

INSERT INTO t1 VALUES (1);

CREATE TABLE t2 (c2 INT);

INSERT INTO t2 VALUES (1);

SET optimizer_switch='derived_merge=off';

set optimizer_trace=1;

EXPLAIN EXTENDED

     SELECT dt1_c1 FROM 

       (SELECT c1 AS dt1_c1 FROM t1) AS dt1

       JOIN 

       (SELECT 1 AS dt2_c2 FROM t2) AS dt2

          ON CHARSET(dt2_c2) = dt1_c1;

To reiterate: CHARSET(dt2_c2) = dt1_c1 is pushed down into dt1: SELECT c1 AS dt1_c1 FROM t1 WHERE CHARSET(dt2_c2) = dt1_c1 and CHARSET(dt2_c2) is a const item. The optimizer considers using KEY(c1) on t1 for value CHARSET(dt2_c2) and performs some operations at optimize_keyuse(). Since CHARSET(dt2_c2)::used_tables() == 2 and the dt1's JOIN::table_count == 1, a crash occurs:

if (keyuse->used_tables &

	(map= (keyuse->used_tables & ~join->const_table_map &

	       ~OUTER_REF_TABLE_BIT)))

    {

      uint n_tables= my_count_bits(map);

      if (n_tables == 1)			// Only one table

      {

        DBUG_ASSERT(!(map & PSEUDO_TABLE_BITS)); // Must be a real table

        Table_map_iterator it(map);

        int tablenr= it.next_bit();

        DBUG_ASSERT(tablenr != Table_map_iterator::BITMAP_END);

	TABLE *tmp_table=join->table[tablenr];

        if (tmp_table) // already created

          keyuse->ref_table_rows= MY_MAX(tmp_table->file->stats.records, 100);

      }

    }

The first idea: condition with CHARSET(dt2_c2) must not be pushed into dt1 (but why not if it's const?)
Second idea: shouldn't we have JOIN::const_table_map == 2 after the pushdown? That eliminates the crash at optimize_keyuse().

The first idea: condition with CHARSET(dt2_c2) must not be pushed into dt1 (but why not if it's const?)

This may or may not be the solution. Will elaborate below.
(I'm wondering, why does pushdown code check const_tables and not just use used_tables ?)

Second idea: shouldn't we have JOIN::const_table_map == 2 after the pushdown? That eliminates the crash at optimize_keyuse().

Do you the child join object should have const_table_map=2? This doesn't look like a good idea, because the number 2 (table bit #1 counting from 0) comes from the parent select. The child join may not have two tables (like in this example) or it may have another table with table->map=2.

The fact that CHARSET(dt2_c2)::used_tables() == 2 after this expression has been pushed into dt1 is wrong.
I can see two options:

• If CHARSET(...) continues to refer to dt2, then it should have used_tables()=OUTER_REF_TABLE_BIT now. This is a bad approach to take as the optimizer doesn't currently support outer references inside derived tables.
• Can we assume that CHARSET(dt2.dt2_c2) does not depend on dt2 and so it should have used_tables()=0?

bar, any objections to making CHARSET(table.column) have used_tables()=0 ? currently it seems to have arg[0]->used_tables() ?

psergei, Sorry, I know methods used_tables() and const_tables() very badly. Cannot suggest anything.

The idea discussed during a meeting: move args[0]->charset_for_protocol() call to the preparation phase (fix_fields()) and remember the charset returned:

String *Item_func_charset::val_str(String *str)

{

  DBUG_ASSERT(fixed());

  uint dummy_errors;

  CHARSET_INFO *cs= args[0]->charset_for_protocol();

  null_value= 0;

  str->copy(cs->cs_name.str, cs->cs_name.length,

            &my_charset_latin1, collation.collation, &dummy_errors);

  return str;

}

So we can safely set used_tables() to return 0 and avoid referring to arguments (args[0]) during the execution phase (call to val_str() is made during the execution).

sanja, can you please review the PR?

Hi oleg.smirnov,

the patch is OK to push.

I have just one small suggestion: Please change the data type of the new member from:

CHARSET_INFO *m_cached_charset_info;

to

LEX_CSTRING m_cached_charset_info;

to avoid strlen() evaluation per row by evaluating it during fix_length_and_dec() instead.

Note, starting from 10.6 CHARSET_INFO was modified:

  const char *csname;

was changed to:

  LEX_CSTRING cs_name;

So starting from 10.6 strlen() won't be needed even in fix_length_and_dec().

Good point, bar! Implemented your suggestion and pushed to 10.5 (commit)

Note for the changelog: if a query used a derived table DT (a CTE or a
mergeable VIEW would work as well) and the WHERE clause compared columns
of DT with the value of CHARSET() or COERCIBILITY() function, the query
could produce wrong result or crash. The cause was incorrect processing
of these functions by derived condition pushdown optimization (
https://mariadb.com/kb/en/condition-pushdown-into-derived-table-optimization/)