Konstantin Osipov wrote:
> * Davi Arnaut <davi@stripped> [07/09/13 20:28]:
>> - if (lex->copy_db_to(&db.str, &db.length))
>> + if (thd->db && lex->copy_db_to(&db.str,
> &db.length))
>> MYSQL_YYABORT;
>
> I believe this is incorrect. lex->copy_db_to takes db either from
> thd->db or from lex->sphead. Checking *just* for thd->db here
> cancels the option of looking up the db in lex->sphead.
For the majority of the uses of the sp_name rule, lex->sphead will be
NULL anyway because you can't drop or alter or create a routine from
within another stored routine. The committed snippet was:
if (thd->db && thd->copy_db_to(&db.str, &db.length))
MYSQL_YYABORT;
> Generally, I disagree with the approach taken by the patch.
>
> The idea of lex->copy_db_to() is to consolidate all checks and
> assignements of the db in one place.
>
> Now the logic is split, again, something I was trying to avoid.
But we should keep logic where it matters...
> Additionally, it breaks an internal invariant of class sp_name
> which is *it always has a db initialized*.
But.. that was the problem! Some routines might not be associated to a
db at all.
> I would take the approach that defers creation of sp_name instance
> till the moment when it's clear we're dealing with a stored
> function.
We could move the sp_name object create from sp_name rule (bad) or we
could create a special name rule for stored functions (good). What do
you think? We still have the "problem of sp_name without db.
> Additionally, I would prohibit this grammar:
>
> create function db1.func1 udf_soname 'foo';
>
> - a udf does not belong to any database, and we should not accept
> this syntax (e.g. in 4.1 we don't, so this is, in a way, a 5.0
> regression).
That's another issue, I will open a bug report on it.
Regards,
--
Davi Arnaut, Software Engineer
MySQL Inc, www.mysql.com
Are you MySQL certified? www.mysql.com/certification
