Bug#28318 CREATE FUNCTION (UDF) requires a schema

Bug#29816 Syntactically wrong query fails with misleading error message

The core problem is that an SQL-invoked function name can be a <schema
qualified routine name> that contains no <schema name>, but the mysql
parser insists that all stored procedures (function, procedures and
triggers) must have a <schema name>, which is not true for functions.
This problem is especially visible when trying to create a function
or when a query contains a syntax error after a function call (in the
same query), both will fail with a "No database selected" message if
the session is not attached to a particular schema, but the first
one should succeed and the second fail with a "syntax error" message.

Part of the fix is to revamp the sp name handling so that a schema
name may be omitted for functions -- this means that the internal
function name representation may not have a dot, which represents
that the function doesn't have a schema name. The other part is
to place schema checks after the type (function, trigger or procedure)
of the routine is known.

mysql-test/r/sp-error.result

@@ -1452,3 +1452,16 @@ end
 until true end repeat retry;
 end//
 ERROR 42000: LEAVE with no matching label: retry
+DROP DATABASE IF EXISTS mysqltest;
+CREATE DATABASE mysqltest;
+USE mysqltest;
+DROP DATABASE mysqltest;
+SELECT inexistent(), 1 + ,;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1
+SELECT inexistent();
+ERROR 42000: FUNCTION inexistent does not exist
+SELECT .inexistent();
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '()' at line 1
+SELECT ..inexistent();
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '.inexistent()' at line 1
+USE test;

mysql-test/r/udf.result

@@ -296,4 +296,11 @@ Qcache_queries_in_cache	0
 drop table t1;
 drop function metaphon;
 set GLOBAL query_cache_size=default;
+DROP DATABASE IF EXISTS mysqltest;
+CREATE DATABASE mysqltest;
+USE mysqltest;
+DROP DATABASE mysqltest;
+CREATE FUNCTION metaphon RETURNS STRING SONAME "UDF_EXAMPLE_LIB";
+DROP FUNCTION metaphon;
+USE test;
 End of 5.0 tests.

mysql-test/t/sp-error.test

@@ -2089,6 +2089,26 @@ end//
 
 delimiter ;//
 
+#
+# Bug#29816 Syntactically wrong query fails with misleading error message
+#
+
+--disable_warnings
+DROP DATABASE IF EXISTS mysqltest;
+--enable_warnings
+CREATE DATABASE mysqltest;
+USE mysqltest;
+DROP DATABASE mysqltest;
+--error ER_PARSE_ERROR
+SELECT inexistent(), 1 + ,;
+--error ER_SP_DOES_NOT_EXIST
+SELECT inexistent();
+--error ER_PARSE_ERROR
+SELECT .inexistent();
+--error ER_PARSE_ERROR
+SELECT ..inexistent();
+USE test;
+
 #
 # BUG#NNNN: New bug synopsis
 #

mysql-test/t/udf.test

@@ -311,5 +311,19 @@ drop table t1;
 drop function metaphon;
 set GLOBAL query_cache_size=default;
 
+#
+# Bug#28318  CREATE FUNCTION (UDF) requires a schema
+#
+
+--disable_warnings
+DROP DATABASE IF EXISTS mysqltest;
+--enable_warnings
+CREATE DATABASE mysqltest;
+USE mysqltest;
+DROP DATABASE mysqltest;
+--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB
+eval CREATE FUNCTION metaphon RETURNS STRING SONAME "$UDF_EXAMPLE_LIB";
+DROP FUNCTION metaphon;
+USE test;
 
 --echo End of 5.0 tests.

sql/sp.cc

