> From e26a84946712e346d43f2eaa5c9d7ec86500910a Mon Sep 17 00:00:00 2001 > From: jb > Date: Mon, 6 Feb 2017 10:13:35 +0100 > Subject: [PATCH] MDEV-10697 sql_mode=ORACLE: GOTO statement > > --- > mysql-test/suite/compat/oracle/r/sp-goto.result | 830 ++++++++++++++++++++++ > mysql-test/suite/compat/oracle/t/sp-goto.test | 868 ++++++++++++++++++++++++ > sql/lex.h | 1 + > sql/sp_head.cc | 131 +++- > sql/sp_head.h | 31 + > sql/sp_pcontext.cc | 62 +- > sql/sp_pcontext.h | 55 +- > sql/sql_lex.cc | 48 ++ > sql/sql_lex.h | 2 + > sql/sql_yacc.yy | 2 + > sql/sql_yacc_ora.yy | 96 ++- > 11 files changed, 2084 insertions(+), 42 deletions(-) > create mode 100644 mysql-test/suite/compat/oracle/r/sp-goto.result > create mode 100644 mysql-test/suite/compat/oracle/t/sp-goto.test > > diff --git a/mysql-test/suite/compat/oracle/r/sp-goto.result b/mysql-test/suite/compat/oracle/r/sp-goto.result > new file mode 100644 > index 0000000..8a1bf31 > --- /dev/null > +++ b/mysql-test/suite/compat/oracle/r/sp-goto.result It seems you forgot to do "perl mtr --record compat/oracle.sp-goto" after last update of sp-goto.test. This test fails for me. Also, it seems "DROP PROCEDURE" is missing in the last test chunk. > diff --git a/sql/sp_head.cc b/sql/sp_head.cc > index 627b239..0caa26d 100644 > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > -sp_head::push_backpatch(THD *thd, sp_instr *i, sp_label *lab) > +sp_head::push_backpatch(THD *thd, sp_instr *i, sp_label *lab, > + List * list, backpatch_instr_type itype) In pointer declarations we use no spaces between * and the variable name. List *list, > +int > +sp_head::push_backpatch_goto(THD *thd, sp_pcontext *ctx, sp_label *lab) > +{ > + uint ip= instructions(); > + > + // Add cpop/hpop : they will be removed or updated later if target is in > + // the same block or not Multi-line comments should use /* */ style: /* Add cpop/hpop : they will be removed or updated later if target is in the same block or not */ > + DBUG_ENTER("sp_head::backpatch_goto"); > + while ((bp= li++)) > + { > + if ((bp->instr->m_ip < lab_begin_block->ip) || (bp->instr->m_ip > lab->ip)) if (bp->instr->m_ip < lab_begin_block->ip || bp->instr->m_ip > lab->ip) > + { > + // update only jump target from the beginning of the block where the > + // label is defined. /* update only jump target from the beginning of the block where the label is defined. */ > +sp_head::check_unresolved_goto() > +{ > + DBUG_ENTER("sp_head::check_unresolved_goto"); > + bool has_unresolved_label=false; > + if (m_backpatch_goto.elements>0) We use spaces around comparison operators: if (m_backpatch_goto.elements > 0) > --- a/sql/sp_head.h > +++ b/sql/sp_head.h > @@ -525,11 +525,19 @@ class sp_head :private Query_arena > DYNAMIC_ARRAY m_instr; ///< The "instructions" > + > + enum backpatch_instr_type { GOTO, CPOP, HPOP }; > typedef struct > { > sp_label *lab; > sp_instr *instr; > + uint32 instr_type; I suggest to change the data type for inst_type from uint32 to backpatch_instr_type. > + int > + push_backpatch(THD *thd, sp_instr *, sp_label *, List * list); > + int > + push_backpatch(THD *thd, sp_instr *, sp_label *, List * list, > + backpatch_instr_type itype); Please remove the space between * and list in the above two declarations. > diff --git a/sql/sp_pcontext.cc b/sql/sp_pcontext.cc > index 06c5164..48be9b2 100644 > --- a/sql/sp_pcontext.cc > +++ b/sql/sp_pcontext.cc > > +bool cmp_labels(sp_label *a, sp_label *b) > +{ > + return ((my_strcasecmp(system_charset_info, a->name.str, b->name.str)==0) > + && (a->type==b->type)); Please add spaces around == and remove redundant paretheses: return my_strcasecmp(system_charset_info, a->name.str, b->name.str == 0) && a->type == b->type; > +} > + > sp_pcontext *sp_pcontext::pop_context() > { > m_parent->m_max_var_index+= m_max_var_index; > @@ -140,6 +147,18 @@ sp_pcontext *sp_pcontext::pop_context() > if (m_num_case_exprs > m_parent->m_num_case_exprs) > m_parent->m_num_case_exprs= m_num_case_exprs; > > + /* > + ** Push unresolved goto label to parent context > + */ > + sp_label * label; Space between * and label. > @@ -227,9 +246,9 @@ sp_variable *sp_pcontext::add_variable(THD *thd, LEX_STRING name) > return m_vars.append(p) ? NULL : p; > } > > - > sp_label *sp_pcontext::push_label(THD *thd, LEX_STRING name, uint ip, > - sp_label::enum_type type) > + sp_label::enum_type type, > + List * list) Space between * and list. > + if (!recusive) > + { > + return NULL; > + } If there is only one statement, we don't use { }. if (!recursive) return NULL; > + > + return (m_parent && (m_scope == REGULAR_SCOPE)) ? > + m_parent->find_goto_label(name) : > + NULL; return m_parent && m_scope == REGULAR_SCOPE ? m_parent->find_goto_label(name) : NULL; > diff --git a/sql/sp_pcontext.h b/sql/sp_pcontext.h > index 00cc819..d526e2f 100644 > --- a/sql/sp_pcontext.h > +++ b/sql/sp_pcontext.h > @@ -94,6 +94,7 @@ class sp_variable : public Sql_alloc > + > + sp_label *sp_pcontext::push_label(THD *thd, LEX_STRING name, uint ip, > + sp_label::enum_type type) > + { return push_label(thd, name, ip, type, &m_labels); } > + > + sp_label *sp_pcontext::push_goto_label(THD *thd, LEX_STRING name, uint ip, > + sp_label::enum_type type) > + { return push_label(thd, name, ip, type, &m_goto_labels); } > > sp_label *push_label(THD *thd, const LEX_STRING name, uint ip) > - { > - return push_label(thd, name, ip, sp_label::IMPLICIT); > - } > + { return push_label(thd, name, ip, sp_label::IMPLICIT); } > + > + sp_label *push_goto_label(THD *thd, const LEX_STRING name, uint ip) > + { return push_goto_label(thd, name, ip, sp_label::GOTO); } > > sp_label *find_label(const LEX_STRING name); > > + sp_label *find_goto_label(const LEX_STRING name, bool recusive); > + > + sp_label *sp_pcontext::find_goto_label(const LEX_STRING name) > + { return find_goto_label(name, true); } > + > sp_label *find_label_current_loop_start(); Please remove all "sp_pcontext::" in the above declarations. It does not compile with GCC. > + /// List of labels for block labels > List m_labels; > + /// List of labels for goto labels > + List m_goto_labels; I suggest to remove double "labels" wording in the above comments, as follows: /// List of block labels /// List of goto labels > /// Children contexts, used for destruction. > Dynamic_array m_children; > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index c361ee0..6d799fb 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -5511,6 +5511,54 @@ bool LEX::sp_leave_statement(THD *thd, const LEX_STRING label_name) > return sp_exit_block(thd, lab, NULL); > } > > +bool LEX::sp_goto_statement(THD *thd, const LEX_STRING label_name) > +{ > + sp_label *lab= spcont->find_goto_label(label_name); > + if ((!lab) || (lab->ip == 0)) if (!lab || lab->ip == 0) > + { > + sp_label * delayedlabel; sp_label *delayedlabel; > + if (!lab) > + { > + // Label not found --> add forward jump to an unknown label > + spcont->push_goto_label(thd, label_name, 0, sp_label::GOTO); > + delayedlabel= spcont->last_goto_label(); > + } > + else > + { > + delayedlabel= lab; > + } > + return sphead->push_backpatch_goto(thd,spcont,delayedlabel); Please add spaces after commas: return sphead->push_backpatch_goto(thd, spcont, delayedlabel); > +bool LEX::sp_push_goto_label(THD *thd, const LEX_STRING label_name) > +{ > + sp_label *lab= spcont->find_goto_label(label_name,false); sp_label *lab= spcont->find_goto_label(label_name, false); > + if (lab) > + { > + if (lab->ip != 0) > + { > + my_error(ER_SP_LABEL_REDEFINE, MYF(0), label_name.str); > + return true; > + } > + lab->ip= sphead->instructions(); > + > + sp_label * beginblocklabel= spcont->find_label(empty_lex_str); sp_label *beginblocklabel= spcont->find_label(empty_lex_str); > diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy > index 0ff2318..0df9c3f 100644 > --- a/sql/sql_yacc_ora.yy > +++ b/sql/sql_yacc_ora.yy > + sp_proc_stmt_goto: > + GOTO_SYM label_ident > + { > + if (Lex->sp_goto_statement(thd, $2)) > + MYSQL_YYABORT; > + } > + ; > + > + In rule declarations we don't use spaces before "rule:". Please remove the spaces before "sp_proc_stmt_goto:".