In order to make ALTER PROCEDURE|FUNCTION work correctly, and in general to

make characteristics (and SHOW) work right, we had to separate the old
definition blob in the mysql.proc table into separate fields for parameters,
return type, and body, and handle the characteristics (like SQL SECURITY)
separately... and then reassemble the CREATE string for parsing, of course.
This is rather ugly, mostly the parser bit. (Hopefully that will be better
with the new parser.)


Docs/sp-imp-spec.txt:
  Separated the definitions string of the procedure into different columns
  in the mysql.proc schema.
mysql-test/r/sp.result:
  New characteristics tests.
mysql-test/t/sp.test:
  New characteristics tests.
scripts/mysql_create_system_tables.sh:
  Separated the definitions string of the procedure into different columns
  in the mysql.proc schema.
scripts/mysql_fix_privilege_tables.sql:
  Separated the definitions string of the procedure into different columns
  in the mysql.proc schema.
sql/sp.cc:
  Separated the definitions string of the procedure into different columns.
  Rewrote much of the code related this (have a assemble the definition
  string from its different parts now) and the way characteristics are now
  handled, in order to make ALTER actually work.
sql/sp.h:
  Changed prototypes.
sql/sp_head.cc:
  Rewrote much of the code related to the new mysql.proc schema with separate
  definition fields (have to assemble the definition string from its different
  parts now) and the way characteristics are now handled, in order to make ALTER
  actually work.
sql/sp_head.h:
  Separated the different parts of the definition strings: name, parameters,
  return type (for functions) and body.
sql/sql_yacc.yy:
  Separated the different parts of the definition strings: name, parameters,
  return type (for functions) and body.
  This is ugly and messy; hopefully there's a more elegant way to do this
  when the new parser is installed.

Docs/sp-imp-spec.txt

@@ -1063,8 +1063,10 @@
       language          enum('SQL') DEFAULT 'SQL' NOT NULL,
       sql_data_access   enum('CONTAINS_SQL') DEFAULT 'CONTAINS_SQL' NOT NULL,
       is_deterministic  enum('YES','NO') DEFAULT 'NO' NOT NULL,
-      definition        blob DEFAULT '' NOT NULL,
-      security_type     enum('INVOKER','DEFINER') DEFAULT 'INVOKER' NOT NULL,
+      security_type     enum('INVOKER','DEFINER') DEFAULT 'DEFINER' NOT NULL,
+      param_list        blob DEFAULT '' NOT NULL,
+      returns           char(64) DEFAULT '' NOT NULL,
+      body              blob DEFAULT '' NOT NULL,
       definer           char(77) binary DEFAULT '' NOT NULL,
       created           timestamp,
       modified          timestamp,
@@ -1094,4 +1096,5 @@
       PRIMARY KEY (schema,name,type)
     ) comment='Stored Procedures';
         
---
+ --
+ 
\ No newline at end of file

mysql-test/r/sp.result

@@ -691,6 +691,43 @@ delete from t1;
 delete from t2;
 drop table t3;
 drop procedure cur2;
+create procedure chistics()
+language sql
+not deterministic
+sql security definer
+comment 'Characteristics procedure test'
+ insert into t1 values ("chistics", 1);
+call chistics();
+select * from t1;
+id	data
+chistics	1
+delete from t1;
+alter procedure chistics sql security invoker name chistics2;
+show create procedure chistics2;
+Procedure	Create Procedure
+chistics2	CREATE PROCEDURE chistics2()
+    SQL SECURITY INVOKER
+    COMMENT 'Characteristics procedure test'
+insert into t1 values ("chistics", 1)
+drop procedure chistics2;
+create function chistics() returns int
+language sql
+deterministic
+sql security invoker
+comment 'Characteristics procedure test'
+ return 42;
+select chistics();
+chistics()
+42
+alter function chistics name chistics2 comment 'Characteristics function test';
+show create function chistics2;
+Function	Create Function
+chistics2	CREATE FUNCTION chistics2() RETURNS int
+    DETERMINISTIC
+    SQL SECURITY INVOKER
+    COMMENT 'Characteristics function test'
+return 42
+drop function chistics2;
 insert into t1 values ("foo", 1), ("bar", 2), ("zip", 3);
 set @@sql_mode = 'ANSI';
 create procedure modes(out c1 int, out c2 int)
