Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(trunk) PS-9155: Crash in row_sel_convert_mysql_key_to_innobase #5284

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

kamil-holubicki
Copy link
Contributor

No description provided.

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.
https://perconadev.atlassian.net/browse/PS-9155

Merge branch 'PS-9155-8.0' into PS-9155-trunk

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.
Copy link
Contributor

@satya-bodapati satya-bodapati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kamil-holubicki kamil-holubicki merged commit fe85b4f into percona:trunk Apr 16, 2024
7 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants