Skip to content

Commit e2cb8ac

Browse files
committed
Bug#36758378: Signal 11 in Table_ref::is_external
This solution is for MySQL 8.0. The problem may occur when Sql_cmd_dml::execute() is called, and we have certain error situations, and we check whether the query involves an external table. The function has_external_table() traverses the tables of the query and checks each whether it is "external". But this involves inspecting the TABLE object, and we may hit a null pointer since the TABLE pointer inside the Table_ref object is set to NULL when the table is closed. This patch makes checking for external tables foolproof: Instead of traversing all tables whenever we need to know the value of this property, the value of the property is stored in the LEX object while opening the tables. We know that the information is valid at that point in time, and we know the information is valid as long as the query (and the LEX object) exists. The test case is from bug#36725524. Change-Id: Ie45672def3ea2297907d742c19f0f53c547fe9e3
1 parent 148ebfb commit e2cb8ac

File tree

6 files changed

+16
-22
lines changed

6 files changed

+16
-22
lines changed

sql/sql_base.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6620,6 +6620,9 @@ static bool open_secondary_engine_tables(THD *thd, uint flags) {
66206620
// The previous execution context should have been destroyed.
66216621
assert(lex->secondary_engine_execution_context() == nullptr);
66226622

6623+
// Property of having external tables is always set in this function:
6624+
assert(!lex->has_external_tables());
6625+
66236626
// If use of secondary engines has been disabled for the statement,
66246627
// there is nothing to do.
66256628
if (sql_cmd == nullptr || sql_cmd->secondary_storage_engine_disabled())
@@ -6700,6 +6703,9 @@ static bool open_secondary_engine_tables(THD *thd, uint flags) {
67006703
}
67016704
assert(tl->table->s->is_secondary_engine());
67026705
tl->table->file->ha_set_primary_handler(primary_table->file);
6706+
if (tl->is_external()) {
6707+
lex->set_has_external_tables();
6708+
}
67036709
}
67046710

67056711
// Prepare the secondary engine for executing the statement.

sql/sql_lex.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ void LEX::reset() {
496496

497497
grant_if_exists = false;
498498
ignore_unknown_user = false;
499+
m_has_external_tables = false;
499500
reset_rewrite_required();
500501
}
501502

sql/sql_lex.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4008,12 +4008,16 @@ struct LEX : public Query_tables_list {
40084008
/// True if statement references UDF functions
40094009
bool m_has_udf{false};
40104010
bool ignore;
4011+
/// True if query has at least one external table
4012+
bool m_has_external_tables{false};
40114013

40124014
public:
40134015
bool is_ignore() const { return ignore; }
40144016
void set_ignore(bool ignore_param) { ignore = ignore_param; }
40154017
void set_has_udf() { m_has_udf = true; }
40164018
bool has_udf() const { return m_has_udf; }
4019+
void set_has_external_tables() { m_has_external_tables = true; }
4020+
bool has_external_tables() const { return m_has_external_tables; }
40174021
st_parsing_options parsing_options;
40184022
Alter_info *alter_info;
40194023
/* Prepared statements SQL syntax:*/

sql/sql_prepare.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3118,7 +3118,7 @@ bool Prepared_statement::execute_loop(THD *thd, String *expanded_query,
31183118
thd->secondary_engine_optimization() ==
31193119
Secondary_engine_optimization::SECONDARY &&
31203120
!m_lex->unit->is_executed()) {
3121-
if (has_external_table(m_lex->query_tables)) {
3121+
if (m_lex->has_external_tables()) {
31223122
set_external_engine_fail_reason(m_lex,
31233123
thd->get_stmt_da()->message_text());
31243124
}

sql/sql_select.cc

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -660,15 +660,6 @@ bool Sql_cmd_select::prepare_inner(THD *thd) {
660660
return false;
661661
}
662662

663-
bool has_external_table(Table_ref *query_tables) {
664-
for (Table_ref *ref = query_tables; ref != nullptr; ref = ref->next_global) {
665-
if (ref->is_external()) {
666-
return true;
667-
}
668-
}
669-
return false;
670-
}
671-
672663
bool Sql_cmd_dml::execute(THD *thd) {
673664
DBUG_TRACE;
674665

@@ -741,7 +732,7 @@ bool Sql_cmd_dml::execute(THD *thd) {
741732
}
742733

743734
if (lex->thd->variables.use_secondary_engine == SECONDARY_ENGINE_OFF) {
744-
if (has_external_table(lex->query_tables)) {
735+
if (lex->has_external_tables()) {
745736
my_error(ER_SECONDARY_ENGINE_PLUGIN, MYF(0),
746737
"Query could not be offloaded to the secondary engine");
747738
external_table_not_offloaded = true;
@@ -751,7 +742,7 @@ bool Sql_cmd_dml::execute(THD *thd) {
751742
Secondary_engine_optimization::PRIMARY_ONLY &&
752743
lex->thd->variables.use_secondary_engine !=
753744
SECONDARY_ENGINE_FORCED) &&
754-
has_external_table(lex->query_tables)) {
745+
lex->has_external_tables()) {
755746
// throw the propagated error from the external engine in case there is an
756747
// external table
757748
external_engine_fail_reason(lex);
@@ -848,7 +839,7 @@ bool Sql_cmd_dml::execute(THD *thd) {
848839
assert(thd->get_stmt_da() != nullptr);
849840
// here we check if there is any table in an external engine to set the
850841
// error there as well.
851-
if (has_external_table(lex->query_tables)) {
842+
if (lex->has_external_tables()) {
852843
set_external_engine_fail_reason(lex,
853844
thd->get_stmt_da()->message_text());
854845
}
@@ -943,7 +934,7 @@ static bool retry_with_secondary_engine(THD *thd) {
943934
// is higher than the specified cost threshold.
944935
// We allow any query to be executed in the secondary_engine when it involves
945936
// external tables.
946-
if (!has_external_table(thd->lex->query_tables) &&
937+
if (!thd->lex->has_external_tables() &&
947938
(thd->m_current_query_cost <=
948939
thd->variables.secondary_engine_cost_threshold)) {
949940
Opt_trace_context *const trace = &thd->opt_trace;

sql/sql_select.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,14 +1059,6 @@ void accumulate_statement_cost(const LEX *lex);
10591059
*/
10601060
const handlerton *get_secondary_engine_handlerton(const LEX *lex);
10611061

1062-
/**
1063-
Checks if any of the tables referenced belong to an external engine.
1064-
If an external table is found, return true, false otherwise.
1065-
1066-
@param query_tables the referenced tables.
1067-
*/
1068-
bool has_external_table(Table_ref *query_tables);
1069-
10701062
/**
10711063
Sets the reason of failure for the statement to the external engine.
10721064

0 commit comments

Comments
 (0)