-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
MDEV-36484 Atomic DDL: Assertion `!param->got_error' failed upon unsu… #3991
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
Open
Olernov
wants to merge
17
commits into
bb-11.8-MDEV-34870-join-order
Choose a base branch
from
bb-12.0-MDEV-36484-rename
base: bb-11.8-MDEV-34870-join-order
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or 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
Implementing a recursive descent parser for optimizer hints.
This commit introduces: - the infrastructure for optimizer hints; - hints for join buffering: BNL(), NO_BNL(), BKA(), NO_BKA(); - NO_ICP() hint for disabling index condition pushdown; - MRR(), MO_MRR() hint for multi-range reads control; - NO_RANGE_OPTIMIZATION() for disabling range optimization; - QB_NAME() for assigning names for query blocks.
- Using Lex_ident_sys to scan identifiers, like the SQL parser does. This fixes handling of double-quote-delimited and backtick-delimited identifiers, as well as handling of non-ASCII identifiers. Unescaping and converting from the client character set to the system character set is now done using Lex_ident_cli_st and Lex_ident_sys, like it's done in the SQL tokenizer/parser. Adding helper methods to_ident_cli() and to_ident_sys() in Optimizer_hint_parser::Token. - Fixing the hint parser to report a syntax error when an empty identifiers: SELECT /*+ BKA(``) */ * FROM t1; - Moving a part of the code from opt_hints_parser.h to opt_hints_parser.cc Moving these method definitions: - Optimizer_hint_tokenizer::find_keyword() - Optimizer_hint_tokenizer::get_token() to avoid huge pieces of the code in the header file. - A Lex_ident_cli_st cleanup Fixing a few Lex_ident_cli_st methods to return Lex_ident_cli_st & instead of void, to use them easier in the caller code. - Fixing the hint parser to display the correct line number Adding a new data type Lex_comment_st (a combination of LEX_CSTRING and a line number) Using it in sql_yacc.yy - Getting rid of redundant dependencies on sql_hints_parser.h Moving void LEX::resolve_optimizer_hints() from sql_lex.h to sql_lex.cc Adding a class Optimizer_hint_parser_output, deriving from Optimizer_hint_parser::Hint_list. Fixing the hint parser to return a pointer to an allocated instance of Optimizer_hint_parser_output rather than an instance of Optimizer_hint_parser::Hint_list. This allows to use a forward declaration of Optimizer_hint_parser_output in sql_lex.h and thus avoid dependencies on sql_hints_parser.h.
Forbid adding optimizer hints to view definitions. In the case when optimizer hints are added to the view definition at a `CREATE (OR REPLACE) VIEW`/`ALTER VIEW` statement, a warning is generated and the hints are ignored. This commit also disables ps-protocol for test cases where `Unresolved table/index name` warnings are generated. The reason for this is such warnings are generated during both PREPARE and EXECUTE stages. Since opt_hints.test has `--enable_prepare_warnings`, running it with `--ps-protocol` causes duplication of warning messages
join_cache_level=0 disables join cache buffers, but the hint BNL() now allows to employ BNL(H) buffers for particular tables or query blocks. This commit also adds a number of test cases including OUTER JOINs to make sure hints do not break the rules of join buffers application
BNL() hint effectively increases join_cache_level up to 4 if it is set to value less than 4. This commit also makes the BKA() hint override not only `join_cache_bka` optimizer switch but `join_cache_level` as well. I.e., BKA() hint enables BKA and BKAH join buffers both flat and incremental despite `join_cache_level` and `join_cache_bka` setting.
It places a limit N (a timeout value in milliseconds) on how long a statement is permitted to execute before the server terminates it. Syntax: SELECT /*+ MAX_EXECUTION_TIME(milliseconds) */ ... Only top-level SELECT statements support the hint.
Since BNL() hint does not specify which whether hashed or non-hashed join cache should be employed, allow usage of hashed ones where possible
… max_statement_time
- add a comment that opt_hints_global->check_unresolved() is never called - improve comments - rename everything with "resolved_children" to "fully_resolved_children" - Opt_hints_table::adjust_key_hints() now returns value - less "reach-back-to-parent" logic - rename Hint "adjustment" and "resolution" (yes, both terms were used) to "fixing". "Resolution" is already used for parse-tree objects
1. Disable opt_hint_timeout.test for embedded server. Make sure the test does not crash even when started for embedded server. Disable view-protocol since hints are not supported inside views. 2. Hints are designed to behave like regular /* ... */ comments: `SELECT /*+ " */ 1;` -- a valid SQL query However, the mysql client program waits for the closing doublequote character. Another problem is observed when there is no space character between closing `*/` of the hint and the following `*`: `SELECT /*+ some_hints(...) */* FROM t1;` In this case the client treats `/*` as a comment section opening and waits for the closing `*/` sequence. This commit fixes all of these issues
- remove get_args_printer() from hints printing - add append_hint_arguments(THD *thd, opt_hints_enum hint, String *str) - add more comments - rename st_opt_hint_info::hint_name to hint_type - add pptimizer trace support for hints - add dbug_print_hints() - make print_warn() not be a template - introduce Printable_parser_rule interface, make grammar rules that emit warnings implement it and print_warn invokes its function) - remove Parser::Hint::append_args() as it is not used anywhere (it used to be necessary call print_warn(... (Parser::Hint*)NULL);
This commit implements optimizer hints allowing to affect the order of joining tables: - JOIN_FIXED_ORDER similar to existing STRAIGHT_JOIN hint; - JOIN_ORDER to apply the specified table order; - JOIN_PREFIX to hint what tables should be first in the join; - JOIN_SUFFIX to hint what tables should be last in the join.
…ccessful multi-RENAME TABLE When executing an atomic sequence of RENAME operations, such as: RENAME TABLE t1 TO t2, t3 TO t4, ... any failure in the sequence triggers a rollback of previously completed renames to preserve atomicity. However, when an error occurs, `my_error()` is invoked immediately, which sets the `thd->is_error()` flag. This premature flag setting causes the rollback logic to misinterpret the thread state, leading to incorrect reversion behavior and assertion failures. To address this, the error is now captured and deferred — stored during the failure and only passed to `my_error()` after the DDL reversion is complete. This ensures the error state is not prematurely set, allowing the rollback logic to execute safely and correctly.
f9311a9
to
42af3f2
Compare
e09e95d
to
8041255
Compare
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.
…ccessful multi-RENAME TABLE
Description
When executing an atomic sequence of RENAME operations, such as: RENAME TABLE t1 TO t2, t3 TO t4, ...
any failure in the sequence triggers a rollback of previously completed renames to preserve atomicity.
However, when an error occurs,
my_error()
is invoked immediately, which sets thethd->is_error()
flag. This premature flag setting causes the rollback logic to misinterpret the thread state, leading to incorrect reversion behavior and assertion failures.To address this, the error is now captured and deferred — stored during the failure and only passed to
my_error()
after the DDL reversion is complete. This ensures the error state is not prematurely set, allowing the rollback logic to execute safely and correctly.How can this PR be tested?
./mtr main.rename
If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.
Basing the PR against the correct MariaDB version
main
branch.PR quality check