@@ -1405,12 +1405,12 @@ static bool add_used_routine(LEX *lex, Query_arena *arena,
   {
     Sroutine_hash_entry *rn=
       (Sroutine_hash_entry *)arena->alloc(sizeof(Sroutine_hash_entry) +
-                                          key->length);
+                                          key->length + 1);
     if (!rn)              // OOM. Error will be reported using fatal_error().
       return FALSE;
     rn->key.length= key->length;
     rn->key.str= (char *)rn + sizeof(Sroutine_hash_entry);
-    memcpy(rn->key.str, key->str, key->length);
+    memcpy(rn->key.str, key->str, key->length + 1);
     my_hash_insert(&lex->sroutines, (byte *)rn);
     lex->sroutines_list.link_in_list((byte *)rn, (byte **)&rn->next);
     rn->belong_to_view= belong_to_view;
@@ -1595,7 +1595,7 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex,
 
   for (Sroutine_hash_entry *rt= start; rt; rt= rt->next)
   {
-    sp_name name(rt->key.str, rt->key.length);
+    sp_name name(thd, rt->key.str, rt->key.length);
     int type= rt->key.str[0];
     sp_head *sp;
 
@@ -1603,13 +1603,6 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex,
                               &thd->sp_func_cache : &thd->sp_proc_cache),
                               &name)))
     {
-      name.m_name.str= strchr(name.m_qname.str, '.');
-      name.m_db.length= name.m_name.str - name.m_qname.str;
-      name.m_db.str= strmake_root(thd->mem_root, name.m_qname.str,
-                                  name.m_db.length);
-      name.m_name.str+= 1;
-      name.m_name.length= name.m_qname.length - name.m_db.length - 1;
-
       switch ((ret= db_find_routine(thd, type, &name, &sp)))
       {
       case SP_OK:

sql/sp_head.cc

@@ -369,17 +369,42 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
  *
  */
 
+sp_name::sp_name(THD *thd, char *key, uint key_len)
+{
+  m_sroutines_key.str= key;
+  m_sroutines_key.length= key_len;
+  m_qname.str= ++key;
+  m_qname.length= key_len - 1;
+  if ((m_name.str= strchr(m_qname.str, '.')))
+  {
+    m_db.length= m_name.str - key;
+    m_db.str= strmake_root(thd->mem_root, key, m_db.length);
+    m_name.str++;
+    m_name.length= m_qname.length - m_db.length - 1;
+  }
+  else
+  {
+    m_name.str= m_qname.str;
+    m_name.length= m_qname.length;
+    m_db.str= 0;
+    m_db.length= 0;
+  }
+  m_explicit_name= false;
+}
+
 void
 sp_name::init_qname(THD *thd)
 {
-  m_sroutines_key.length=  m_db.length + m_name.length + 2;
+  const uint dot= !!m_db.length;
+  /* m_sroutines format: m_type + [database + dot] + name + nul */
+  m_sroutines_key.length= 1 + m_db.length + dot + m_name.length;
   if (!(m_sroutines_key.str= thd->alloc(m_sroutines_key.length + 1)))
     return;
   m_qname.length= m_sroutines_key.length - 1;
   m_qname.str= m_sroutines_key.str + 1;
-  sprintf(m_qname.str, "%.*s.%.*s",
+  sprintf(m_qname.str, "%.*s%.*s%.*s",
 	  m_db.length, (m_db.length ? m_db.str : ""),
-	  m_name.length, m_name.str);
+          dot, ".", m_name.length, m_name.str);
 }
 
 

sql/sp_head.h

@@ -72,16 +72,7 @@ public:
     Creates temporary sp_name object from key, used mainly
     for SP-cache lookups.
   */
-  sp_name(char *key, uint key_len)
-  {
-    m_sroutines_key.str= key;
-    m_sroutines_key.length= key_len;
-    m_name.str= m_qname.str= key + 1;
-    m_name.length= m_qname.length= key_len - 1;
-    m_db.str= 0;
-    m_db.length= 0;
-    m_explicit_name= false;
-  }
+  sp_name(THD *thd, char *key, uint key_len);
 
   // Init. the qualified name from the db and name.
   void init_qname(THD *thd);	// thd for memroot allocation

sql/sql_yacc.yy

@@ -1569,13 +1569,15 @@ sp_name:
 	| ident
 	  {
             LEX *lex= Lex;
-            LEX_STRING db;
+            LEX_STRING db= {0,0};
+            THD *thd= YYTHD;
+
 	    if (check_routine_name($1))
             {
 	      my_error(ER_SP_WRONG_NAME, MYF(0), $1.str);
 	      MYSQL_YYABORT;
 	    }
-            if (lex->copy_db_to(&db.str, &db.length))
+            if (thd->db && thd->copy_db_to(&db.str, &db.length))
               MYSQL_YYABORT;
 	    $$= new sp_name(db, $1, false);
             if ($$)
@@ -1625,6 +1627,13 @@ create_function_tail:
 	      my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "FUNCTION");
 	      MYSQL_YYABORT;
 	    }
+
+            if (!lex->spname->m_db.length)
+            {
+              my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
+              MYSQL_YYABORT;
+            }
+
 	    /* Order is important here: new - reset - init */
 	    sp= new sp_head();
 	    sp->reset_thd_mem_root(thd);
@@ -5194,8 +5203,8 @@ simple_expr:
 #endif /* HAVE_DLOPEN */
             {
               THD *thd= lex->thd;
-              LEX_STRING db;
-              if (lex->copy_db_to(&db.str, &db.length))
+              LEX_STRING db= {0,0};
+              if (thd->db && thd->copy_db_to(&db.str, &db.length))
                 MYSQL_YYABORT;
               sp_name *name= new sp_name(db, $1, false);
               if (name)
@@ -9730,7 +9739,13 @@ trigger_tail:
 	    my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "TRIGGER");
 	    MYSQL_YYABORT;
 	  }
-	
+
+	  if (!$3->m_db.length)
+	  {
+	    my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
+	    MYSQL_YYABORT;
+	  }
+
 	  if (!(sp= new sp_head()))
 	    MYSQL_YYABORT;
 	  sp->reset_thd_mem_root(thd);
@@ -9813,6 +9828,12 @@ sp_tail:
 	    MYSQL_YYABORT;
 	  }
 
+	  if (!$3->m_db.length)
+	  {
+	    my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
+	    MYSQL_YYABORT;
+	  }
+
 	  lex->stmt_definition_begin= $2;
 
 	  /* Order is important here: new - reset - init */