Hi, Alexander, Some comments/questions here. I'll need to look at the patch again after I understand why you did it this way better.
On Apr 18, Alexander Barkov wrote: > revision-id: 96b40a5e2f5 (mariadb-10.3.33-61-g96b40a5e2f5) > parent(s): 7355f7b1f5c > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-04-07 16:22:17 +0400 > message: > > MDEV-27744 InnoDB: Failing assertion: !cursor->index->is_committed() in > row0ins.cc (from row_ins_sec_index_entry_by_modify) | Assertion `0' failed in > row_upd_sec_index_entry (debug) | Corruption A detailed comment here is a must. What was the problem, how it was fixed. > diff --git a/sql/item_func.h b/sql/item_func.h > index 754b1cd1eb2..d844e45280a 100644 > --- a/sql/item_func.h > +++ b/sql/item_func.h > @@ -35,6 +35,36 @@ extern "C" /* Bug in BSDI include > file */ > #include <cmath> > > > +template<class FUNC> > +FUNC* item_func_create_2_3_args(THD *thd, const LEX_CSTRING &name, > + List<Item> *item_list) > +{ > + int arg_count= item_list ? item_list->elements : 0; > + > + switch (arg_count) { > + case 2: > + { > + Item *param_1= item_list->pop(); > + Item *param_2= item_list->pop(); > + return new (get_thd_memroot(thd)) FUNC(thd, param_1, param_2); > + } > + case 3: > + { > + Item *param_1= item_list->pop(); > + Item *param_2= item_list->pop(); > + Item *param_3= item_list->pop(); > + return new (get_thd_memroot(thd)) FUNC(thd, param_1, param_2, param_3); A constructor that takes a List<Item> wouldn't need a switch. And wouldn't need a template either, your create_check_args_ge() isn't a template. > + break; > + } > + default: > + my_error(ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT, MYF(0), name.str); > + break; > + } > + > + return NULL; > +} > + > + > class Item_func :public Item_func_or_sum > { > void sync_with_sum_func_and_with_field(List<Item> &list); > @@ -56,8 +86,41 @@ class Item_func :public Item_func_or_sum > bool check_argument_types_can_return_text(uint start, uint end) const; > bool check_argument_types_can_return_date(uint start, uint end) const; > bool check_argument_types_can_return_time(uint start, uint end) const; > + > + void print_schema_name_if_needed(String *to) const > + { > + const LEX_CSTRING schema_name= schema_name_cstring(); > + if (schema_name.length) > + { > + to->append(schema_name); > + to->append('.'); > + } this should check that schema_name isn't the same as the schema in the path (implicit path, assumed by the current sql_mode). > + } > + void print_maybe_qualified_name(String *to) const > + { > + print_schema_name_if_needed(to); > + to->append(func_name()); > + } > + > public: > > + // Print an error message for a builtin-schema qualified function call > + static void wrong_param_count_error(const LEX_CSTRING &schema_name, > + const LEX_CSTRING &func_name); > + > + // Check that the number of arguments is greater or equal to "expected" > + static bool create_check_args_ge(const LEX_CSTRING &name, > + const List<Item> *item_list, > + uint expected) > + { > + if (!item_list || item_list->elements < expected) > + { > + my_error(ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT, MYF(0), name.str); what's the point of your wrong_param_count_error() if you aren't using it? > + return true; > + } > + return false; > + } > + > table_map not_null_tables_cache; > > enum Functype { UNKNOWN_FUNC,EQ_FUNC,EQUAL_FUNC,NE_FUNC,LT_FUNC,LE_FUNC, > diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc > index 75abd9906cd..e45fe6ef6d7 100644 > --- a/sql/item_strfunc.cc > +++ b/sql/item_strfunc.cc > @@ -3204,6 +3239,39 @@ static String *default_pad_str(String *pad_str, > CHARSET_INFO *collation) > return pad_str; > } > > + > +Item_func_lpad* > +Item_func_lpad::create(THD *thd, const LEX_CSTRING &name, > + List<Item> *item_list) > +{ > + return item_func_create_2_3_args<Item_func_lpad>(thd, name, item_list); may be item_func_create_2_3_args should be in Item_func? Like create_check_args_ge() is? and may be create_check_args_ge() should be a template too? > +} > + > + > +Item_func_lpad_oracle* > +Item_func_lpad_oracle::create(THD *thd, const LEX_CSTRING &name, > + List<Item> *item_list) > +{ > + return item_func_create_2_3_args<Item_func_lpad_oracle>(thd, name, > item_list); > +} > + > + > +Item_func_rpad* > +Item_func_rpad::create(THD *thd, const LEX_CSTRING &name, > + List<Item> *item_list) > +{ > + return item_func_create_2_3_args<Item_func_rpad>(thd, name, item_list); > +} > + > + > +Item_func_rpad_oracle* > +Item_func_rpad_oracle::create(THD *thd, const LEX_CSTRING &name, > + List<Item> *item_list) > +{ > + return item_func_create_2_3_args<Item_func_rpad_oracle>(thd, name, > item_list); > +} > + > + > bool Item_func_pad::fix_length_and_dec() > { > if (arg_count == 3) > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index 7f1b362d91d..6b0a351f76e 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -1542,7 +1541,18 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, > THD *thd) > if (lex->parsing_options.lookup_keywords_after_qualifier) > next_state= MY_LEX_IDENT_OR_KEYWORD; > else > - next_state= MY_LEX_IDENT_START; // Next is ident (not keyword) > + { > + /* > + Next is: > + - A qualified func with a special syntax: > + mariadb_schema.REPLACE('a','b','c') > + mariadb_schema.SUSTRING('a',1,2) > + mariadb_schema.TRIM('a') > + - Or an identifier otherwise. No keyword lookup is done, > + all keywords are treated as identifiers. > + */ > + next_state= MY_LEX_IDENT_OR_QUALIFIED_SPECIAL_FUNC; I don't understand why you did it in the lexer and not in the parser, like - | REPLACE '(' expr ',' expr ',' expr ')' + | opt_schema REPLACE '(' expr ',' expr ',' expr ')' or (if the above won't work in LARL(1) parser) - | ident_cli '.' ident_cli '(' opt_expr_list ')' + | ident_cli '.' func_2nd_part with + func_2nd_part: ident_cli '(' opt_expr_list ')' + | REPLACE '(' expr ',' expr ',' expr ')' etc (not reviewing lexer changes anymore, until I understand the above) > + } > if (!ident_map[(uchar) yyPeek()]) // Probably ` or " > next_state= MY_LEX_START; > return((int) c); Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp