Bug#18239 (Possible to overload internal functions with stored functions)

Bug#21025 (misleading error message when creating functions named 'x', or 'y')
Bug#22619 (Spaces considered harmful)

This change contains a fix to report warnings or errors, and multiple tests
cases.

Before this fix, name collisions between:
- Native functions
- User Defined Functions
- Stored Functions
were not systematically reported, leading to confusing behavior.

I) Native / User Defined Function

Before this fix, is was possible to create a UDF named "foo", with the same
name as a native function "foo", but it was impossible to invoke the UDF,
since the syntax "foo()" always refer to the native function.
After this fix, creating a UDF fails with an error if there is a name
collision with a native function.

II) Native / Stored Function

Before this fix, is was possible to create a SF named "db.foo", with the same
name as a native function "foo", but this was confusing since the syntax
"foo()" would refer to the native function. To refer to the Stored Function,
the user had to use the "db.foo()" syntax.
After this fix, creating a Stored Function reports a warning if there is a
name collision with a native function.

III) User Defined Function / Stored Function

Before this fix, creating a User Defined Function "foo" and a Stored Function
"db.foo" are mutually exclusive operations. Whenever the second function is
created, an error is reported. However, the test suite did not cover this
behavior.
After this fix, the  behavior is unchanged, and is now covered by test cases.

Note that the code change in this patch depends on the fix for Bug 21114.


mysql-test/r/sp.result:
  New test cases.
mysql-test/r/udf.result:
  New test cases.
mysql-test/t/sp.test:
  New test cases.
mysql-test/t/udf.test:
  New test cases.
sql/sql_lex.cc:
  Name collisions with native functions.
sql/sql_lex.h:
  Name collisions with native functions.
sql/sql_yacc.yy:
  Name collisions with native functions.
sql/share/errmsg.txt:
  Name collisions with native functions.
mysql-test/r/sp_gis.result:
  New test cases.
mysql-test/t/sp_gis.test:
  New test cases.

10 files changed, 409 insertions, 3 deletions

diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result
index aeccfd9c951..c53656c2546 100644
--- a/mysql-test/r/sp.result
+++ b/mysql-test/r/sp.result
@@ -2178,6 +2178,7 @@ set @stamped_time=in_time;
 set x=2;
 end if;
 end|
+set time_zone='+03:00';
 call bug3426(1000, @i)|
 select @i, from_unixtime(@stamped_time, '%d-%m-%Y %h:%i:%s') as time|
 @i	time
@@ -5627,4 +5628,119 @@ Called B
 drop procedure proc_21462_a|
 drop procedure proc_21462_b|
 End of 5.0 tests
