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
--
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring HRB München 161028