@@ -881,7 +918,7 @@ n	f
 drop table fac;
 show function status like '%f%';
 Name	Type	Definer	Modified	Created	Security_type	Comment
-fac	FUNCTION	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	INVOKER	
+fac	FUNCTION	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	DEFINER	
 drop procedure ifac;
 drop function fac;
 show function status like '%f%';
@@ -946,7 +983,7 @@ end while;
 end;
 show create procedure opp;
 Procedure	Create Procedure
-opp	create procedure opp(n bigint unsigned, out pp bool)
+opp	CREATE PROCEDURE opp(n bigint unsigned, out pp bool)
 begin
 declare r double;
 declare b, s bigint unsigned default 0;
@@ -974,8 +1011,8 @@ end loop;
 end
 show procedure status like '%p%';
 Name	Type	Definer	Modified	Created	Security_type	Comment
-ip	PROCEDURE	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	INVOKER	
-opp	PROCEDURE	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	INVOKER	
+ip	PROCEDURE	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	DEFINER	
+opp	PROCEDURE	root@localhost	0000-00-00 00:00:00	0000-00-00 00:00:00	DEFINER	
 call ip(200);
 select * from primes where i=45 or i=100 or i=199;
 i	p
@@ -1043,8 +1080,8 @@ alter procedure bar2 name bar comment "3333333333";
 alter procedure bar;
 show create procedure bar;
 Procedure	Create Procedure
-bar	create procedure bar(x char(16), y int)
-comment "111111111111" sql security invoker
+bar	CREATE PROCEDURE bar(x char(16), y int)
+    COMMENT '3333333333'
 insert into test.t1 values (x, y)
 show procedure status like 'bar';
 Name	Type	Definer	Modified	Created	Security_type	Comment

mysql-test/t/sp.test

@@ -816,6 +816,36 @@ drop table t3|
 drop procedure cur2|
 
 
+# The few characteristics we parse
+create procedure chistics()
+  language sql
+  not deterministic
+  sql security definer
+  comment 'Characteristics procedure test'
+ insert into t1 values ("chistics", 1)|
+
+# Call it, just to make sure.
+call chistics()|
+select * from t1|
+delete from t1|
+alter procedure chistics sql security invoker name chistics2|
+show create procedure chistics2|
+drop procedure chistics2|
+
+create function chistics() returns int
+  language sql
+  deterministic
+  sql security invoker
+  comment 'Characteristics procedure test'
+ return 42|
+
+# Call it, just to make sure.
+select chistics()|
+alter function chistics name chistics2 comment 'Characteristics function test'|
+show create function chistics2|
+drop function chistics2|
+
+
 # Check mode settings
 insert into t1 values ("foo", 1), ("bar", 2), ("zip", 3)|
 

scripts/mysql_create_system_tables.sh

@@ -296,8 +296,10 @@ then
   c_p="$c_p   language          enum('SQL') DEFAULT 'SQL' NOT NULL,"
   c_p="$c_p   sql_data_access   enum('CONTAINS_SQL') DEFAULT 'CONTAINS_SQL' NOT NULL,"
   c_p="$c_p   is_deterministic  enum('YES','NO') DEFAULT 'NO' NOT NULL,"
-  c_p="$c_p   definition        blob DEFAULT '' NOT NULL,"
-  c_p="$c_p   security_type     enum('INVOKER','DEFINER') DEFAULT 'INVOKER' NOT NULL,"
+  c_p="$c_p   security_type     enum('INVOKER','DEFINER') DEFAULT 'DEFINER' NOT NULL,"
+  c_p="$c_p   param_list        blob DEFAULT '' NOT NULL,"
+  c_p="$c_p   returns           char(64) DEFAULT '' NOT NULL,"
+  c_p="$c_p   body              blob DEFAULT '' NOT NULL,"
   c_p="$c_p   definer           char(77) binary DEFAULT '' NOT NULL,"
   c_p="$c_p   created           timestamp,"
   c_p="$c_p   modified          timestamp,"

scripts/mysql_fix_privilege_tables.sql