+Begin of 5.1 tests
+drop function if exists pi;
+create function pi() returns varchar(50)
+return "pie, my favorite desert.";
+Warnings:
+Note	1578	This function 'pi' has the same name as a native function.
+SET @save_sql_mode=@@sql_mode;
+SET SQL_MODE='IGNORE_SPACE';
+select pi(), pi ();
+pi()	pi ()
+3.141593	3.141593
+select test.pi(), test.pi ();
+test.pi()	test.pi ()
+pie, my favorite desert.	pie, my favorite desert.
+Warnings:
+Note	1578	This function 'pi' has the same name as a native function.
+SET SQL_MODE='';
+select pi(), pi ();
+pi()	pi ()
+3.141593	3.141593
+select test.pi(), test.pi ();
+test.pi()	test.pi ()
+pie, my favorite desert.	pie, my favorite desert.
+SET @@sql_mode=@save_sql_mode;
+drop function pi;
+drop function if exists test.database;
+drop function if exists test.current_user;
+drop function if exists test.md5;
+create database nowhere;
+use nowhere;
+drop database nowhere;
+SET @save_sql_mode=@@sql_mode;
+SET SQL_MODE='IGNORE_SPACE';
+select database(), database ();
+database()	database ()
+NULL	NULL
+select current_user(), current_user ();
+current_user()	current_user ()
+root@localhost	root@localhost
+select md5("aaa"), md5 ("aaa");
+md5("aaa")	md5 ("aaa")
+47bce5c74f589f4867dbd57e9ca9f808	47bce5c74f589f4867dbd57e9ca9f808
+SET SQL_MODE='';
+select database(), database ();
+database()	database ()
+NULL	NULL
+select current_user(), current_user ();
+current_user()	current_user ()
+root@localhost	root@localhost
+select md5("aaa"), md5 ("aaa");
+md5("aaa")	md5 ("aaa")
+47bce5c74f589f4867dbd57e9ca9f808	47bce5c74f589f4867dbd57e9ca9f808
+use test;
+create function `database`() returns varchar(50)
+return "Stored function database";
+Warnings:
+Note	1578	This function 'database' has the same name as a native function.
+create function `current_user`() returns varchar(50)
+return "Stored function current_user";
+Warnings:
+Note	1578	This function 'current_user' has the same name as a native function.
+create function md5(x varchar(50)) returns varchar(50)
+return "Stored function md5";
+Warnings:
+Note	1578	This function 'md5' has the same name as a native function.
+SET SQL_MODE='IGNORE_SPACE';
+select database(), database ();
+database()	database ()
+test	test
+select current_user(), current_user ();
+current_user()	current_user ()
+root@localhost	root@localhost
+select md5("aaa"), md5 ("aaa");
+md5("aaa")	md5 ("aaa")
+47bce5c74f589f4867dbd57e9ca9f808	47bce5c74f589f4867dbd57e9ca9f808
+select test.database(), test.database ();
+test.database()	test.database ()
+Stored function database	Stored function database
+Warnings:
+Note	1578	This function 'database' has the same name as a native function.
+select test.current_user(), test.current_user ();
+test.current_user()	test.current_user ()
+Stored function current_user	Stored function current_user
+Warnings:
+Note	1578	This function 'current_user' has the same name as a native function.
+select test.md5("aaa"), test.md5 ("aaa");
+test.md5("aaa")	test.md5 ("aaa")
+Stored function md5	Stored function md5
+Warnings:
+Note	1578	This function 'md5' has the same name as a native function.
+SET SQL_MODE='';
+select database(), database ();
+database()	database ()
+test	test
+select current_user(), current_user ();
+current_user()	current_user ()
+root@localhost	root@localhost
+select md5("aaa"), md5 ("aaa");
+md5("aaa")	md5 ("aaa")
+47bce5c74f589f4867dbd57e9ca9f808	47bce5c74f589f4867dbd57e9ca9f808
+select test.database(), test.database ();
+test.database()	test.database ()
+Stored function database	Stored function database
+select test.current_user(), test.current_user ();
+test.current_user()	test.current_user ()
+Stored function current_user	Stored function current_user
+select test.md5("aaa"), test.md5 ("aaa");
+test.md5("aaa")	test.md5 ("aaa")
+Stored function md5	Stored function md5
+SET @@sql_mode=@save_sql_mode;
+drop function test.database;
+drop function test.current_user;
+drop function md5;
+use test;
+End of 5.1 tests
 drop table t1,t2;
diff --git a/mysql-test/r/sp_gis.result b/mysql-test/r/sp_gis.result
new file mode 100644
index 00000000000..2d538b52279
--- /dev/null
+++ b/mysql-test/r/sp_gis.result
@@ -0,0 +1,33 @@
+use test;
+drop function if exists a;
+drop function if exists x;
+drop function if exists y;
+create function a() returns int
+return 1;
+create function x() returns int
+return 2;
+Warnings:
+Note	1578	This function 'x' has the same name as a native function.
+create function y() returns int
+return 3;
+Warnings:
+Note	1578	This function 'y' has the same name as a native function.
+select a();
+a()
+1
+select x();
+ERROR 42000: Incorrect parameter count in the call to native function 'x'
+select y();
+ERROR 42000: Incorrect parameter count in the call to native function 'y'
+select x(PointFromText("POINT(10 20)")), y(PointFromText("POINT(10 20)"));
+x(PointFromText("POINT(10 20)"))	y(PointFromText("POINT(10 20)"))
+10	20
+select test.a(), test.x(), test.y();
+test.a()	test.x()	test.y()
+1	2	3
+Warnings:
+Note	1578	This function 'x' has the same name as a native function.
+Note	1578	This function 'y' has the same name as a native function.
+drop function a;
+drop function x;
+drop function y;
diff --git a/mysql-test/r/udf.result b/mysql-test/r/udf.result
index a552282fece..851818e7a79 100644
--- a/mysql-test/r/udf.result
+++ b/mysql-test/r/udf.result
@@ -106,6 +106,22 @@ id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	Using temporary; Using filesort
 drop table t1;
 End of 5.0 tests.
