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

MDEV-31618: Server crashes in process_i_s_table_temporary_tables/get_all_tables #2685

Merged

Conversation

an3l
Copy link
Contributor

@an3l an3l commented Jul 4, 2023

[x] The Jira issue number for this PR is: MDEV-31618

Basing the PR against the correct MariaDB version

  • This is a new feature and the PR is based against the latest MariaDB development branch.
  • This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@an3l an3l added the MariaDB Foundation Pull requests created by MariaDB Foundation label Jul 4, 2023
@an3l an3l added this to the 11.2 milestone Jul 4, 2023
@an3l an3l requested a review from vuvova July 4, 2023 15:28
SELECT table_schema, table_name FROM INFORMATION_SCHEMA.TABLES WHERE table_type='temporary sequence';
table_schema table_name
test seq1
mysqltest s2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vuvova not sure how this happened, when mysqltest is removed before?
tmp_mem_root maybe?

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I don't understand. what do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

The temporary sequences are shown because they where created in previous tests that did not clean up itself properly.
Fix by moving:
DROP TABLE mysqltest.s1;
DROP TABLE mysqltest.s2;
before the line
#MDEV-31618: Server crashes in

sql/sql_show.cc Outdated
@@ -5310,6 +5310,8 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond)
}

TABLE *tmp_tbl= share_temp->all_tmp_tables.front();
if(!tmp_tbl)
continue;
Copy link
Member

Choose a reason for hiding this comment

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

No, this is completely wrong. There cannot be a temporary table share without a single temporary table.

If this is the case here — it's a bug.

an3l added a commit to an3l/server that referenced this pull request Jul 19, 2023
…all_tables

- Mark temporary sequence as not needed to be reopen in case of
renaming during alter prepare phase, since that operation will fail.
- Without this patch, `find_temporary_table(0)` deletes the table from
share
- Closes PR MariaDB#2685
- Reviewer: <>
@an3l an3l force-pushed the preview-11.2-preview-anel-MDEV-31618 branch from b47a95d to 39f3418 Compare July 19, 2023 07:21
an3l added a commit to an3l/server that referenced this pull request Jul 19, 2023
…all_tables

- Pre-open temporary table on sequence creation.
- Without this patch, if rename alter is done on the temporary sequence,
  and after that `create replace`, since table is not preopened and
  alter rename marked the table as reopen, and such table is deleted in
  the `find_temporary_table()` leaving the share without the table, that
  causes `show tables` to fail
- Closes PR MariaDB#2685
- Reviewer: <serg@mariadb.com>
@an3l an3l force-pushed the preview-11.2-preview-anel-MDEV-31618 branch from 39f3418 to e334197 Compare July 19, 2023 12:01
@vuvova
Copy link
Member

vuvova commented Jul 20, 2023

please, try to construct a test case that fails in earlier versions

…all_tables

- Pre-open temporary table on sequence creation.
- Without this patch, if rename alter is done on the temporary sequence,
  and after that `create replace`, since table is not preopened and
  alter rename marked the table as reopen, and such table is deleted in
  the `find_temporary_table()` leaving the share without the table, that
  causes `show tables` to fail
- Closes PR MariaDB#2685
- Reviewer: <serg@mariadb.com>
@an3l an3l force-pushed the preview-11.2-preview-anel-MDEV-31618 branch from e334197 to d4fe932 Compare August 2, 2023 14:38
…isammrg::info

- Fix MSAN uninitialized value error, obtained by calling handlerton's
`info()`, and referencing uninitialized `errkey`.
- Reviewer: serg@mariadb.com
@an3l an3l force-pushed the preview-11.2-preview-anel-MDEV-31618 branch from c204487 to c7edf8b Compare August 2, 2023 17:02
@an3l an3l merged commit 1a7f3a9 into MariaDB:preview-11.2-preview Aug 2, 2023
12 of 13 checks passed
an3l added a commit that referenced this pull request Aug 2, 2023
…all_tables

- Pre-open temporary table on sequence creation.
- Without this patch, if rename alter is done on the temporary sequence,
  and after that `create replace`, since table is not preopened and
  alter rename marked the table as reopen, and such table is deleted in
  the `find_temporary_table()` leaving the share without the table, that
  causes `show tables` to fail
- Closes PR #2685
- Reviewer: <serg@mariadb.com>
vuvova pushed a commit that referenced this pull request Aug 11, 2023
…all_tables

- Pre-open temporary table on sequence creation.
- Without this patch, if rename alter is done on the temporary sequence,
  and after that `create replace`, since table is not preopened and
  alter rename marked the table as reopen, and such table is deleted in
  the `find_temporary_table()` leaving the share without the table, that
  causes `show tables` to fail
- Closes PR #2685
- Reviewer: <serg@mariadb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MariaDB Foundation Pull requests created by MariaDB Foundation
3 participants