@@ -147,8 +147,10 @@ CREATE TABLE IF NOT EXISTS proc (
   language          enum('SQL') DEFAULT 'SQL' NOT NULL,
   sql_data_access   enum('CONTAINS_SQL') DEFAULT 'CONTAINS_SQL' NOT NULL,
   is_deterministic  enum('YES','NO') DEFAULT 'NO' NOT NULL,
-  definition        blob DEFAULT '' NOT NULL,
-  security_type     enum('INVOKER','DEFINER') DEFAULT 'INVOKER' NOT NULL,
+  security_type     enum('INVOKER','DEFINER') DEFAULT 'DEFINER' NOT NULL,
+  param_list        blob DEFAULT '' NOT NULL,
+  returns           char(64) DEFAULT '' NOT NULL,
+  body              blob DEFAULT '' NOT NULL,
   definer           char(77) binary DEFAULT '' NOT NULL,
   created           timestamp,
   modified          timestamp,

sql/sp.h

@@ -32,8 +32,7 @@ sp_head *
 sp_find_procedure(THD *thd, LEX_STRING *name);
 
 int
-sp_create_procedure(THD *thd, char *name, uint namelen, char *def, uint deflen,
-		    st_sp_chistics *chistics);
+sp_create_procedure(THD *thd, sp_head *sp);
 
 int
 sp_drop_procedure(THD *thd, char *name, uint namelen);
@@ -54,8 +53,7 @@ sp_head *
 sp_find_function(THD *thd, LEX_STRING *name);
 
 int
-sp_create_function(THD *thd, char *name, uint namelen, char *def, uint deflen,
-		   st_sp_chistics *chistics);
+sp_create_function(THD *thd, sp_head *sp);
 
 int
 sp_drop_function(THD *thd, char *name, uint namelen);

sql/sp_head.cc

@@ -163,18 +163,51 @@ sp_head::sp_head()
 }
 
 void
-sp_head::init(LEX_STRING *name, LEX *lex)
+sp_head::init(LEX *lex)
 {
   DBUG_ENTER("sp_head::init");
-  const char *dstr = (const char*)lex->buf;
+
+  lex->spcont= m_pcont= new sp_pcontext();
+  my_init_dynamic_array(&m_instr, sizeof(sp_instr *), 16, 8);
+  m_param_begin= m_param_end= m_returns_begin= m_returns_end= m_body_begin= 0;
+  m_name.str= m_params.str= m_retstr.str= m_body.str= m_defstr.str= 0;
+  m_name.length= m_params.length= m_retstr.length= m_body.length=
+    m_defstr.length= 0;
+  DBUG_VOID_RETURN;
+}
+
+void
+sp_head::init_strings(LEX_STRING *name, LEX *lex)
+{
+  DBUG_ENTER("sp_head::init_strings");
 
   DBUG_PRINT("info", ("name: %*s", name->length, name->str));
   m_name.length= name->length;
   m_name.str= lex->thd->strmake(name->str, name->length);
+  m_params.length= m_param_end- m_param_begin;
+  m_params.str= lex->thd->strmake((char *)m_param_begin, m_params.length);
+  if (m_returns_begin && m_returns_end)
+  {
+    /* QQ KLUDGE: We can't seem to cut out just the type in the parser
+       (without the RETURNS), so we'll have to do it here. :-( */
+    char *p= (char *)m_returns_begin+strspn((char *)m_returns_begin,"\t\n\r ");
+    p+= strcspn(p, "\t\n\r ");
+    p+= strspn(p, "\t\n\r ");
+    if (p < (char *)m_returns_end)
+      m_returns_begin= (uchar *)p;
+    /* While we're at it, trim the end too. */
+    p= (char *)m_returns_end-1;
+    while (p > (char *)m_returns_begin &&
+	   (*p == '\t' || *p == '\n' || *p == '\r' || *p == ' '))
+      p-= 1;
+    m_returns_end= (uchar *)p+1;
+    m_retstr.length=  m_returns_end - m_returns_begin;
+    m_retstr.str= lex->thd->strmake((char *)m_returns_begin, m_retstr.length);
+  }
+  m_body.length= lex->end_of_query - m_body_begin;
+  m_body.str= lex->thd->strmake((char *)m_body_begin, m_body.length);
   m_defstr.length= lex->end_of_query - lex->buf;
-  m_defstr.str= lex->thd->strmake(dstr, m_defstr.length);
-  lex->spcont= m_pcont= new sp_pcontext();
-  my_init_dynamic_array(&m_instr, sizeof(sp_instr *), 16, 8);
+  m_defstr.str= lex->thd->strmake((char *)lex->buf, m_defstr.length);
   DBUG_VOID_RETURN;
 }
 
