Skip to content

Commit d7255a3

Browse files
roylysengroylyseng
committed
Bug#34929930: Sub-statement of a stored procedure support in secondary engine.
This patch fixes a problem where statements inside stored procedures are never offloaded to a secondary engine. There are two major components in the patch: 1. Function THD::is_secondary_storage_engine_eligible() is changed so that statements within stored procedures are allowed. 2. Enable class sp_lex_instr to manage preparation and optimization against a secondary engine. The support is mostly the same as for class Prepared_statement, but since these classes are not related, a separate implementation is required: - sp_lex_instr::reset_lex_and_exec_core() must recognize the error codes ER_PREPARE_FOR_PRIMARY_ENGINE and ER_PREPARE_FOR_SECONDARY_ENGINE, in order to switch optimization for secondary and primary engine. - sp_lex_instr::validate_lex_and_execute_core() implements a loop that performs repeated parsing, resolving and optimization of a statement, before finally executing it (unless there was an earlier error). This loop is enhanced to detect that resolving and optimization in a secondary engine is required, as well as detecting that resolving and optimization in the primary engine is required, if statement cannot be executed in secondary engine, or cost of using primary is decided to be the most efficient. Notice also that the call to open_and_lock_tables() in sp_lex_instr::execute_expression() has been replaced with calls to open_tables_for_query() and lock_tables(). This is because only open_tables_for_query() will perform explicit opening of secondary engine tables, thus it is required for proper validation of SQL expressions within stored routines. However, since SET_VAR hints are not evaluated for SQL expressions currently, such expressions will be executed in the primary engine anyway, regardless of any hint. See legacy bug#35569621. Secondary engine and Rapid tests have been adjusted to allow statements within stored procedures. Change-Id: I75c99622e1009ffe712fd4b8f932b94b16135ed4
1 parent f6e4386 commit d7255a3

14 files changed

+140
-67
lines changed

Diff for: ‎mysql-test/suite/secondary_engine/r/query_preparation.result

+7-10
Original file line numberDiff line numberDiff line change
@@ -379,41 +379,38 @@ FLUSH STATUS;
379379
CREATE PROCEDURE p1() SELECT COUNT(*) FROM t1;
380380
CALL p1();
381381
COUNT(*)
382-
3
382+
0
383383
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
384384
Variable_name Value
385-
Secondary_engine_execution_count 0
385+
Secondary_engine_execution_count 1
386386
PREPARE ps1 FROM 'SELECT * FROM t1';
387387
EXECUTE ps1;
388388
id x y
389389
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
390390
Variable_name Value
391-
Secondary_engine_execution_count 1
391+
Secondary_engine_execution_count 2
392392
CREATE PROCEDURE p2() EXECUTE ps1;
393393
CREATE PROCEDURE p3() PREPARE ps2 FROM 'SELECT * FROM t1';
394394
FLUSH STATUS;
395395
CALL p2();
396396
id x y
397-
1 2 3
398-
4 5 6
399-
7 8 9
400397
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
401398
Variable_name Value
402-
Secondary_engine_execution_count 0
399+
Secondary_engine_execution_count 1
403400
CALL p3();
404401
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
405402
Variable_name Value
406-
Secondary_engine_execution_count 0
403+
Secondary_engine_execution_count 1
407404
EXECUTE ps1;
408405
id x y
409406
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
410407
Variable_name Value
411-
Secondary_engine_execution_count 1
408+
Secondary_engine_execution_count 2
412409
EXECUTE ps2;
413410
id x y
414411
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
415412
Variable_name Value
416-
Secondary_engine_execution_count 2
413+
Secondary_engine_execution_count 3
417414
DROP PREPARE ps1;
418415
DROP PREPARE ps2;
419416
DROP PROCEDURE p1;

Diff for: ‎mysql-test/suite/secondary_engine/r/query_preparation_debug.result

+1-3
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,16 @@ CREATE PROCEDURE p()
2828
SELECT * FROM t;
2929
CALL p();
3030
x
31-
1
3231
SET DEBUG = '+d,secondary_engine_mock_prepare_error';
3332
CALL p();
3433
x
3534
1
3635
SET DEBUG = '-d,secondary_engine_mock_prepare_error';
3736
CALL p();
3837
x
39-
1
4038
SHOW SESSION STATUS LIKE 'Secondary_engine_execution_count';
4139
Variable_name Value
42-
Secondary_engine_execution_count 0
40+
Secondary_engine_execution_count 2
4341
DROP PROCEDURE p;
4442
DROP TABLE t;
4543
#