+drop function if exists pi;
+CREATE FUNCTION pi RETURNS STRING SONAME "should_not_parse.so";
+ERROR HY000: This function 'pi' has the same name as a native function.
+DROP FUNCTION IF EXISTS metaphon;
+CREATE FUNCTION metaphon(a int) RETURNS int
+return 0;
+CREATE FUNCTION metaphon RETURNS STRING SONAME "UDF_EXAMPLE_LIB";
+ERROR HY000: Function 'metaphon' already exists
+DROP FUNCTION metaphon;
+CREATE FUNCTION metaphon RETURNS STRING SONAME "UDF_EXAMPLE_LIB";
+CREATE FUNCTION metaphon(a int) RETURNS int
+return 0;
+ERROR HY000: Function 'metaphon' already exists
+CREATE FUNCTION test.metaphon(a int) RETURNS int
+return 0;
+ERROR HY000: Function 'metaphon' already exists
 DROP FUNCTION metaphon;
 DROP FUNCTION myfunc_double;
 DROP FUNCTION myfunc_nonexist;
diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test
index 7c1fd59c50b..d65c4cb526f 100644
--- a/mysql-test/t/sp.test
+++ b/mysql-test/t/sp.test
@@ -11,6 +11,7 @@
 # Tests that uses 'goto' to into sp-goto.test (currently disabled)
 # Tests that destroys system tables (e.g. mysql.proc) for error testing
 #   go to sp-destruct.
+# Tests that require --with-geometry go into sp_gis.test
 
 use test;
 
@@ -2584,6 +2585,9 @@ begin
   end if;
 end|
 
+# so that from_unixtime() has a deterministic result
+set time_zone='+03:00';
+
 call bug3426(1000, @i)|
 select @i, from_unixtime(@stamped_time, '%d-%m-%Y %h:%i:%s') as time|
 call bug3426(NULL, @i)|
@@ -6585,6 +6589,108 @@ drop procedure proc_21462_b|
 
 --echo End of 5.0 tests
 
+--echo Begin of 5.1 tests
+
+#
+# BUG#18239: Possible to overload internal functions with stored functions
+#
+
+delimiter ;|
+
+--disable_warnings
+drop function if exists pi;
+--enable_warnings
+
+create function pi() returns varchar(50)
+return "pie, my favorite desert.";
+
+SET @save_sql_mode=@@sql_mode;
+
+SET SQL_MODE='IGNORE_SPACE';
+
+select pi(), pi ();
+select test.pi(), test.pi ();
+
+SET SQL_MODE='';
+
+select pi(), pi ();
+select test.pi(), test.pi ();
+
+SET @@sql_mode=@save_sql_mode;
+
+drop function pi;
+# End of BUG#18239
+
+#
+# BUG#22619: Spaces considered harmful
+#
+
+--disable_warnings
+drop function if exists test.database;
+drop function if exists test.current_user;
+drop function if exists test.md5;
+--enable_warnings
+
+create database nowhere;
+use nowhere;
+drop database nowhere;
+
+SET @save_sql_mode=@@sql_mode;
+
+SET SQL_MODE='IGNORE_SPACE';
+
+select database(), database ();
+select current_user(), current_user ();
+select md5("aaa"), md5 ("aaa");
+
+SET SQL_MODE='';
+
+select database(), database ();
+select current_user(), current_user ();
+select md5("aaa"), md5 ("aaa");
+
+use test;
+
+create function `database`() returns varchar(50)
+return "Stored function database";
+
+create function `current_user`() returns varchar(50)
+return "Stored function current_user";
+
+create function md5(x varchar(50)) returns varchar(50)
+return "Stored function md5";
+
+SET SQL_MODE='IGNORE_SPACE';
+
+select database(), database ();
+select current_user(), current_user ();
+select md5("aaa"), md5 ("aaa");
+
+select test.database(), test.database ();
+select test.current_user(), test.current_user ();
+select test.md5("aaa"), test.md5 ("aaa");
+
+SET SQL_MODE='';
+
+select database(), database ();
+select current_user(), current_user ();
+select md5("aaa"), md5 ("aaa");
+
+select test.database(), test.database ();
+select test.current_user(), test.current_user ();
+select test.md5("aaa"), test.md5 ("aaa");
+
+SET @@sql_mode=@save_sql_mode;
+
+drop function test.database;
+drop function test.current_user;
+drop function md5;
+
+use test;
+delimiter |;
+# End of BUG#22619
+
+--echo End of 5.1 tests
 
 #
 # BUG#NNNN: New bug synopsis