@@ -184,18 +217,12 @@ sp_head::create(THD *thd)
   DBUG_ENTER("sp_head::create");
   int ret;
 
-  DBUG_PRINT("info", ("type: %d name: %s def: %s",
-		      m_type, m_name.str, m_defstr.str));
+  DBUG_PRINT("info", ("type: %d name: %s params: %s body: %s",
+		      m_type, m_name.str, m_params.str, m_body.str));
   if (m_type == TYPE_ENUM_FUNCTION)
-    ret= sp_create_function(thd,
-			    m_name.str, m_name.length,
-			    m_defstr.str, m_defstr.length,
-			    m_chistics);
+    ret= sp_create_function(thd, this);
   else
-    ret= sp_create_procedure(thd,
-			     m_name.str, m_name.length,
-			     m_defstr.str, m_defstr.length,
-			     m_chistics);
+    ret= sp_create_procedure(thd, this);
 
   DBUG_RETURN(ret);
 }

sql/sp_head.h

@@ -56,6 +56,18 @@ public:
   List<char *> m_calls;		// Called procedures.
   List<char *> m_tables;	// Used tables.
 #endif
+  LEX_STRING m_name;
+  LEX_STRING m_params;
+  LEX_STRING m_retstr;		// For FUNCTIONs only
+  LEX_STRING m_body;
+  LEX_STRING m_defstr;
+  char *m_creator;
+  uint m_creatorlen;
+  longlong m_created;
+  longlong m_modified;
+  // Pointers set during parsing
+  uchar *m_param_begin, *m_param_end, *m_returns_begin, *m_returns_end,
+    *m_body_begin;
 
   static void *
   operator new(size_t size);
@@ -67,7 +79,11 @@ public:
 
   // Initialize after we have reset mem_root
   void
-  init(LEX_STRING *name, LEX *lex);
+  init(LEX *lex);
+
+  // Initialize strings after parsing header
+  void
+  init_strings(LEX_STRING *name, LEX *lex);
 
   int
   create(THD *thd);
@@ -136,6 +152,8 @@ public:
     return m_name.str;
   }
 
+  char *create_string(THD *thd, ulong *lenp);
+
   inline Item_result result()
   {
     return sp_map_result_type(m_returns);
@@ -143,15 +161,13 @@ public:
 
   void set_info(char *creator, uint creatorlen,
 		longlong created, longlong modified,
-		bool suid, char *comment, uint commentlen)
+		st_sp_chistics *chistics)
   {
     m_creator= creator;
     m_creatorlen= creatorlen;
     m_created= created;
     m_modified= modified;
-    m_chistics->comment.length= commentlen;
-    m_chistics->comment.str= comment;
-    m_chistics->suid= (suid ? IS_SUID : IS_NOT_SUID);
+    m_chistics= chistics;
   }
 
   inline void reset_thd_mem_root(THD *thd)
@@ -180,13 +196,6 @@ private:
   Item *m_free_list;		// Where the items go
   THD *m_thd;			// Set if we have reset mem_root
 
-  LEX_STRING m_name;
-  LEX_STRING m_defstr;
-  char *m_creator;
-  uint m_creatorlen;
-  longlong m_created;
-  longlong m_modified;
-
   sp_pcontext *m_pcont;		// Parse context
   List<LEX> m_lex;		// Temp. store for the other lex
   DYNAMIC_ARRAY m_instr;	// The "instructions"

sql/sql_yacc.yy

@@ -1040,7 +1040,7 @@ create:
 	    /* Order is important here: new - reset - init */
 	    sp= new sp_head();
 	    sp->reset_thd_mem_root(YYTHD);
-	    sp->init(&$3, lex);
+	    sp->init(lex);
 
 	    sp->m_type= TYPE_ENUM_PROCEDURE;
 	    lex->sphead= sp;
