Skip to content

MDEV-31918: Optimize check on sequences earlier for alter - #2729

Open
an3l wants to merge 1 commit into
MariaDB:11.2from
an3l:bb-11.2-anel-fix-early-check-sequence
Open

MDEV-31918: Optimize check on sequences earlier for alter#2729
an3l wants to merge 1 commit into
MariaDB:11.2from
an3l:bb-11.2-anel-fix-early-check-sequence

Conversation

@an3l

@an3l an3l commented Aug 14, 2023

Copy link
Copy Markdown
Contributor
  • The Jira issue number for this PR is: MDEV-______

Description

TODO: fill description here

How can this PR be tested?

Run the test case and check in gdb backtrace.

Result bt before the patch backtrace

#0  my_message_sql (error=21845, str=0x7ffff1f2e570 "\240\345\362\361\377\177", MyFlags=93825015206591) at /home/anel/GitHub/mariadb/server/src/10.5/sql/mysqld.cc:3094
#1  0x0000555556b2f5a6 in my_error (nr=4086, MyFlags=0) at /home/anel/GitHub/mariadb/server/src/10.5/mysys/my_error.c:124
#2  0x000055555612f898 in check_sequence_fields (lex=0x7fffe0004e30, fields=0x7ffff1f31270) at /home/anel/GitHub/mariadb/server/src/10.5/sql/sql_sequence.cc:237
#3  0x0000555555f92b67 in mysql_prepare_create_table (thd=0x7fffe0000d48, create_info=0x7ffff1f312e0, alter_info=0x7ffff1f311f0, db_options=0x7ffff1f2ec4c, file=0x7fffe0017760, key_info_buffer=0x7ffff1f2f5f8, key_count=0x7ffff1f2f5d8, create_table_mode=-2) at /home/anel/GitHub/mariadb/server/src/10.5/sql/sql_table.cc:3621
#4  0x0000555555f979d9 in mysql_create_frm_image (thd=0x7fffe0000d48, create_info=0x7ffff1f312e0, alter_info=0x7ffff1f311f0, create_table_mode=-2, key_info=0x7ffff1f2f5f8, key_count=0x7ffff1f2f5d8, frm=0x7ffff1f2f670) at /home/anel/GitHub/mariadb/server/src/10.5/sql/sql_table.cc:5069
#5  0x0000555555f98600 in create_table_impl (thd=0x7fffe0000d48, orig_db=@0x7ffff1f306b0: {str = 0x7fffe0016d10 "test", length = 4}, orig_table_name=@0x7ffff1f306c0: {str = 0x7fffe00165d0 "seq1", length = 4}, db=@0x7ffff1f306e0: {str = 0x7fffe0016d10 "test", length = 4}, table_name=@0x7ffff1f30710: {str = 0x7ffff1f30a4b "#sql-alter-9696-3", length = 17}, path=0x7ffff1f30f0e "/home/anel/GitHub/mariadb/server/build/10.5/mysql-test/var/tmp/mysqld.1/#sql-temptable-9696-3-1", options={m_options = DDL_options_st::OPT_NONE}, create_info=0x7ffff1f312e0, alter_info=0x7ffff1f311f0, create_table_mode=-2, is_trans=0x0, key_info=0x7ffff1f2f5f8, key_count=0x7ffff1f2f5d8, frm=0x7ffff1f2f670) at /home/anel/GitHub/mariadb/server/src/10.5/sql/sql_table.cc:5349
#6  0x0000555555fa8b43 in mysql_alter_table (thd=0x7fffe0000d48, new_db=0x7fffe00057d8, new_name=0x7fffe0005c28, create_info=0x7ffff1f312e0, table_list=0x7fffe0016610, recreate_info=0x7ffff1f311b0, alter_info=0x7ffff1f311f0, order_num=0, order=0x0, ignore=false, if_exists=false) at /home/anel/GitHub/mariadb/server/src/10.5/sql/sql_table.cc:10753

Result bt after the patch (both for temporary and normal sequences):

#0  my_message_sql (error=4086, str=0x7ffff20194a0 "Sequence 'test.seq1' table structure is invalid (cache_size)", MyFlags=0) at /home/anel/GitHub/mariadb/server/src/11.2/sql/mysqld.cc:3372
#1  0x0000555556895573 in my_error (nr=nr@entry=4086, MyFlags=MyFlags@entry=0) at /home/anel/GitHub/mariadb/server/src/11.2/mysys/my_error.c:124
#2  0x0000555556001ba8 in check_sequence_fields (lex=0x7fffe0005180, fields=fields@entry=0x7ffff20198c0) at /home/anel/GitHub/mariadb/server/src/11.2/sql/sql_lex.h:3212
#3  0x0000555555e69772 in mysql_prepare_alter_table (thd=thd@entry=0x7fffe0000d48, table=table@entry=0x7fffe001e0e8, create_info=create_info@entry=0x7ffff201c270, alter_info=alter_info@entry=0x7ffff201c180, alter_ctx=alter_ctx@entry=0x7ffff201b460) at /home/anel/GitHub/mariadb/server/src/11.2/sql/sql_table.cc:8525
#4  0x0000555555e79330 in mysql_alter_table (thd=thd@entry=0x7fffe0000d48, new_db=new_db@entry=0x7fffe0005b38, new_name=new_name@entry=0x7fffe0005f88, create_info=create_info@entry=0x7ffff201c270, table_list=, table_list@entry=0x7fffe0016d40, recreate_info=recreate_info@entry=0x7ffff201c140, alter_info=0x7ffff201c180, order_num=0, order=0x0, ignore=false, if_exists=false) at /home/anel/GitHub/mariadb/server/src/11.2/sql/sql_table.cc:10489
#5  0x0000555555f101c9 in Sq

Basing the PR against the correct MariaDB version

  • [x ] 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 enhancement MariaDB Foundation Pull requests created by MariaDB Foundation labels Aug 14, 2023
@an3l an3l added this to the 11.2 milestone Aug 14, 2023
@an3l
an3l force-pushed the bb-11.2-anel-fix-early-check-sequence branch 2 times, most recently from 4f4b929 to 5fce99d Compare August 17, 2023 14:53
@an3l
an3l force-pushed the bb-11.2-anel-fix-early-check-sequence branch 3 times, most recently from a6db639 to 519413d Compare March 13, 2024 08:34
- When `alter` table is perfomed there is no reason to go to function
  `create_table_impl->mysql_create_frm_image->mysql_prepare_create_table`,
  (as code is currently doing, and what is happening for the `create` statement),
  but instead do check on the directly on created altered fields from function
  `mysql_prepare_alter_table`, that is called earlier than
  `create_table_impl`, from `mysql_alter_table`.

- In that way benefit would be to do error check for the `alter` query only,
  on `mysql_prepare_alter_table` and to generate error earlier in the stack frame.
  The check will be done only once, and second check we are bypassing
  with newly created `alter_info->seq_checked` bit, so there will be no
  double calls of function `check_sequence_fields`.

- For the `create` table statements error check will be done only once,
  from function `mysql_prepare_create_table` as it is intended and the
  patch is not related to `create` statement.

- Reviewer: <>
@an3l
an3l force-pushed the bb-11.2-anel-fix-early-check-sequence branch from 519413d to d6ede79 Compare March 14, 2024 11:09
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

Development

Successfully merging this pull request may close these issues.

3 participants