diff --git a/mysql-test/t/sp_gis.test b/mysql-test/t/sp_gis.test
new file mode 100644
index 00000000000..f1d489e4cc5
--- /dev/null
+++ b/mysql-test/t/sp_gis.test
@@ -0,0 +1,35 @@
+-- source include/have_geometry.inc
+
+use test;
+
+#
+# BUG#21025: misleading error message when creating functions named 'x', or 'y'
+#
+
+--disable_warnings
+drop function if exists a;
+drop function if exists x;
+drop function if exists y;
+--enable_warnings
+
+create function a() returns int
+return 1;
+
+create function x() returns int
+return 2;
+
+create function y() returns int
+return 3;
+
+select a();
+--error ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT
+select x();
+--error ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT
+select y();
+select x(PointFromText("POINT(10 20)")), y(PointFromText("POINT(10 20)"));
+select test.a(), test.x(), test.y();
+
+drop function a;
+drop function x;
+drop function y;
+
diff --git a/mysql-test/t/udf.test b/mysql-test/t/udf.test
index 96e559f5c05..cab55fdb32a 100644
--- a/mysql-test/t/udf.test
+++ b/mysql-test/t/udf.test
@@ -130,6 +130,42 @@ drop table t1;
 --echo End of 5.0 tests.
 
 #
+# BUG#18239: Possible to overload internal functions with stored functions
+#
+
+--disable_warnings
+drop function if exists pi;
+--enable_warnings
+
+--error ER_NATIVE_FCT_NAME_COLLISION
+CREATE FUNCTION pi RETURNS STRING SONAME "should_not_parse.so";
+
+# Verify that Stored Functions and UDF are mutually exclusive
+DROP FUNCTION IF EXISTS metaphon;
+
+CREATE FUNCTION metaphon(a int) RETURNS int
+return 0;
+
+--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB
+--error ER_UDF_EXISTS
+eval CREATE FUNCTION metaphon RETURNS STRING SONAME "$UDF_EXAMPLE_LIB";
+
+DROP FUNCTION metaphon;
+
+--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB
+eval CREATE FUNCTION metaphon RETURNS STRING SONAME "$UDF_EXAMPLE_LIB";
+
+--error ER_UDF_EXISTS
+CREATE FUNCTION metaphon(a int) RETURNS int
+return 0;
+
+--error ER_UDF_EXISTS
+CREATE FUNCTION test.metaphon(a int) RETURNS int
+return 0;
+
+# End of Bug#18239
+
+#
 # Drop the example functions from udf_example
 #
 
diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt
index 3022967eeeb..5f9b9e6d563 100644
--- a/sql/share/errmsg.txt
+++ b/sql/share/errmsg.txt
@@ -6010,4 +6010,6 @@ ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT 42000
         eng "Incorrect parameter count in the call to native function '%-.64s'"
 ER_WRONG_PARAMETERS_TO_NATIVE_FCT 42000
         eng "Incorrect parameters in the call to native function '%-.64s'"
+ER_NATIVE_FCT_NAME_COLLISION
+        eng "This function '%-.64s' has the same name as a native function."
 
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
index c35ef4079d3..1c2d2cc1100 100644
--- a/sql/sql_lex.cc
+++ b/sql/sql_lex.cc
@@ -242,6 +242,12 @@ bool is_keyword(const char *name, uint len)
   return get_hash_symbol(name,len,0)!=0;
 }
 
+bool is_lex_native_function(const LEX_STRING *name)
+{
+  DBUG_ASSERT(name != NULL);
+  return (get_hash_symbol(name->str, name->length, 1) != 0);
+}
+
 /* make a copy of token before ptr and set yytoklen */
 
 static LEX_STRING get_token(LEX *lex,uint length)
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
index 7e09675cb0a..e8f275d5caf 100644
--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -1230,4 +1230,6 @@ extern void lex_end(LEX *lex);
 extern int MYSQLlex(void *arg, void *yythd);
 extern const uchar *skip_rear_comments(const uchar *ubegin, const uchar *uend);
 