@@ -1052,10 +1052,18 @@ create:
 	    sp->m_old_cmq= YYTHD->client_capabilities & CLIENT_MULTI_QUERIES;
 	    YYTHD->client_capabilities &= (~CLIENT_MULTI_QUERIES);
 	  }
-          '(' sp_pdparam_list ')'
+          '('
 	  {
 	    LEX *lex= Lex;
 
+	    lex->sphead->m_param_begin= lex->tok_start+1;
+	  }
+	  sp_pdparam_list
+	  ')'
+	  {
+	    LEX *lex= Lex;
+
+	    lex->sphead->m_param_end= lex->tok_start;
 	    lex->spcont->set_params();
 	    bzero((char *)&lex->sp_chistics, sizeof(st_sp_chistics));
 	  }
@@ -1064,11 +1072,13 @@ create:
 	    LEX *lex= Lex;
 
 	    lex->sphead->m_chistics= &lex->sp_chistics;
+	    lex->sphead->m_body_begin= lex->tok_start;
 	  }
 	  sp_proc_stmt
 	  {
 	    LEX *lex= Lex;
 
+	    lex->sphead->init_strings(&$3, lex);
 	    lex->sql_command= SQLCOM_CREATE_PROCEDURE;
 	    /* Restore flag if it was cleared above */
 	    if (lex->sphead->m_old_cmq)
@@ -1103,7 +1113,7 @@ create_function_tail:
 	    /* Order is important here: new - reset - init */
 	    sp= new sp_head();
 	    sp->reset_thd_mem_root(YYTHD);
-	    sp->init(&lex->udf.name, lex);
+	    sp->init(lex);
 
 	    sp->m_type= TYPE_ENUM_FUNCTION;
 	    lex->sphead= sp;
@@ -1114,16 +1124,25 @@ create_function_tail:
 	     */
 	    sp->m_old_cmq= YYTHD->client_capabilities & CLIENT_MULTI_QUERIES;
 	    YYTHD->client_capabilities &= ~CLIENT_MULTI_QUERIES;
+	    lex->sphead->m_param_begin= lex->tok_start+1;
 	  }
           sp_fdparam_list ')'
 	  {
-	    Lex->spcont->set_params();
+	    LEX *lex= Lex;
+
+	    lex->spcont->set_params();
+	    lex->sphead->m_param_end= lex->tok_start;
+	  }
+	  RETURNS_SYM
+	  {
+	    Lex->sphead->m_returns_begin= Lex->tok_start;
 	  }
-	  RETURNS_SYM type
+	  type
 	  {
 	    LEX *lex= Lex;
 
-	    lex->sphead->m_returns= (enum enum_field_types)$7;
+	    lex->sphead->m_returns_end= lex->tok_start;
+	    lex->sphead->m_returns= (enum enum_field_types)$8;
 	    bzero((char *)&lex->sp_chistics, sizeof(st_sp_chistics));
 	  }
 	  sp_c_chistics
@@ -1131,16 +1150,19 @@ create_function_tail:
 	    LEX *lex= Lex;
 
 	    lex->sphead->m_chistics= &lex->sp_chistics;
+	    lex->sphead->m_body_begin= lex->tok_start;
 	  }
 	  sp_proc_stmt
 	  {
 	    LEX *lex= Lex;
+	    sp_head *sp= lex->sphead;
 
 	    lex->sql_command= SQLCOM_CREATE_SPFUNCTION;
+	    sp->init_strings(&lex->udf.name, lex);
 	    /* Restore flag if it was cleared above */
-	    if (lex->sphead->m_old_cmq)
+	    if (sp->m_old_cmq)
 	      YYTHD->client_capabilities |= CLIENT_MULTI_QUERIES;
-	    lex->sphead->restore_thd_mem_root(YYTHD);
+	    sp->restore_thd_mem_root(YYTHD);
 	  }
 	;
 
@@ -5884,6 +5906,7 @@ keyword:
 	| RESET_SYM		{}
 	| RESOURCES		{}
 	| RESTORE_SYM		{}
+	| RETURNS_SYM           {}
 	| ROLLBACK_SYM		{}
 	| ROLLUP_SYM		{}
 	| ROWS_SYM		{}