[MDEV-31616] Problems with a stored function EMPTY() on upgrade to 10.6 Created: 2023-07-04  Updated: 2024-01-24  Resolved: 2024-01-24

Status: Closed
Project: MariaDB Server
Component/s: JSON, Stored routines
Affects Version/s: 10.6, 10.7, 10.8, 10.9, 10.10, 10.11, 11.0, 11.1, 11.2
Fix Version/s: 10.6.17, 10.11.7, 11.0.5, 11.1.4, 11.2.3

Type: Bug Priority: Blocker
Reporter: Alexander Barkov Assignee: Alexey Botchkov
Resolution: Fixed Votes: 0
Labels: regression-10.6

Issue Links:
Problem/Incident
is caused by MDEV-17399 Add support for JSON_TABLE Closed

 Description   

This script:

CREATE OR REPLACE FUNCTION empty(a VARCHAR(128)) RETURNS int RETURN LENGTH(a)=0;
 
SELECT empty('1');

used to work without any unexpected side effects prior to 10.6:

MariaDB [test]> CREATE OR REPLACE FUNCTION empty(a VARCHAR(128)) RETURNS int RETURN LENGTH(a)=0;
 
Query OK, 0 rows affected (0.007 sec)
 
 
 
MariaDB [test]> SELECT empty('1');
 
+------------+
 
| empty('1') |
 
+------------+
 
|          0 |
 
+------------+
 
1 row in set (0.002 sec)

Starting from 10.6, behavior changed:

The CREATE statement now raises a warnings:

CREATE OR REPLACE FUNCTION empty(a VARCHAR(128)) RETURNS int RETURN LENGTH(a)=0;
 
SHOW WARNINGS;


+-------+------+--------------------------------------------------------------+
 
| Level | Code | Message                                                      |
 
+-------+------+--------------------------------------------------------------+
 
| Note  | 1585 | This function 'empty' has the same name as a native function |
 
+-------+------+--------------------------------------------------------------+

The warning is not correct. 'empty' is not a native function! It's only a new 10.6 non-reserved keyword. There should not be any warnings.

The SELECT statement now raises a syntax error:

SELECT empty('1');


ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '('1')' at line 1

This is wrong. No errors expected.

These behavior changes raise problems on upgrade.

The problem was caused by MDEV-17399.

Other keywords added in MDEV-17399 should also be checked for the same problem:

+%token  <kwd> EMPTY_SYM                     /* SQL-2016-R */
 
+%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 */



 Comments   
Comment by Alexey Botchkov [ 2023-12-18 ]

proposed fix
https://github.com/MariaDB/server/commit/0c0721cb2f7679b7c76da03928809d33a075cb47

Comment by Alexey Botchkov [ 2024-01-22 ]

https://github.com/MariaDB/server/commit/4ec01ec186aa93d4e7259bb4e8fc660a47b9d900

Comment by Alexander Barkov [ 2024-01-23 ]

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?

Comment by Alexander Barkov [ 2024-01-23 ]

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)
Comment by Alexander Barkov [ 2024-01-23 ]

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

Comment by Alexander Barkov [ 2024-01-23 ]

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 */

Comment by Alexey Botchkov [ 2024-01-23 ]

Please see the updated patch.
https://github.com/MariaDB/server/commit/558628ef952fa0ba0dc0eac4da1b2547d1cc3185

Comment by Alexander Barkov [ 2024-01-24 ]

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.
Generated at Thu Feb 08 10:25:13 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.