MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:August 5 2009 12:54pm
Subject:Re: bzr commit into mysql-5.4 branch (mikael:3477) Bug#32115
View as plain text  
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
Thread
bzr commit into mysql-5.4 branch (mikael:3477) Bug#32115Mikael Ronstrom29 Jul
  • Re: bzr commit into mysql-5.4 branch (mikael:3477) Bug#32115Ingo Strüwing29 Jul
    • Re: bzr commit into mysql-5.4 branch (mikael:3477) Bug#32115Mattias Jonsson5 Aug