+extern bool is_lex_native_function(const LEX_STRING *name);
+
 #endif /* MYSQL_SERVER */
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index f2c72654473..d7e212bf737 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -97,6 +97,17 @@ void turn_parser_debug_on()
 }
 #endif
 
+static bool is_native_function(THD *thd, const LEX_STRING *name)
+{
+  if (find_native_function_builder(thd, *name))
+    return true;
+
+  if (is_lex_native_function(name))
+    return true;
+
+  return false;
+}
+
 %}
 %union {
   int  num;
@@ -1537,6 +1548,7 @@ sp_name:
 create_function_tail:
 	  RETURNS_SYM udf_type SONAME_SYM TEXT_STRING_sys
 	  {
+            THD *thd= YYTHD;
 	    LEX *lex=Lex;
             if (lex->definer != NULL)
             {
@@ -1549,6 +1561,12 @@ create_function_tail:
 	      my_error(ER_WRONG_USAGE, MYF(0), "SONAME", "DEFINER");
               YYABORT;
             }
+            if (is_native_function(thd, & lex->spname->m_name))
+            {
+              my_error(ER_NATIVE_FCT_NAME_COLLISION, MYF(0),
+                       lex->spname->m_name.str);
+              YYABORT;
+            }
 	    lex->sql_command = SQLCOM_CREATE_FUNCTION;
 	    lex->udf.name = lex->spname->m_name;
 	    lex->udf.returns=(Item_result) $2;
@@ -1637,6 +1655,7 @@ create_function_tail:
 	  }
 	  sp_proc_stmt
 	  {
+            THD *thd= YYTHD;
 	    LEX *lex= Lex;
 	    sp_head *sp= lex->sphead;
 
@@ -1644,15 +1663,50 @@ create_function_tail:
               YYABORT;
 
 	    lex->sql_command= SQLCOM_CREATE_SPFUNCTION;
-	    sp->init_strings(YYTHD, lex);
+	    sp->init_strings(thd, lex);
             if (!(sp->m_flags & sp_head::HAS_RETURN))
             {
               my_error(ER_SP_NORETURN, MYF(0), sp->m_qname.str);
               YYABORT;
             }
+            if (is_native_function(thd, & sp->m_name))
+            {
+              /*
+                This warning will be printed when
+                [1] A client query is parsed,
+                [2] A stored function is loaded by db_load_routine.
+                Printing the warning for [2] is intentional, to cover the
+                following scenario:
+                - A user define a SF 'foo' using MySQL 5.N
+                - An application uses select foo(), and works.
+                - MySQL 5.{N+1} defines a new native function 'foo', as
+                part of a new feature.
+                - MySQL 5.{N+1} documentation is updated, and should mention
+                that there is a potential incompatible change in case of
+                existing stored function named 'foo'.
+                - The user deploys 5.{N+1}. At this point, 'select foo()'
+                means something different, and the user code is most likely
+                broken (it's only safe if the code is 'select db.foo()').
+                With a warning printed when the SF is loaded (which has to occur
+                before the call), the warning will provide a hint explaining
+                the root cause of a later failure of 'select foo()'.
+                With no warning printed, the user code will fail with no
+                apparent reason.
+                Printing a warning each time db_load_routine is executed for
+                an ambiguous function is annoying, since that can happen a lot,
+                but in practice should not happen unless there *are* name
+                collisions.
+                If a collision exists, it should not be silenced but fixed.
+              */
+              push_warning_printf(thd,
+                                  MYSQL_ERROR::WARN_LEVEL_NOTE,
+                                  ER_NATIVE_FCT_NAME_COLLISION,
+                                  ER(ER_NATIVE_FCT_NAME_COLLISION),
+                                  sp->m_name.str);
+            }
 	    /* Restore flag if it was cleared above */
-	    YYTHD->client_capabilities |= $<ulong_num>2;
-	    sp->restore_thd_mem_root(YYTHD);
+	    thd->client_capabilities |= $<ulong_num>2;
+	    sp->restore_thd_mem_root(thd);
 	  }
 	;