Hi Sanja, Please find my review below. Lots of typos, but the two important comments are at the bottom, for the changes in sql_select.cc and sql_trigger.cc On Sat, Jan 27, 2018 at 11:39:10AM +0100, Oleksandr Byelkin wrote:
revision-id: a0ea3a5822486fa1702d6615f1be30684c3d412f (mariadb-10.1.30-26-ga0ea3a58224) parent(s): 524221e7a34d42214e82dd868348b453f2ef1591 author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2018-01-27 11:38:38 +0100 message:
MDEV-14857: problem with 10.2.11 server crashing when executing stored procedure
Counter for select numbering made stored with the statement (before was global) So now it does have always accurate value which does not depend on interruption of statement prepare by errors like lack of table in a view definition.
---
diff --git a/sql/sql_class.h b/sql/sql_class.h index 4556487bdfe..2dc2fb61e9d 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1027,6 +1027,21 @@ class Statement: public ilink, public Query_arena
LEX_STRING name; /* name for named prepared statements */ LEX *lex; // parse tree descriptor + /* + LEX whith represent curent statement (conventional, SP or PS) s/whith/which/ s/curent/current/ s/represent/represents/
+ + For example during view parsing THD::lex will point to the views LEX and + THD::stmt_lex will point on LEX of the statement where the view will be s/point on/point to/ + included + + Now used to have alwais correct select numbering inside statemet s/Now used/Currently it is used/ s/alwais/always/ s/statemet/statement/
+ (LEX::current_select_number) without storing and restoring a global + counter which was THD::select_number. + + TODO: make some unified statement representation (now SP has different) + to store such data like LEX::current_select_number. + */ + LEX *stmt_lex; /* Points to the query associated with this statement. It's const, but we need to declare it char * because all table handlers are written
diff --git a/sql/sql_lex.h b/sql/sql_lex.h index b57fba08b47..a2da6f672ca 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -727,7 +727,13 @@ class st_select_lex: public st_select_lex_node Item *prep_having;/* saved HAVING clause for prepared statement processing */ /* Saved values of the WHERE and HAVING clauses*/ Item::cond_result cond_value, having_value; - /* point on lex in which it was created, used in view subquery detection */ + /* + Point on lex in which it was created, used in view subquery detection. "Points to the LEX...". Also please shift the whole comment 1 position to the left.
+ + TODO: make also st_select_lex::parent_stmt_lex (see THD::stmt_lex) + and use st_select_lex::parent_lex & st_select_lex::parent_stmt_lex + instead of global (from THD) references where it is possible. + */ LEX *parent_lex; enum olap_type olap; /* FROM clause - points to the beginning of the TABLE_LIST::next_local list. */ @@ -2452,7 +2458,7 @@ struct LEX: public Query_tables_list /** SELECT of CREATE VIEW statement */ LEX_STRING create_view_select;
- uint number_of_selects; // valid only for view + uint current_select_number; // valid for statmet LEX (not view) s/statmet/statement/
/** Start of 'ON table', in trigger statements. */ const char* raw_trg_on_table_name_begin; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 6084c59a257..8d3116cd8b3 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -6903,7 +6903,12 @@ void THD::reset_for_next_command(bool do_clear_error) clear_error(1);
thd->free_list= 0; - thd->select_number= 1; + /* + The same thd->stmt_lex made in lex_start, but during bootstrup this "We also assign thd->stmt_lex in lex_start()"
s/bootstrup/bootstrap
+ code executed first code is executed
+ */ + thd->stmt_lex= &main_lex; thd->stmt_lex->current_select_number= 1; + DBUG_PRINT("info", ("Lex %p stmt_lex: %p", thd->lex, thd->stmt_lex)); /* Those two lines below are theoretically unneeded as THD::cleanup_after_query() should take care of this already.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index b9fe8f3162a..7a6a028ee9c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2462,6 +2462,17 @@ void JOIN::save_explain_data(Explain_query *output, bool can_overwrite, bool need_tmp_table, bool need_order, bool distinct) { + /* + If there is SELECT in this statemet with the same number it must be the + same SELECT + */ + DBUG_ASSERT(select_lex->select_number == UINT_MAX || + select_lex->select_number == INT_MAX || + !output ||
Is it really possible that output==NULL? If yes, that's probably a bug.
+ !output->get_select(select_lex->select_number) || + output->get_select(select_lex->select_number)->select_lex == + select_lex); + if (select_lex->select_number != UINT_MAX && select_lex->select_number != INT_MAX /* this is not a UNION's "fake select */ && have_query_plan != JOIN::QEP_NOT_PRESENT_YET && @@ -24601,6 +24612,11 @@ int JOIN::save_explain_data_intern(Explain_query *output, bool need_tmp_table, { explain= new (output->mem_root) Explain_select(output->mem_root, thd->lex->analyze_stmt); + if (!explain) + DBUG_RETURN(1); // EoM +#ifndef DBUG_OFF + explain->select_lex= select_lex; +#endif join->select_lex->set_explain_type(true);
explain->select_id= join->select_lex->select_number; diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 0b4978b2862..878cd5a3b57 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -1378,7 +1378,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, ulonglong save_sql_mode= thd->variables.sql_mode; LEX_STRING *on_table_name;
- thd->lex= &lex; + thd->lex= thd->stmt_lex= &lex;
The same comment as before: we change thd->stmt_lex here. The function has code below that restores the value of thd->lex: // QQ: anything else ? lex_end(&lex); thd->lex= old_lex; But the value of thd->stmt_lex is not restored?
save_db.str= thd->db; save_db.length= thd->db_length;
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog