← Back to team overview

maria-developers team mailing list archive

Re: MDEV-8389 patch review

 

On Wed, Aug 12, 2015 at 01:58:40AM +0530, Diwas Joshi wrote:
> hello sergei,
> After I mailed you the last patch, I kept working further. I guess the code
> has come to a point where it is giving some results. So, I did the
> following things:
> Earlier we were storing all the parameters in a select query inside
> lex->value_list, but since there can be multiple functions in a query, I
> declared a list for parameters inside table_list so we can store params
> corresponding to each table function in it.

Good.

> Then instead of function name i created a table with table alias defined in
> the function and change the table name to table alias inside select query
> as well. This makes it possible for the queries inside the function
> definition to access the temporary table we created. There is a problem
> with this approach that there might already be a table present in the
> database with that name, I am still thinking of a solution for it but we
> can discuss a solution for it later.

Yes, you're right. There is a problem here.

It is not limited to the already existing tables with the same name.

Let's work with this example:

delimiter |

CREATE FUNCTION f10(param1 VARCHAR(11))
RETURNS TABLE return_table(name VARCHAR(11)) 
deterministic
BEGIN
insert into return_table values('foo');
END|
DELIMITER ;

MariaDB [test]> select name from f10('aaa');
+------+
| name |
+------+
| foo  |
+------+
1 row in set (0.29 sec)

Good. 

However, attempts to invoke the same function multiple times fail:

MariaDB [test]> select name from f10('aaa') as TBL1 , f10('bbb') as TBL2;
ERROR 1050 (42S01): Table 'return_table' already exists

Your code uses this approach:
1. Create a temporary table in a manner similar to how user-specified
CREATE TEMPORARY TABLE works. 
2. Rely on regular MySQL's name resolution process to "see" the temporary
table.

The properties of this solution are:

- If a temporary table has the same name as a regular db table, temporary
table will "shadow" the regular table (this is ok).

- user-created temporary tables are per-thread. 
Other threads do not see the temp. table (good).
However, everything that executes within the thread will see the temp. table.
For example, if a query calls two Table Functions, they will both see each
others return tables.

This is a problem.

I think, a better approach would be:
- switch to something like create_tmp_table() for creation of temporary tables.
  Tables that are created this way are normally not visible to the name
  resolution code.
- Patch the name resolution code so that the statements inside a Table Function
  see the return table of that Table Function.

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog