maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #08826
Re: MDEV-8389 patch review
Hi Diwas,
I've got a crash when trying the first example.
The example was:
CREATE FUNCTION f1(a INT, b VARCHAR(11)) RETURNS TABLE t1(id INT,
name VARCHAR(11))
DETERMINISTIC
BEGIN
INSERT INTO t1 SELECT id, name FROM t2 WHERE id = a;
END|;
delimiter ;
select f1(1,'aa');
The crash was:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7eb8700 (LWP 24655)]
0x0000555555c6a82a in Field_str::Field_str (this=0x7fff5c007d30, ptr_arg=0x0, len_arg=4294967295, null_ptr_arg=0x55555644299c "", null_bit_arg=1 '\001', unireg_check_arg=Field::NONE, field_name_arg=0x7fff5c006780 "f1(1,'aa')", charset_arg=0x0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/field.cc:1845
(gdb) wher
#0 0x0000555555c6a82a in Field_str::Field_str (this=0x7fff5c007d30, ptr_arg=0x0, len_arg=4294967295, null_ptr_arg=0x55555644299c "", null_bit_arg=1 '\001', unireg_check_arg=Field::NONE, field_name_arg=0x7fff5c006780 "f1(1,'aa')", charset_arg=0x0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/field.cc:1845
#1 0x0000555555ae6820 in Field_longstr::Field_longstr (this=0x7fff5c007d30, ptr_arg=0x0, len_arg=4294967295, null_ptr_arg=0x55555644299c "", null_bit_arg=1 '\001', unireg_check_arg=Field::NONE, field_name_arg=0x7fff5c006780 "f1(1,'aa')", charset_arg=0x0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/field.h:1200
#2 0x0000555555c7d85c in Field_blob::Field_blob (this=0x7fff5c007d30, ptr_arg=0x0, null_ptr_arg=0x55555644299c "", null_bit_arg=1 '\001', unireg_check_arg=Field::NONE, field_name_arg=0x7fff5c006780 "f1(1,'aa')", share=0x7fff5c0062a0, blob_pack_length=4, cs=0x0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/field.cc:7415
#3 0x0000555555c85725 in make_field (share=0x7fff5c0062a0, ptr=0x0, field_length=2779096485, null_pos=0x55555644299c "", null_bit=1 '\001', pack_flag=2779096485, field_type=MYSQL_TYPE_NULL, field_charset=0x0, geom_type=Field::GEOM_GEOMETRY, srid=0, unireg_check=Field::NONE, interval=0x0, field_name=0x7fff5c006780 "f1(1,'aa')") at /home/psergey/dev-git/10.1-gsoc-repo/sql/field.cc:9861
#4 0x0000555555db4d35 in sp_head::create_result_field (this=0x7fff5c01c398, field_max_length=0, field_name=0x7fff5c006780 "f1(1,'aa')", table=0x7fff5c005710) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sp_head.cc:885
#5 0x0000555555d0d2b3 in Item_func_sp::init_result_field (this=0x7fff5c0055e0, thd=0x55555acf2800) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item_func.cc:6593
#6 0x0000555555d0dc99 in Item_func_sp::fix_fields (this=0x7fff5c0055e0, thd=0x55555acf2800, ref=0x7fff5c006778) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item_func.cc:6844
#7 0x00005555559fd1ed in setup_fields (thd=0x55555acf2800, ref_pointer_array=0x7fff5c007c90, fields=..., mark_used_columns=MARK_COLUMNS_READ, sum_func_list=0x7fff5c007a80, allow_sum_func=true) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_base.cc:7925
#8 0x0000555555aa6a83 in JOIN::prepare (this=0x7fff5c007750, rref_pointer_array=0x55555acf6b28, tables_init=0x0, wild_num=0, conds_init=0x0, og_num=0, order_init=0x0, skip_order_by=false, group_init=0x0, having_init=0x0, proc_param_init=0x0, select_lex_arg=0x55555acf68b0, unit_arg=0x55555acf61c0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:782
#9 0x0000555555aafa67 in mysql_select (thd=0x55555acf2800, rref_pointer_array=0x55555acf6b28, tables=0x0, wild_num=0, fields=..., conds=0x0, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2147748608, result=0x7fff5c007730, unit=0x55555acf61c0, select_lex=0x55555acf68b0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:3301
#10 0x0000555555aa5d93 in handle_select (thd=0x55555acf2800, lex=0x55555acf60f8, result=0x7fff5c007730, setup_tables_done_option=0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:371
#11 0x0000555555a65e98 in execute_sqlcom_select (thd=0x55555acf2800, all_tables=0x0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:5798
#12 0x0000555555a5c151 in mysql_execute_command (thd=0x55555acf2800) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:2939
#13 0x0000555555a6942e in mysql_parse (thd=0x55555acf2800, rawbuf=0x7fff5c0051b8 "select f1(1,'aa')", length=17, parser_state=0x7ffff7eb7100) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:7174
#14 0x0000555555a5838b in dispatch_command (command=COM_QUERY, thd=0x55555acf2800, packet=0x55555acf9b41 "select f1(1,'aa')", packet_length=17) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:1470
#15 0x0000555555a570a5 in do_command (thd=0x55555acf2800) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:1093
(gdb) list
1840 const char *field_name_arg, CHARSET_INFO *charset_arg)
1841 :Field(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
1842 unireg_check_arg, field_name_arg)
1843 {
1844 field_charset= charset_arg;
1845 if (charset_arg->state & MY_CS_BINSORT)
1846 flags|=BINARY_FLAG;
1847 field_derivation= DERIVATION_IMPLICIT;
1848 }
1849
(gdb) p charset_arg
$10 = (CHARSET_INFO *) 0x0
Another piece of feedback: Please don't change old tests into new tests. Tests
that were made for previous MDEVs should stay. New tasks should add new tests.
I'll continue to look at the patch.
On Sun, Aug 02, 2015 at 06:51:26PM +0530, Diwas Joshi wrote:
> I am attaching the patch for MDEV-8389. As discussed earlier, I am creating
> tables in the query corresponding to table functions before they are being
> accessed. I am checking for each table if its a table-function using
> sp_find_routine. If it is then we make a table for it. Going by the same
> mechanism used in sp_find_routine to parse the function, i parse a create
> table query which initializes create_info and alter_info which i later use
> in mysql_create_table() to create temporary tables. I also added a test
> case for it. Seems to be working fine.
>
> I guess we'll also need a function that will delete the tables create after
> use. I am working on it right now, but i guess that won't be needed in this
> patch.
> Please review the patch.
>
> regards
> Diwas Joshi
> diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result
> index cacfeea..4373925 100644
> --- a/mysql-test/r/sp-error.result
> +++ b/mysql-test/r/sp-error.result
> @@ -2866,7 +2866,3 @@ SELECT @msg;
> DROP FUNCTION f1;
> DROP FUNCTION f2;
> DROP TABLE t1;
> -SELECT * FROM f1('xyz');
> -ERROR 42000: This version of MariaDB doesn't yet support 'Queries using Table Functions'
> -SELECT * FROM t1, f1('xyz',t1.a);
> -ERROR 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 '' at line 1
> diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result
> index f06939b..d0f6578 100644
> --- a/mysql-test/r/sp.result
> +++ b/mysql-test/r/sp.result
> @@ -7943,29 +7943,7 @@ RETURNS TABLE t1(id INT, name VARCHAR(11))
> BEGIN
> INSERT INTO t1 SELECT id, name FROM t2 WHERE id = a;
> END|
> -SELECT * FROM mysql.proc WHERE db = database() and name = 'f1';
> -db test
> -name f1
> -type FUNCTION
> -specific_name f1
> -language SQL
> -sql_data_access CONTAINS_SQL
> -is_deterministic NO
> -security_type DEFINER
> -param_list a INT, b VARCHAR(11)
> -returns TABLE t1(id int(11), name varchar(11))
> -body BEGIN
> -INSERT INTO t1 SELECT id, name FROM t2 WHERE id = a;
> -END
> -definer root@localhost
> -created 2015-07-03 16:36:36
> -modified 2015-07-03 16:36:36
> -sql_mode
> -comment
> -character_set_client latin1
> -collation_connection latin1_swedish_ci
> -db_collation latin1_swedish_ci
> -body_utf8 BEGIN
> -INSERT INTO t1 SELECT id, name FROM t2 WHERE id = a;
> -END
> +SELECT * FROM f1(10,'XYZ');
> +id name
> +DROP TABLE f1;
> DROP FUNCTION f1;
> diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test
> index ccf5b98..c1a21c8 100644
> --- a/mysql-test/t/sp-error.test
> +++ b/mysql-test/t/sp-error.test
> @@ -3845,8 +3845,3 @@ SELECT @msg;
> DROP FUNCTION f1;
> DROP FUNCTION f2;
> DROP TABLE t1;
> -
> ---error 1235
> -SELECT * FROM f1('xyz');
> ---error 1064
> -SELECT * FROM t1, f1('xyz',t1.a);
> diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test
> index 5c42df0..906c91c 100644
> --- a/mysql-test/t/sp.test
> +++ b/mysql-test/t/sp.test
> @@ -9397,7 +9397,6 @@ END|
>
> DELIMITER ;|
>
> ---vertical_results
> -SELECT * FROM mysql.proc WHERE db = database() and name = 'f1';
> -
> +SELECT * FROM f1(10,'XYZ');
> +DROP TABLE f1;
> DROP FUNCTION f1;
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index 2998972..dd16c36 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -2467,6 +2467,8 @@ static bool do_execute_sp(THD *thd, sp_head *sp)
> /* should be assigned after making first tables same */
> all_tables= lex->query_tables;
> /* set context for commands which do not use setup_tables */
> + if (lex->sql_command == SQLCOM_SELECT)
> + check_and_process_table_functions(thd, all_tables);
> select_lex->
> context.resolve_in_table_list_only(select_lex->
> table_list.first);
> @@ -9222,3 +9224,142 @@ CHARSET_INFO *find_bin_collation(CHARSET_INFO *cs)
> }
> return cs;
> }
> +
> +/*
> + @brief
> + Check for table functions and create corresponding tables to be used in query.
> +
> + @param thd Thread handle
> + @param table_list A list of tables being processed in the query.
> +
> + @return
> + False on success
> + @retval
> + TRUE on error
> +*/
> +bool check_and_process_table_functions(THD *thd, TABLE_LIST *table_list)
> +{
> + TABLE_LIST *tables = table_list;
> + int err= 0;
> + while(tables)
> + {
> + if (tables->table_name == NULL)
> + {
> + tables = tables->next_local;
> + continue;
> + }
> + sp_head *sp;
> + sp_name *name;
> + LEX_STRING lex_db;
> + LEX_STRING lex_name;
> + lex_db.length= strlen(tables->db);
> + lex_name.length= strlen(tables->table_name);
> + lex_db.str= thd->strmake(tables->db, lex_db.length);
> + lex_name.str= thd->strmake(tables->table_name, lex_name.length);
> + name= new sp_name(lex_db, lex_name, true);
> + name->init_qname(thd);
> + Object_creation_ctx *creation_ctx;
> + int err=0;
> +
> + if (!err && (sp = sp_find_routine(thd,TYPE_ENUM_FUNCTION, name,
> + &thd->sp_proc_cache, FALSE)) &&
> + sp->m_type== TYPE_ENUM_TABLE) //check if the table is a table-valued function.
> + {
> + LEX *old_lex= thd->lex, newlex;
> + thd->lex= &newlex;
> + ulonglong sql_mode= 0 , saved_mode= thd->variables.sql_mode;
> + String defstr; //query in defstr will be parsed to create temporary table.
> + defstr.append(STRING_WITH_LEN("CREATE TEMPORARY TABLE "));
> + defstr.append(tables->table_name);
> + defstr.append(STRING_WITH_LEN(" ("));
> + List_iterator_fast<Create_field> it(sp->m_cols_list);
> +
> + TABLE table;
> + TABLE_SHARE share;
> + bzero((char*) &table, sizeof(table));
> + bzero((char*) &share, sizeof(share));
> + table.in_use= thd;
> + table.s = &share;
> + Create_field *cols_field;
> + cols_field= it++;
> + 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(512);
> + 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);
> + defstr.append(cols_field->field_name);
> + defstr.append(STRING_WITH_LEN(" "));
> + field->sql_type(tmp_result);
> + defstr.append(tmp_result);
> + cols_field= it++;
> + if(cols_field)
> + defstr.append(STRING_WITH_LEN(", "));
> + delete field;
> + }
> + defstr.append(STRING_WITH_LEN(")"));
> + char saved_cur_db_name_buf[SAFE_NAME_LEN+1];
> + LEX_STRING saved_cur_db_name= { saved_cur_db_name_buf,
> + sizeof(saved_cur_db_name_buf) };
> + newlex.create_info.DDL_options_st::init();
> + Stored_program_creation_ctx *creation_ctx;
> + char definer_user_name_holder[USERNAME_LENGTH + 1];
> + LEX_STRING definer_user_name= { definer_user_name_holder, USERNAME_LENGTH };
> + char definer_host_name_holder[HOSTNAME_LENGTH + 1];
> + LEX_STRING definer_host_name= { definer_host_name_holder, HOSTNAME_LENGTH };
> + newlex.current_select= NULL;
> + Parser_state parser_state;
> + thd->variables.sql_mode= sql_mode;
> + err= parser_state.init(thd, defstr.c_ptr_safe(), defstr.length());
> + if (!err) {
> + lex_start(thd);
> + err= parse_sql(thd, &parser_state, creation_ctx) || thd->lex == NULL;
> + }
> + if (!err)
> + {
> + Table_specification_st create_info(thd->lex->create_info);
> + Alter_info alter_info(thd->lex->alter_info, thd->mem_root);
> + thd->lex->create_info.default_table_charset= NULL;
> + SELECT_LEX *select_lex= &thd->lex->select_lex;
> + TABLE_LIST *create_table= select_lex->table_list.first;
> + if (append_file_to_dir(thd, &create_info.data_file_name,
> + create_table->table_name) ||
> + append_file_to_dir(thd, &create_info.index_file_name,
> + create_table->table_name))
> + {
> + err= 1;
> + }
> + if (!(create_info.used_fields & HA_CREATE_USED_ENGINE))
> + create_info.use_default_db_type(thd);
> +
> + err= mysql_create_table(thd, create_table, &create_info, &alter_info);
> + thd->variables.option_bits|= OPTION_KEEP_LOG;
> + }
> + thd->variables.sql_mode= saved_mode;
> + lex_end(thd->lex);
> + thd->lex= old_lex;
> + }
> + tables = tables->next_local;
> + }
> + return err;
> +}
> \ No newline at end of file
> diff --git a/sql/sql_parse.h b/sql/sql_parse.h
> index 76e1b7a..6b89b35 100644
> --- a/sql/sql_parse.h
> +++ b/sql/sql_parse.h
> @@ -121,7 +121,7 @@ bool push_new_name_resolution_context(THD *thd,
> Item *normalize_cond(Item *cond);
> Item *negate_expression(THD *thd, Item *expr);
> bool check_stack_overrun(THD *thd, long margin, uchar *dummy);
> -
> +bool check_and_process_table_functions(THD *thd, TABLE_LIST *table_list);
> /* Variables */
>
> extern const char* any_db;
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index d7d7d7b..dd1f9f6 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -10802,7 +10802,6 @@ table_factor:
> Yacc_state *state= & thd->m_parser_state->m_yacc;
> if (state->table_factor_part_type== 2)
> {
> - mysql_init_select(lex);
> List_iterator_fast<Item> it(lex->value_list);
> Item *item;
> while ((item = it++))
> @@ -10817,9 +10816,6 @@ table_factor:
> MYSQL_YYABORT;
> sel->add_joined_table($$);
> lex->pop_context();
> -
> - my_error(ER_NOT_SUPPORTED_YET, MYF(0),
> - "Queries using Table Functions"); //to be removed later
> }
> else
> {
--
BR
Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
