Hi Diwas, On Thu, Jul 02, 2015 at 04:26:58PM +0530, Diwas Joshi wrote:
for the return type i am storing a string containing TABLE(<col definitions>). eg. forCREATE FUNCTION f1(a INT, b VARCHAR(11)) RETURNS TABLE t1(id INT, name VARCHAR(11)) BEGIN set @a=3; END| we store TABLE t1(id INT, name VARCHAR(11))
Ok.
for the type of function we can store it as TYPE_ENUM_FUNCTION only and later see if it creates a problem, although we can always differentiate between the functions based on the return type stored above.
Yes, let's keep it this way for now.
I included the table alias with the return type string as you suggested. There is a warning that i am getting, which i am failing to figure the reason for but I am still trying. Please review.
The first thing: please add a testcase that checks what the code code does. I can think of doing this: CREATE table_function .... < whatever here> select * from mysql.proc where name='table_function'; This will show what was stored in the system table. This will also make it clear if/what warning gets produced. Since you've changed CREATE FUNCTION table_function ... not to return error, I guess some of testcases you previously added will need to be modified, too. Overall the patch seems to be ok but I would like to look at the final version of it before giving the approval.
diff --git a/sql/sp.cc b/sql/sp.cc index 9fd17b0..3413ba1 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -925,21 +925,66 @@ class Bad_db_error_handler : public Internal_error_handler bzero((char*) &share, sizeof(share)); table.in_use= thd; table.s = &share; - field= sp->create_result_field(0, 0, &table); - field->sql_type(result); - - if (field->has_charset()) + if (sp->m_type == TYPE_ENUM_FUNCTION) { - result.append(STRING_WITH_LEN(" CHARSET ")); - result.append(field->charset()->csname); - if (!(field->charset()->state & MY_CS_PRIMARY)) + field= sp->create_result_field(0, 0, &table); + field->sql_type(result); + + if (field->has_charset()) { - result.append(STRING_WITH_LEN(" COLLATE ")); - result.append(field->charset()->name); + result.append(STRING_WITH_LEN(" CHARSET ")); + result.append(field->charset()->csname); + if (!(field->charset()->state & MY_CS_PRIMARY)) + { + result.append(STRING_WITH_LEN(" COLLATE ")); + result.append(field->charset()->name); + } } + delete field; + } + else if (sp->m_type == TYPE_ENUM_TABLE) + { + List_iterator_fast<Create_field> it(sp->m_cols_list); + Create_field *cols_field; + cols_field= it++; + result.append(STRING_WITH_LEN("TABLE ")); + result.append(sp->m_table_alias.str); + result.append(STRING_WITH_LEN("(")); + while (cols_field) + { + uint unused1= 0; + sp_prepare_create_field(thd, cols_field); + prepare_create_field(cols_field, &unused1, HA_CAN_GEOMETRY); + uint field_length; + Field *field; + String tmp_result(64); + field_length= cols_field->length; + cols_field->flags= 0; + field= make_field(table.s, /* TABLE_SHARE ptr */ + (uchar*) 0, /* field ptr */ + field_length, /* field [max] length */ + (uchar*) "", /* null ptr */ + 0, /* null bit */ + cols_field->pack_flag, + cols_field->sql_type, + cols_field->charset, + cols_field->geom_type, cols_field->srid, + Field::NONE, /* unreg check */ + cols_field->interval, + cols_field->field_name); + field->vcol_info= cols_field->vcol_info; + field->stored_in_db= cols_field->stored_in_db; + if (field) + field->init(&table); + field->sql_type(tmp_result); + result.append(tmp_result); + cols_field= it++; + if(cols_field) + result.append(STRING_WITH_LEN(", ")); + delete field; + } + result.append(STRING_WITH_LEN(")")); } - - delete field; }
@@ -1020,7 +1065,8 @@ class Bad_db_error_handler : public Internal_error_handler char definer_buf[USER_HOST_BUFF_SIZE]; LEX_STRING definer; ulonglong saved_mode= thd->variables.sql_mode; - MDL_key::enum_mdl_namespace mdl_type= type == TYPE_ENUM_FUNCTION ? + MDL_key::enum_mdl_namespace mdl_type= type == (type == TYPE_ENUM_FUNCTION || + type == TYPE_ENUM_TABLE) ? MDL_key::FUNCTION : MDL_key::PROCEDURE; Looks reasonable.
CHARSET_INFO *db_cs= get_default_db_collation(thd, sp->m_db.str); @@ -1036,7 +1082,8 @@ class Bad_db_error_handler : public Internal_error_handler retstr.set_charset(system_charset_info);
DBUG_ASSERT(type == TYPE_ENUM_PROCEDURE || - type == TYPE_ENUM_FUNCTION); + type == TYPE_ENUM_FUNCTION || + type == TYPE_ENUM_TABLE);
/* Grab an exclusive MDL lock. */ if (lock_object_name(thd, mdl_type, sp->m_db.str, sp->m_name.str)) @@ -1117,7 +1164,8 @@ class Bad_db_error_handler : public Internal_error_handler
store_failed= store_failed || table->field[MYSQL_PROC_MYSQL_TYPE]-> - store((longlong)type, TRUE); + store((longlong)(type == TYPE_ENUM_TABLE ? + TYPE_ENUM_FUNCTION : type), TRUE);
store_failed= store_failed || table->field[MYSQL_PROC_FIELD_SPECIFIC_NAME]-> @@ -1145,7 +1193,7 @@ class Bad_db_error_handler : public Internal_error_handler table->field[MYSQL_PROC_FIELD_PARAM_LIST]-> store(sp->m_params.str, sp->m_params.length, system_charset_info);
- if (sp->m_type == TYPE_ENUM_FUNCTION) + if (sp->m_type == TYPE_ENUM_FUNCTION || sp->m_type == TYPE_ENUM_TABLE) { sp_returns_type(thd, retstr, sp);
@@ -1153,7 +1201,7 @@ class Bad_db_error_handler : public Internal_error_handler table->field[MYSQL_PROC_FIELD_RETURNS]-> store(retstr.ptr(), retstr.length(), system_charset_info); } - + String body_with_alias(64);
What is this for? It's not used anywhere.
store_failed= store_failed || table->field[MYSQL_PROC_FIELD_BODY]-> store(sp->m_body.str, sp->m_body.length, system_charset_info); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index b53d939..d7d7d7b 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -16392,12 +16392,6 @@ sf_tail: Lex_input_stream *lip= YYLIP;
lex->sphead->set_body_start(thd, lip->get_cpp_tok_start()); - - if (Lex->sphead->m_type== TYPE_ENUM_TABLE) //to be removed later. - { - my_error(ER_NOT_SUPPORTED_YET, MYF(0), "Table functions"); - MYSQL_YYABORT; - } } sp_proc_stmt /* $15 */ {
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog