* Davi Arnaut <davi@stripped> [07/10/10 16:59]:
> > 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;
Currently it is the case. It will not be the case when all
statements are allowed in stored routines.
Why introduce a hard-to-find bug?
Besides, sp_name rule is used not only for ALTER/DROP routine, but
also for CALL.
That means your patch broke db resolution of CALL statement when it
is used from within a stored routine.
> > 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...
The only question is to how to do that. I think that the fix
should be to not use sp_name rule (stands for a name of a stored
routine) to resolve a name for a user defined function (a plugin).
> > 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.
You could classify user defined functions as "routines". I would
rather classify them as plug-ins. INSTALL PLUGIN doesn't use
sp_name rule. There are other objects which scope is site-wide,
such as tablespaces, they do not use sp_name rule either.
> > 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.
I think that instead of
CREATE_SYM FUNCTION_SYM sp_name rule we should have two rules:
CREATE_SYM FUNCTION_SYM ident . ident
and
CREATE_SYM FUNCTION_SYM ident
in the second case we should create an sp_name instance only when
we're certain it's not a udf.
> > 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.
OK.
--
-- Konstantin Osipov Software Developer, Moscow, Russia
-- MySQL AB, www.mysql.com The best DATABASE COMPANY in the GALAXY
