Hi Mikael,
After looking through the patch and your changes according to Ingo's
mail, I approve the updated patch. Ok to push for me.
/Mattias
Ingo Strüwing wrote:
> Hi Mikael,
>
> Mikael Ronstrom, 29.07.2009 16:06:
>
> ...
>> 3477 Mikael Ronstrom 2009-07-29
>> Bug#32115, made use of local lex object to avoid side effects of opening
> partitioned tables
>
>
> Please have a comment here: 1. What was the problem, 2. how was it fixed.
>
>> modified:
>> sql/sql_partition.cc
>>
>> === modified file 'sql/sql_partition.cc'
>> --- a/sql/sql_partition.cc 2009-07-08 12:17:27 +0000
>> +++ b/sql/sql_partition.cc 2009-07-29 14:06:35 +0000
>> @@ -859,6 +859,84 @@ int check_signed_flag(partition_info *pa
>> return error;
>> }
>>
>> +/*
>> + Initialize lex object for use in fix_fields and parsing.
>> +
>> + SYNOPSIS
>> + init_lex_with_single_table()
>> + thd The thread object
>> + table The table object
>> + RETURN VALUE
>> + TRUE An error occurred, memory allocation error
>> + FALSE Ok
>> +
>> + DESCRIPTION
>> + This function is used to initialize a lex object on the
>> + stack for use by fix_fields and for parsing. In order to
>> + work properly it also needs to initialize the
>> + Name_resolution_context object of the lexer.
>> + Finally it needs to set a couple of variables to ensure
>> + proper functioning of fix_fields.
>> +*/
>
>
> Please use the doxygen format for function comments as described in
> http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines#Commenting_Code
>
>> +
>> +static int
>> +init_lex_with_single_table(THD *thd, TABLE *table)
>> +{
>> + TABLE_LIST *table_list;
>> + Table_ident *table_ident;
>> + LEX *lex= thd->lex;
>
>
> I suggest to have pointer variables for the multiply used objects
> lex->select_lex and lex->select_lex.context. For me this would make the
> code easier to read.
>
>> + /*
>> + We will call the parser to create a part_info struct based on the
>> + partition string stored in the frm file.
>> + We will use a local lex object for this purpose. However we also
>> + need to set the Name_resolution_object for this lex object. We
>> + do this by using add_table_to_list where we add the table that
>> + we're working with to the Name_resolution_context.
>> + */
>> + lex_start(thd);
>> + lex->select_lex.context.init();
>> + if ((!(table_ident= new Table_ident(thd,
>> + table->s->table_name,
>> + table->s->db, TRUE))) ||
>> + (!(table_list= lex->select_lex.add_table_to_list(thd,
>> + table_ident,
>> + NULL,
>> + 0))))
>> + return TRUE;
>> + lex->select_lex.context.table_list=
>> + lex->select_lex.context.first_name_resolution_table= table_list;
>
>
> I suggest to replace the last two lines by
> lex->select_lex.context.resolve_in_table_list_only(table_list);
>
>> + lex->use_only_table_context= TRUE;
>
>
> This comment is unrelated to the patch: Why is the
> 'use_only_table_context' variable needed in addition to
> Name_resolution_context::resolve_in_select_list ?
>
> ...
>
> Regards
> Ingo
