Hi,
I agree with some of the Rafal's comments. Please see below.
On 31 January 2008 01:30:35 Rafal Somla wrote:
> > diff -Nrup a/sql/si_objects.cc b/sql/si_objects.cc
> > --- a/sql/si_objects.cc 2008-01-14 19:10:55 -05:00
> > +++ b/sql/si_objects.cc 2008-01-30 10:29:53 -05:00
> > @@ -271,7 +271,7 @@ bool drop_object(THD *thd, const char *o
> >
> > This is a private helper function to the implementation.
> > */
> > -TABLE* open_schema_table(THD *thd, ST_SCHEMA_TABLE *st)
> > +TABLE* open_schema_table(THD *thd, ST_SCHEMA_TABLE *st, List<LEX_STRING>
> > db_list)
>
> I would extend open_schema_table() with COND* parameter - that would make
> it much more general as one could open a I_S table with arbitrary select
> condition. The usage would be then:
>
> COND *cond;
>
> <build condition cond>
>
> TABLE *t= open_schema_table(thd, st, cond);
Agreed with open_schema_table() signature.
However, I think the code should be something like:
COND *cond= create_db_select_condition(...)
TABLE *t= open_schema_table(thd, st, cond);
...
delete cond;
> > +/*
> > + Creates a WHERE clause for information schema table lookups of the
> > + for FROM INFORMATION_SCHEMA.X WHERE <db_col> IN ('a','b','c').
> > +*/
>
> OPTIONAL: I'm not sure if it really makes sense to extract this
> functionality into a separate function. The code creating appropriate WHERE
> clause could be used directly inside
> InformationSchemaIterator::prepare_is_table().
I would keep it in a separate function for the sake of keeping functions
small.
> > +COND *create_db_select_condition(THD *thd,
> > + TABLE *t,
> > + List<LEX_STRING> db_list)
Chuck, you pass List by value here. I think, you didn't mean that.
Let's pass it by pointer.
Another thing: I guess created COND* object should be deleted
somewhere/somehow?
--
Alexander Nozdrin, Software Developer
MySQL AB, Moscow, Russia, www.mysql.com
