From 3d6647c48d7cfbff65596b0eaafd67fa720289be Mon Sep 17 00:00:00 2001 From: Kamil Holubicki Date: Mon, 15 Apr 2024 13:51:00 +0200 Subject: [PATCH] PS-9155: Crash in row_sel_convert_mysql_key_to_innobase https://perconadev.atlassian.net/browse/PS-9155 Problem: Server crashes during execution of the complicated query with 9 CTEs. Cause: For CTE queries, the optimizer can request the creation of a temp table for a particular CTE. If such a temp result is used in table joins, indexes are added to such a table, based on join columns. All possible indexes are added at the 1st stage and then the optimizer decides which of them will be used, removing unused ones. If the CTE temp result is used in joins in other CTEs of the same query, the process of finding indexes is repeated several times. Indexes are created in TABLE::add_tmp_key() as , where ARRAY_INDEX is the index in TABLE_SHARE::key_names array. Let's say , , are created. Then the query optimizer decides that only will be used, so and are removed. is shifted to the 1st unused position (TABLE::move_tmp_key()), so TABLE_SHARE::key_names contains on position 0. Then the query optimizer makes a plan for another join, repeating the above steps. It adds new keys to TABLE_SHARE::key_names and it results with , , . So we've got two keys, having different definitions. Then the temp table is created in the InnoDB world. Then the query is executed. We get to ha_innobase::open() which calls dict_table_get_index_on_name() requesting (the 2nd one). But the function returns the 1st index. innobase_match_index_columns() is called to check the consistency between MySql and InnoDB index definition and here we get the message: [ERROR] [MY-010880] [InnoDB] Found index whose column info does not match that of MySQL. [ERROR] [MY-010882] [InnoDB] Build InnoDB index translation table for Table /var/lib/mysql/tmp/#sql3dad_13_4 failed Then we get to ha_innobase::index_read() where row_sel_convert_mysql_key_to_innobase() is called. The function doesn't do many checks, just converts the key. As the definition of MySql key is longer (e.g. 8 + 8 bytes) than InnoDB's (e.g. 8 + 6), we try to read too much, causing a segmentation fault. The problem is in the way of naming auto keys by the optimizer. The current algorithm does not guarantee the uniqueness of key names, causing name clashes in InnoDB world. Solution: Introduce a variable that will serve the unique number for auto_key names causing index names to be unique. MTR tests re-recorded, because now key ids are generated starting from zero, even if the 1st generated key is on position 1. Note that there can be on position added before. --- mysql-test/r/derived.result | 2 +- mysql-test/r/derived_condition_pushdown.result | 6 +++--- mysql-test/r/view.result | 2 +- sql/table.cc | 2 +- sql/table.h | 1 + 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/derived.result b/mysql-test/r/derived.result index bf12ff47bf50..5455f60f3a88 100644 --- a/mysql-test/r/derived.result +++ b/mysql-test/r/derived.result @@ -2304,7 +2304,7 @@ explain SELECT * FROM t1 JOIN (SELECT * FROM t2 UNION SELECT * FROM t2) AS dt ON t1.a=dt.a; id select_type table partitions type possible_keys key key_len ref rows filtered Extra 1 PRIMARY t1 NULL ALL NULL NULL NULL NULL 2 100.00 Using where -1 PRIMARY NULL ref 5 test.t1.a 2 100.00 NULL +1 PRIMARY NULL ref 5 test.t1.a 2 100.00 NULL 2 DERIVED t2 NULL ALL NULL NULL NULL NULL 2 100.00 NULL 3 UNION t2 NULL ALL NULL NULL NULL NULL 2 100.00 NULL 4 UNION RESULT NULL ALL NULL NULL NULL NULL NULL NULL Using temporary diff --git a/mysql-test/r/derived_condition_pushdown.result b/mysql-test/r/derived_condition_pushdown.result index fe378798e4d2..75ff28481a6e 100644 --- a/mysql-test/r/derived_condition_pushdown.result +++ b/mysql-test/r/derived_condition_pushdown.result @@ -1882,7 +1882,7 @@ EXPLAIN -> Nested loop inner join (rows=8) -> Filter: ((t1.f2 > 40) and (t1.f2 is not null)) (rows=4) -> Table scan on t1 (rows=12) - -> Index lookup on dt using (f2=t1.f2) (rows=2) + -> Index lookup on dt using (f2=t1.f2) (rows=2) -> Union materialize with deduplication (rows=2) -> Filter: (t1.f1 > 31) (rows=1) -> Covering index range scan on t1 using f1_2 over (31 < f1) (rows=1) @@ -1996,7 +1996,7 @@ EXPLAIN -> Nested loop inner join (rows=104) -> Filter: (t2.f2 is not null) (rows=13) -> Table scan on t2 (rows=13) - -> Index lookup on v using (f2=t2.f2) (rows=2) + -> Index lookup on v using (f2=t2.f2) (rows=2) -> Union materialize with deduplication (rows=8) -> Filter: (t1.f2 > 10) (rows=4) -> Table scan on t1 (rows=12) @@ -2025,7 +2025,7 @@ EXPLAIN -> Nested loop inner join (rows=104) -> Filter: (t2.f2 is not null) (rows=13) -> Table scan on t2 (rows=13) - -> Index lookup on v using (f2=t2.f2) (rows=2) + -> Index lookup on v using (f2=t2.f2) (rows=2) -> Union materialize with deduplication (rows=8) -> Filter: (t1.f2 > 10) (rows=4) -> Table scan on t1 (rows=12) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 94e0cf36447b..eae5f580a358 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -4465,7 +4465,7 @@ explain SELECT * FROM t1 JOIN vu_1 AS dt ON t1.a=dt.a; id select_type table partitions type possible_keys key key_len ref rows filtered Extra 1 PRIMARY t1 NULL ALL NULL NULL NULL NULL # # Using where -1 PRIMARY NULL ref 5 test.t1.a # # NULL +1 PRIMARY NULL ref 5 test.t1.a # # NULL 2 DERIVED t2 NULL ALL NULL NULL NULL NULL # # NULL 3 UNION t2 NULL ALL NULL NULL NULL NULL # # NULL 4 UNION RESULT NULL ALL NULL NULL NULL NULL # # Using temporary diff --git a/sql/table.cc b/sql/table.cc index d2f89befde17..23141fe6d62c 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -6029,7 +6029,7 @@ bool TABLE::add_tmp_key(Field_map *key_parts, bool invisible, s->max_key_length = std::max(s->max_key_length, key_len); s->key_parts += key_part_count; assert(s->keys < s->max_tmp_keys); - sprintf(s->key_names[s->keys].name, "", s->keys); + sprintf(s->key_names[s->keys].name, "", s->temp_table_key_id++); s->keys++; } diff --git a/sql/table.h b/sql/table.h index 951e25949c1e..1c09aef7bc59 100644 --- a/sql/table.h +++ b/sql/table.h @@ -842,6 +842,7 @@ struct TABLE_SHARE { uint fields{0}; /* Number of fields */ uint rec_buff_length{0}; /* Size of table->record[] buffer */ uint keys{0}; /* Number of keys defined for the table*/ + uint temp_table_key_id{0}; /* Serves the unique number for */ uint key_parts{0}; /* Number of key parts of all keys defined for the table */