Alexey Botchkov added a comment - 2023-12-18 21:28 proposed fix https://github.com/MariaDB/server/commit/0c0721cb2f7679b7c76da03928809d33a075cb47

Alexey Botchkov added a comment - 2024-01-22 09:33 https://github.com/MariaDB/server/commit/4ec01ec186aa93d4e7259bb4e8fc660a47b9d900

Alexander Barkov added a comment - 2024-01-23 07:30 Hi holyfoot , These tests fail for me in your branch: ./mtr \ perfschema.start_server_low_digest_sql_length \ main.parser \ gcol.gcol_blocked_sql_funcs_innodb \ perfschema.digest_view \ funcs_1.storedproc \ gcol.gcol_blocked_sql_funcs_myisam \ compat/oracle.parser \ vcol.vcol_blocked_sql_funcs \ main.information_schema_inno Can you please check?

Alexander Barkov added a comment - 2024-01-23 07:31 - edited Hello holyfoot , The patch looks very good for me. I'd like only to ask to rename grammar rules as follows, please: IDENT_funcname to ident_funcname. All rules containing identifiers in the system character set (versus client character set) should start with lower case "ident". (IDENT_sys should also be renamed to ident_sys eventually). This is to distinguish system character set identifiers easier from IDENT and IDENT_QUOTED, coming from the tokenizer in the client character set . ident_func to ident_cli_funcname, as all identifiers in the client character set (versus system character set) should start with "ident_cli". keyword_funcname_and_label to keyword_funcname_sp_var_and_label, as they are stored function and sp variables at the same time (and labels) keyword_funcname_not_label to keyword_funcname_sp_var_not_label, as they are stored function and sp variables at the same time (but not labels)

Alexander Barkov added a comment - 2024-01-23 07:37 - edited Also, the rule keyword_funcname does not seem to be needed. Please just include its parts into ident_cli_funcname directly. Collecting all together, I suggest this incremental patch: diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 7e1417dd6d7..9f93800b94b 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1312,7 +1312,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); %type <ident_sys> IDENT_sys - IDENT_funcname + ident_funcname ident label_ident sp_decl_ident @@ -1337,7 +1337,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); IDENT_cli ident_cli ident_cli_set_usual_case - ident_func + ident_cli_funcname %type <ident_sys_ptr> ident_sys_alloc @@ -1352,9 +1352,8 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); keyword_sp_block_section keyword_sp_decl keyword_sp_head - keyword_funcname - keyword_funcname_and_label - keyword_funcname_not_label + keyword_funcname_sp_var_and_label + keyword_funcname_sp_var_not_label keyword_sp_var_and_label keyword_sp_var_not_label keyword_sysvar_name @@ -10715,7 +10714,7 @@ function_call_conflict: in sql/item_create.cc */ function_call_generic: - IDENT_funcname '(' + ident_funcname '(' { #ifdef HAVE_DLOPEN udf_func *udf= 0; @@ -15571,14 +15570,15 @@ IDENT_sys: } ; -ident_func: +ident_cli_funcname: IDENT | IDENT_QUOTED - | keyword_funcname { $$= $1; } + | keyword_funcname_sp_var_and_label { $$= $1; } + | keyword_funcname_sp_var_not_label { $$= $1; } ; -IDENT_funcname: - ident_func +ident_funcname: + ident_cli_funcname { if (unlikely(thd->to_ident_sys_alloc(&$$, &$1))) MYSQL_YYABORT; @@ -15809,7 +15809,7 @@ non_reserved_keyword_udt: TODO: check if some of them can migrate to keyword_sp_var_and_label. */ keyword_sp_var_not_label: - keyword_funcname_not_label + keyword_funcname_sp_var_not_label | ASCII_SYM | BACKUP_SYM | BINLOG_SYM @@ -15985,12 +15985,8 @@ keyword_cast_type: /* These keywords are fine for stored function names. */ -keyword_funcname: - keyword_funcname_and_label - | keyword_funcname_not_label - ; -keyword_funcname_and_label: +keyword_funcname_sp_var_and_label: ACTION | ACCOUNT_SYM | ADMIN_SYM @@ -16299,7 +16295,7 @@ keyword_funcname_and_label: | WEEK_SYM ; -keyword_funcname_not_label: +keyword_funcname_sp_var_not_label: FORMAT_SYM | COLUMN_CHECK_SYM ; @@ -16307,7 +16303,7 @@ keyword_funcname_not_label: These keywords are fine for both SP variable names and SP labels. */ keyword_sp_var_and_label: - keyword_funcname_and_label + keyword_funcname_sp_var_and_label | ADDDATE_SYM | ANY_SYM | AVG_SYM

Alexander Barkov added a comment - 2024-01-23 08:17 - edited Can you please also add stored function MTR tests with the following tokens added in MDEV-17399 : +%token <rwd> JSON_TABLE_SYM +%token <kwd> NESTED_SYM /* SQL-2003-N */ +%token <kwd> ORDINALITY_SYM /* SQL-2003-N */ +%token <kwd> PATH_SYM /* SQL-2003-N */

Alexey Botchkov added a comment - 2024-01-23 11:46 - edited Please see the updated patch. https://github.com/MariaDB/server/commit/558628ef952fa0ba0dc0eac4da1b2547d1cc3185

Alexander Barkov added a comment - 2024-01-24 06:06 - edited Hello holyfoot , This patch: https://github.com/MariaDB/server/commit/558628ef952fa0ba0dc0eac4da1b2547d1cc3185 looks very good. Ok to push after fixing two small issues: There is a typo on the test: DROP PROCEFURE database ; Please fix the last sentence in the commit comment, to something like this: Moving keywords to new rules keyword_func_sp_var_and_label and keyword_func_sp_var_not_label so the functions with these names are allowed. - as there is no a rule keyword_funcname any more.