Diff for: ‎sql/sp_instr.cc

+106-30
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,10 @@ bool sp_lex_instr::execute_expression(THD *thd, uint *nextp) {
356356
return true;
357357
}
358358
}
359-
if (open_and_lock_tables(thd, m_lex->query_tables, 0)) {
359+
if (open_tables_for_query(thd, m_lex->query_tables, 0)) {
360+
return true;
361+
}
362+
if (lock_tables(thd, m_lex->query_tables, m_lex->table_count, 0)) {
360363
return true;
361364
}
362365

@@ -522,7 +525,9 @@ bool sp_lex_instr::reset_lex_and_exec_core(THD *thd, uint *nextp,
522525
*/
523526
const bool reprepare_error =
524527
error && thd->is_error() &&
525-
thd->get_stmt_da()->mysql_errno() == ER_NEED_REPREPARE;
528+
(thd->get_stmt_da()->mysql_errno() == ER_NEED_REPREPARE ||
529+
thd->get_stmt_da()->mysql_errno() == ER_PREPARE_FOR_PRIMARY_ENGINE ||
530+
thd->get_stmt_da()->mysql_errno() == ER_PREPARE_FOR_SECONDARY_ENGINE);
526531

527532
// Unless there is an error, execution must have started (and completed)
528533
assert(error || m_lex->is_exec_started());
@@ -700,6 +705,10 @@ LEX *sp_lex_instr::parse_expr(THD *thd, sp_head *sp) {
700705

701706
bool sp_lex_instr::validate_lex_and_execute_core(THD *thd, uint *nextp,
702707
bool open_tables) {
708+
// Remember if the general log was temporarily disabled when repreparing the
709+
// statement for a secondary engine.
710+
bool general_log_temporarily_disabled = false;
711+
703712
Reprepare_observer reprepare_observer;
704713

705714
thd->set_secondary_engine_optimization(
@@ -710,8 +719,7 @@ bool sp_lex_instr::validate_lex_and_execute_core(THD *thd, uint *nextp,
710719
if (is_invalid() || (m_lex->has_udf() && !m_first_execution)) {
711720
free_lex();
712721
LEX *lex = parse_expr(thd, thd->sp_runtime_ctx->sp);
713-
714-
if (!lex) return true;
722+
if (lex == nullptr) return true;
715723

716724
set_lex(lex, true);
717725

@@ -750,42 +758,110 @@ bool sp_lex_instr::validate_lex_and_execute_core(THD *thd, uint *nextp,
750758

751759
thd->pop_reprepare_observer();
752760

761+
/*
762+
Re-enable the general log if it was temporarily disabled while repreparing
763+
and executing a statement for a secondary engine.
764+
*/
765+
if (general_log_temporarily_disabled) {
766+
thd->variables.option_bits &= ~OPTION_LOG_OFF;
767+
general_log_temporarily_disabled = false;
768+
}
769+
753770
m_first_execution = false;
754771

772+
// Exit immediately if execution is successful
755773
if (!rc) return false;
756774

757-
/*
758-
Here is why we need all the checks below:
759-
- if the reprepare observer is not set, we've got an error, which should
760-
be raised to the user;
761-
- if we've got fatal error, it should be raised to the user;
762-
- if our thread got killed during execution, the error should be raised
763-
to the user;
764-
- if we've got an error, different from ER_NEED_REPREPARE, we need to
765-
raise it to the user;
766-
*/
767-
if (stmt_reprepare_observer == nullptr || thd->is_fatal_error() ||
768-
thd->killed || thd->get_stmt_da()->mysql_errno() != ER_NEED_REPREPARE) {
775+
// Exit if a fatal error has occurred or statement execution was killed.
776+
if (thd->is_fatal_error() || thd->is_killed()) {
777+
// Make sure next execution is started with a clean statement:
778+
if (m_lex->is_metadata_used() && !m_lex->is_exec_started()) {
779+
invalidate();
780+
}
769781
return true;
770782
}
771-
/*
772-
Reprepare_observer ensures that the statement is retried a maximum number
773-
of times, to avoid an endless loop.
774-
*/
775-
assert(stmt_reprepare_observer->is_invalidated());
776-
if (!stmt_reprepare_observer->can_retry()) {
783+
int my_errno = thd->get_stmt_da()->mysql_errno();
784+
785+
if (my_errno != ER_NEED_REPREPARE &&
786+
my_errno != ER_PREPARE_FOR_PRIMARY_ENGINE &&
787+
my_errno != ER_PREPARE_FOR_SECONDARY_ENGINE) {
788+
if (m_lex->m_sql_cmd != nullptr &&
789+
thd->secondary_engine_optimization() ==
790+
Secondary_engine_optimization::SECONDARY &&
791+
!m_lex->unit->is_executed()) {
792+
if (has_external_table(m_lex->query_tables)) {
793+
set_external_engine_fail_reason(m_lex,
794+
thd->get_stmt_da()->message_text());
795+
}
796+
if (!thd->is_secondary_engine_forced()) {
797+
/*
798+
Some error occurred during resolving or optimization in
799+
the secondary engine, and secondary engine execution is not forced.
800+
Retry execution of the statement in the primary engine.
801+
*/
802+
thd->clear_error();
803+
thd->set_secondary_engine_optimization(
804+
Secondary_engine_optimization::PRIMARY_ONLY);
805+
invalidate();
806+
// Disable the general log. The query was written to the general log
807+
// in the first attempt to execute it. No need to write it twice.
808+
if ((thd->variables.option_bits & OPTION_LOG_OFF) == 0) {
809+
thd->variables.option_bits |= OPTION_LOG_OFF;
810+
general_log_temporarily_disabled = true;
811+
}
812+
continue;
813+
}
814+
}
777815
/*
778-
Reprepare_observer sets error status in DA but Sql_condition is not
779-
added. Please check Reprepare_observer::report_error(). Pushing
780-
Sql_condition for ER_NEED_REPREPARE here.
816+
If an error occurred before execution, make sure next execution is
817+
started with a clean statement:
781818
*/
782-
Diagnostics_area *da = thd->get_stmt_da();
783-
da->push_warning(thd, da->mysql_errno(), da->returned_sqlstate(),
784-
Sql_condition::SL_ERROR, da->message_text());
819+
if (m_lex->is_metadata_used() && !m_lex->is_exec_started()) {
820+
invalidate();
821+
}
822+
assert(thd->is_error());
785823
return true;
786824
}
787-
788-
thd->clear_error();
825+
if (my_errno == ER_NEED_REPREPARE) {
826+
/*
827+
Reprepare_observer ensures that the statement is retried
828+
a maximum number of times, to avoid an endless loop.
829+
*/
830+
assert(stmt_reprepare_observer != nullptr &&
831+
stmt_reprepare_observer->is_invalidated());
832+
if (!stmt_reprepare_observer->can_retry()) {
833+
/*
834+
Reprepare_observer sets error status in DA but Sql_condition is not
835+
added. Please check Reprepare_observer::report_error(). Pushing
836+
Sql_condition for ER_NEED_REPREPARE here.
837+
*/
838+
Diagnostics_area *da = thd->get_stmt_da();
839+
da->push_warning(thd, da->mysql_errno(), da->returned_sqlstate(),
840+
Sql_condition::SL_ERROR, da->message_text());
841+
assert(thd->is_error());
842+
return true;
843+
}
844+
thd->clear_error();
845+
} else {
846+
assert(my_errno == ER_PREPARE_FOR_PRIMARY_ENGINE ||
847+
my_errno == ER_PREPARE_FOR_SECONDARY_ENGINE);
848+
assert(thd->secondary_engine_optimization() ==
849+
Secondary_engine_optimization::PRIMARY_TENTATIVELY);
850+
thd->clear_error();
851+
if (my_errno == ER_PREPARE_FOR_SECONDARY_ENGINE) {
852+
thd->set_secondary_engine_optimization(
853+
Secondary_engine_optimization::SECONDARY);
854+
} else {
855+
thd->set_secondary_engine_optimization(
856+
Secondary_engine_optimization::PRIMARY_ONLY);
857+
}
858+
// Disable the general log. The query was written to the general log in
859+
// the first attempt to execute it. No need to write it twice.
860+
if ((thd->variables.option_bits & OPTION_LOG_OFF) == 0) {
861+
thd->variables.option_bits |= OPTION_LOG_OFF;
862+
general_log_temporarily_disabled = true;
863+
}
864+
}
789865
invalidate();
790866
}
791867
}

Diff for: ‎sql/sql_base.cc

+5-7
Original file line numberDiff line numberDiff line change
@@ -6697,11 +6697,10 @@ static bool open_secondary_engine_tables(THD *thd, uint flags) {
66976697
if (thd->secondary_engine_optimization() ==
66986698
Secondary_engine_optimization::PRIMARY_TENTATIVELY &&
66996699
thd->is_secondary_engine_forced()) {
6700-
if ((sql_cmd->sql_command_code() == SQLCOM_SELECT &&
6701-
lex->table_count >= 1) || // 2
6702-
((sql_cmd->sql_command_code() == SQLCOM_INSERT_SELECT ||
6703-
sql_cmd->sql_command_code() == SQLCOM_CREATE_TABLE) &&
6704-
lex->table_count >= 2)) { // 3
6700+
if ((lex->sql_command == SQLCOM_SELECT && lex->table_count >= 1) ||
6701+
((lex->sql_command == SQLCOM_INSERT_SELECT ||
6702+
lex->sql_command == SQLCOM_CREATE_TABLE) &&
6703+
lex->table_count >= 2)) {
67056704
thd->set_secondary_engine_optimization(
67066705
Secondary_engine_optimization::SECONDARY);
67076706
mysql_thread_set_secondary_engine(true);
@@ -6711,7 +6710,6 @@ static bool open_secondary_engine_tables(THD *thd, uint flags) {
67116710
Secondary_engine_optimization::PRIMARY_ONLY);
67126711
}
67136712
}
6714-
67156713
// Only open secondary engine tables if use of a secondary engine
67166714
// has been requested.
67176715
if (thd->secondary_engine_optimization() !=
@@ -6724,7 +6722,7 @@ static bool open_secondary_engine_tables(THD *thd, uint flags) {
67246722
// future executions of the statement, since these properties will
67256723
// not change between executions.
67266724
const LEX_CSTRING *secondary_engine =
6727-
sql_cmd->eligible_secondary_storage_engine();
6725+
sql_cmd->eligible_secondary_storage_engine(thd);
67286726
const plugin_ref secondary_engine_plugin =
67296727
secondary_engine == nullptr
67306728
? nullptr

Diff for: ‎sql/sql_class.cc

-2
Original file line numberDiff line numberDiff line change
@@ -3117,8 +3117,6 @@ bool THD::is_secondary_storage_engine_eligible() const {
31173117
if ((in_multi_stmt_transaction_mode() &&
31183118
lex->sql_command != SQLCOM_CREATE_TABLE))
31193119
return false;
3120-
// It is a sub-statement of a stored procedure
3121-
if (sp_runtime_ctx != nullptr) return false;
31223120
return true;
31233121
}
31243122

Diff for: ‎sql/sql_cmd.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ class Sql_cmd {
181181
the statement is not eligible for execution in a secondary storage
182182
engine
183183
*/
184-
virtual const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine() const {
184+
virtual const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine(
185+
THD *) const {
185186
return nullptr;
186187
}
187188

Diff for: ‎sql/sql_cmd_ddl_table.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ bool Sql_cmd_create_table::execute(THD *thd) {
471471
}
472472

473473
const MYSQL_LEX_CSTRING *
474-
Sql_cmd_create_table::eligible_secondary_storage_engine() const {
474+
Sql_cmd_create_table::eligible_secondary_storage_engine(THD *) const {
475475
// Now check if the opened tables are available in a secondary
476476
// storage engine. Only use the secondary tables if all the tables
477477
// have a secondary tables, and they are all in the same secondary

Diff for: ‎sql/sql_cmd_ddl_table.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ class Sql_cmd_create_table final : public Sql_cmd_ddl_table {
6969
return SQLCOM_CREATE_TABLE;
7070
}
7171

72-
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine() const override;
72+
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine(
73+
THD *thd) const override;
7374

7475
bool execute(THD *thd) override;
7576
bool prepare(THD *thd) override;

Diff for: ‎sql/sql_cmd_dml.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ class Sql_cmd_dml : public Sql_cmd {
212212
213213
@return nullptr if not eligible or the name of the engine otherwise
214214
*/
215-
const MYSQL_LEX_CSTRING *get_eligible_secondary_engine() const;
215+
const MYSQL_LEX_CSTRING *get_eligible_secondary_engine(THD *thd) const;
216216

217217
protected:
218218
LEX *lex; ///< Pointer to LEX for this statement

Diff for: ‎sql/sql_do.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ class Sql_cmd_do final : public Sql_cmd_select {
4040

4141
enum_sql_command sql_command_code() const override { return SQLCOM_DO; }
4242

43-
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine() const override {
43+
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine(
44+
THD *) const override {
4445
return nullptr;
4546
}
4647
};

Diff for: ‎sql/sql_insert.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -3341,12 +3341,12 @@ bool Sql_cmd_insert_base::accept(THD *thd, Select_lex_visitor *visitor) {
33413341
}
33423342

33433343
const MYSQL_LEX_CSTRING *
3344-
Sql_cmd_insert_select::eligible_secondary_storage_engine() const {
3344+
Sql_cmd_insert_select::eligible_secondary_storage_engine(THD *thd) const {
33453345
// ON DUPLICATE KEY UPDATE cannot be offloaded
33463346
if (!update_field_list.empty()) return nullptr;
33473347

33483348
// Don't use secondary storage engines for REPLACE INTO SELECT statements
33493349
if (is_replace) return nullptr;
33503350

3351-
return get_eligible_secondary_engine();
3351+
return get_eligible_secondary_engine(thd);
33523352
}

Diff for: ‎sql/sql_insert.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,8 @@ class Sql_cmd_insert_select : public Sql_cmd_insert_base {
339339
enum_sql_command sql_command_code() const override {
340340
return is_replace ? SQLCOM_REPLACE_SELECT : SQLCOM_INSERT_SELECT;
341341
}
342-
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine() const override;
342+
const MYSQL_LEX_CSTRING *eligible_secondary_storage_engine(
343+
THD *thd) const override;
343344
};
344345

345346
#endif /* SQL_INSERT_INCLUDED */

Diff for: ‎sql/sql_select.cc

+7-6
Original file line numberDiff line numberDiff line change
@@ -601,9 +601,9 @@ bool Sql_cmd_select::accept(THD *thd, Select_lex_visitor *visitor) {
601601
return thd->lex->unit->accept(visitor);
602602
}
603603

604-
const MYSQL_LEX_CSTRING *Sql_cmd_select::eligible_secondary_storage_engine()
605-
const {
606-
return get_eligible_secondary_engine();
604+
const MYSQL_LEX_CSTRING *Sql_cmd_select::eligible_secondary_storage_engine(
605+
THD *thd) const {
606+
return get_eligible_secondary_engine(thd);
607607
}
608608

609609
/**
@@ -923,7 +923,7 @@ static bool retry_with_secondary_engine(THD *thd) {
923923

924924
// Don't retry if there is a property of the statement that prevents use of
925925
// secondary engines.
926-
if (sql_cmd->eligible_secondary_storage_engine() == nullptr) {
926+
if (sql_cmd->eligible_secondary_storage_engine(thd) == nullptr) {
927927
sql_cmd->disable_secondary_storage_engine();
928928
return false;
929929
}
@@ -1231,8 +1231,9 @@ bool Sql_cmd_dml::check_all_table_privileges(THD *thd) {
12311231
return false;
12321232
}
12331233

1234-
const MYSQL_LEX_CSTRING *Sql_cmd_dml::get_eligible_secondary_engine() const {
1235-
return get_eligible_secondary_engine_from(lex);
1234+
const MYSQL_LEX_CSTRING *Sql_cmd_dml::get_eligible_secondary_engine(
1235+
THD *thd) const {
1236+
return get_eligible_secondary_engine_from(thd->lex);
12361237
}
12371238

12381239
/*****************************************************************************

0 commit comments

Comments
 (0)