PS-9155: Crash in row_sel_convert_mysql_key_to_innobase #5282
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <auto_keyARRAY_INDEX>, where ARRAY_INDEX is the index in TABLE_SHARE::key_names array. Let's say <auto_key0>, <auto_key1>, <auto_key2> are created. Then the query optimizer decides that only <auto_key2> will be used, so <auto_key0> and <auto_key1> are removed. <auto_key2> is shifted to the 1st unused position (TABLE::move_tmp_key()), so TABLE_SHARE::key_names contains <auto_key2> 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 <auto_key2>, <auto_key1>, <auto_key2>. So we've got two <auto_key2> 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 <auto_key2> (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 <auto_key2> 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 <auto_distinct_key> on position added before.
kamil-holubicki
requested review from
percona-ysorokin,
satya-bodapati and
dlenev
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -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 <auto_keyN> */ | |||
There was a problem hiding this comment.
member variable temp_table_key_id has public visibility
There was a problem hiding this comment.
It needs to be public. It follows the convention of this struct. The other way would be implementing an access method, but would not follow convention.
satya-bodapati
approved these changes
Apr 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 <auto_keyARRAY_INDEX>, where ARRAY_INDEX is the index in TABLE_SHARE::key_names array. Let's say <auto_key0>, <auto_key1>, <auto_key2> are created. Then the query optimizer decides that only <auto_key2> will be used, so <auto_key0> and <auto_key1> are removed. <auto_key2> is shifted to the 1st unused position (TABLE::move_tmp_key()),
so TABLE_SHARE::key_names contains <auto_key2> 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 <auto_key2>, <auto_key1>, <auto_key2>. So we've got two <auto_key2> 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 <auto_key2> (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 <auto_key2> 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 <auto_distinct_key> on position added before.