Hi Diwas,
Nice analysis. Please find my comments below.
On Mon, Jun 29, 2015 at 10:01:56AM +0530, Diwas Joshi wrote:
> hello sergey, so i was reading further on MDEV-8356 last night. There are
> two main things here: type of sf and return type of sf. Right now the
> create function... is giving errors at
>
> store_failed= store_failed ||
> table->field[MYSQL_PROC_MYSQL_TYPE]->
> store((longlong)type, TRUE);
>
> in sf_create_routine(), which is right since we have been been using
> TYPE_ENUM_TABLE for m_type so far and mysql.proc takes
> enum('FUNCTION','PROCEDURE') for type. This can be solved by either
> changing this enum in mysql.proc definition and including TABLE, or storing
> table_valued functions as FUNCTION as well. I think first option is better,
> including TABLE in the enum, since this will help us in distinguishing
> these functions while executing later if needed.
It is not clear for me which option is better.
On one hand, there is your argument that adding 'TABLE' into the enum will
make it easier to tell table functions from regular function.
On the other hand, try grepping for 'TYPE_ENUM_FUNCTION' in sql/*.{h,cc}.
You find a lot of code that checks if m_type == TYPE_ENUM_FUNCTION.
You will need to review each check to see what should be done for
TYPE_ENUM_FUNCTION.
If we can't decide towards one option or another, let's use whatever you prefer
right now. If we discover that that works poorly, we will be able to reverse
the decision later on.
> The second thing is about return type of function. Right now
> sp_returns_type() method is being used to store the return type of
> functions. But in our case we have a list of create_fields:
> List<Create_field> m_cols_list keeping list of attribute names and their
> types for the table. We need to store all these, for this we can either
> alter mysql.proc definition to make a field appropriate for storing this
> list. The type of the field storing return type right now is longblob which
> as i read further on it is a binary large object that can hold a variable
> amount of data and are treated as binary strings (byte strings). Maybe we
> can try to find a way to store all the attribute information in this field
> only, maybe putting in some delimiters between different field types or
> some other way since after all its a string. I am still not sure about this
> problem.
I tried creating this function:
CREATE FUNCTION f2(a INT, b VARCHAR(11)) RETURNS VARCHAR(32) DETERMINISTIC
BEGIN set @a=3; RETURN @a; END|
and mysql.proc.returns has:
returns: varchar(32) CHARSET latin1
So, it is a text defition of the return type. It seems natural if for table
functions we
Is it a problem to do similar thing for Table functions? e.g for a function
CREATE FUNCTION f1(a INT, b VARCHAR(11))
RETURNS TABLE t1(id INT, name VARCHAR(11)) BEGIN set @a=3; END|
store
'TABLE t1(id INT, name VARCHAR(11)'
?
I guess it depends on where the value of mysql.proc.returns is used. Please
research this.
>
> Also, another thing is to store the table alias m_table_alias, because
> queries inside the function will be using it. There is no field present
> specifically for this in mysql.proc. So either we can create one. There is
> a field specific_name in it, as i saw in the code is right now storing the
> same thing as name field. So maybe we can use this field to store the table
> alias.
I would prefer that m_table_alias was stored as part of the expression in
mysql.proc.returns field. Can we try that option first? If we figure that
can't work, we can think of other options.
BR
Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
