Skip to content

Commit e4e37d5

Browse files
Stella GiannakopoulouCagri Balkesen
authored andcommitted
WL#15575: Fixes for propagating error to lakehouse [Bug#35282626].
* Propagate the error in MySQL even if we come from secondary engine optimization and the table refers to an external engine. * Mark the table as external in the case where the prepare statement fails too. - Added more test cases to increase coverage of the non-offloaded queries implementation. Change-Id: Ifad0b7cf44a2c77d0fdd7624aab44d7817ae31c1
1 parent 7d98546 commit e4e37d5

File tree

3 files changed

+44
-22
lines changed

3 files changed

+44
-22
lines changed

sql/sql_select.cc

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,9 @@ static void set_external_engine_fail_reason(const LEX *lex,
372372
reason != nullptr) {
373373
for (Table_ref *ref = lex->query_tables; ref != nullptr;
374374
ref = ref->next_global) {
375-
if (ref->table != nullptr && ref->table->file != nullptr &&
376-
(ref->table->file->ht->flags & HTON_SUPPORTS_EXTERNAL_SOURCE) != 0 &&
377-
ref->table->s->has_secondary_engine()) {
378-
ref->table->file->set_external_table_offload_error(reason);
375+
if (ref->is_external()) {
376+
ref->table->get_primary_handler()->set_external_table_offload_error(
377+
reason);
379378
break;
380379
}
381380
}
@@ -385,10 +384,8 @@ static void set_external_engine_fail_reason(const LEX *lex,
385384
static void external_engine_fail_reason(const LEX *lex) {
386385
for (Table_ref *ref = lex->query_tables; ref != nullptr;
387386
ref = ref->next_global) {
388-
if (ref->table != nullptr && ref->table->file != nullptr &&
389-
(ref->table->file->ht->flags & HTON_SUPPORTS_EXTERNAL_SOURCE) != 0 &&
390-
ref->table->s->has_secondary_engine()) {
391-
ref->table->file->external_table_offload_error();
387+
if (ref->is_external()) {
388+
ref->table->get_primary_handler()->external_table_offload_error();
392389
return;
393390
}
394391
}
@@ -665,9 +662,7 @@ bool Sql_cmd_select::prepare_inner(THD *thd) {
665662

666663
static bool has_external_table(Table_ref *query_tables) {
667664
for (Table_ref *ref = query_tables; ref != nullptr; ref = ref->next_global) {
668-
if (ref->table != nullptr && ref->table->file != nullptr &&
669-
(ref->table->file->ht->flags & HTON_SUPPORTS_EXTERNAL_SOURCE) != 0 &&
670-
ref->table->s->has_secondary_engine()) {
665+
if (ref->is_external()) {
671666
return true;
672667
}
673668
}
@@ -684,8 +679,6 @@ bool Sql_cmd_dml::execute(THD *thd) {
684679
bool statement_timer_armed = false;
685680
bool error_handler_active = false;
686681

687-
// flag to denote if the query refers to an external table
688-
bool is_external_table = false;
689682
// flag to determine if execution was not offloaded to the secondary engine
690683
// and ended up in the external engine in which case we throw an error.
691684
bool external_table_not_offloaded = false;
@@ -747,18 +740,18 @@ bool Sql_cmd_dml::execute(THD *thd) {
747740
}
748741
}
749742

750-
is_external_table = has_external_table(lex->query_tables);
751-
if (lex->thd->variables.use_secondary_engine == SECONDARY_ENGINE_OFF &&
752-
is_external_table) {
753-
my_error(ER_SECONDARY_ENGINE_PLUGIN, MYF(0),
754-
"Query could not be offloaded to the secondary engine");
755-
external_table_not_offloaded = true;
756-
goto err; // NOLINT
743+
if (lex->thd->variables.use_secondary_engine == SECONDARY_ENGINE_OFF) {
744+
if (has_external_table(lex->query_tables)) {
745+
my_error(ER_SECONDARY_ENGINE_PLUGIN, MYF(0),
746+
"Query could not be offloaded to the secondary engine");
747+
external_table_not_offloaded = true;
748+
goto err; // NOLINT
749+
}
757750
} else if ((thd->secondary_engine_optimization() ==
758751
Secondary_engine_optimization::PRIMARY_ONLY &&
759752
lex->thd->variables.use_secondary_engine !=
760753
SECONDARY_ENGINE_FORCED) &&
761-
is_external_table) {
754+
has_external_table(lex->query_tables)) {
762755
// throw the propagated error from the external engine in case there is an
763756
// external table
764757
external_engine_fail_reason(lex);
@@ -855,9 +848,10 @@ bool Sql_cmd_dml::execute(THD *thd) {
855848
assert(thd->get_stmt_da() != nullptr);
856849
// here we check if there is any table in an external engine to set the
857850
// error there as well.
858-
if (is_external_table)
851+
if (has_external_table(lex->query_tables)) {
859852
set_external_engine_fail_reason(lex,
860853
thd->get_stmt_da()->message_text());
854+
}
861855
set_secondary_engine_fail_reason(lex,
862856
thd->get_stmt_da()->message_text());
863857
}

sql/table.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7263,6 +7263,13 @@ void TABLE::column_bitmaps_set(MY_BITMAP *read_set_arg,
72637263
if (file && created) file->column_bitmaps_signal();
72647264
}
72657265

7266+
handler *TABLE::get_primary_handler() const {
7267+
if (s->is_primary_engine()) {
7268+
return file;
7269+
}
7270+
return file->ha_get_primary_handler();
7271+
}
7272+
72667273
bool Table_ref::set_recursive_reference() {
72677274
if (query_block->recursive_reference != nullptr) return true;
72687275
query_block->recursive_reference = this;
@@ -7280,6 +7287,18 @@ uint Table_ref::get_hidden_field_count_for_derived() const {
72807287
return derived_result->get_hidden_field_count();
72817288
}
72827289

7290+
bool Table_ref::is_external() const {
7291+
if (m_table_ref_type == TABLE_REF_BASE_TABLE && table != nullptr &&
7292+
table->file != nullptr) {
7293+
handler *primary_handler = table->get_primary_handler();
7294+
return primary_handler != nullptr &&
7295+
Overlaps(primary_handler->ht->flags,
7296+
HTON_SUPPORTS_EXTERNAL_SOURCE) &&
7297+
primary_handler->get_table_share()->has_secondary_engine();
7298+
}
7299+
return false;
7300+
}
7301+
72837302
void LEX_MFA::copy(LEX_MFA *m, MEM_ROOT *alloc) {
72847303
nth_factor = m->nth_factor;
72857304
uses_identified_by_clause = m->uses_identified_by_clause;

sql/table.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,6 +2147,12 @@ struct TABLE {
21472147
void update_covering_prefix_keys(Field *field, uint16 key_read_length,
21482148
Key_map *covering_prefix_keys);
21492149

2150+
/**
2151+
Returns the primary engine handler for the table.
2152+
If none exist, nullptr is returned.
2153+
*/
2154+
handler *get_primary_handler() const;
2155+
21502156
private:
21512157
/**
21522158
Bitmap that tells which columns are eligible for partial update in an
@@ -3157,6 +3163,9 @@ class Table_ref {
31573163
/// Returns true if a MATCH function references this table.
31583164
bool is_fulltext_searched() const { return m_fulltext_searched; }
31593165

3166+
/// Is this table only available in an external storage engine?
3167+
bool is_external() const;
3168+
31603169
/**
31613170
Set table as readonly, ie it is neither updatable, insertable nor
31623171
deletable during this statement.

0 commit comments

Comments
 (0)