Hi Sergey,
On 12/7/10 12:33 PM, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1-bugteam-new/ based on
> revid:sergey.glukhov@stripped
>
> 3514 Sergey Glukhov 2010-12-07
> Fixed following problems:
> --Bug#52157 various crashes and assertions with multi-table update, stored
> function
> --Bug#54475 improper error handling causes cascading crashing failures in
> innodb/ndb
> --Bug#57703 create view cause Assertion failed: 0, file .\item_subselect.cc,
> line 846
> --Bug#57352 valgrind warnings when creating view
> --Recently discovered problem when a nested materialized derived table is
> used
> before being populated and it leads to incorrect result
>
> We have several modes when we should disable subquery evaluation.
> The reasons for disabling are different. It could be
> uselessness of the evaluation as in case of 'CREATE VIEW'
> or 'PREPARE stmt', or we should disable subquery evaluation
> if tables are not locked yet as it happens in bug#54475, or
> too early evaluation of subqueries can lead to wrong result
> as it happened in Bug#19077.
> Main problem is that if subquery items are treated as const
> they are evaluated in ::fix_fields(), ::fix_length_and_dec()
> of the parental items as a lot of these methods have
> Item::val_...() calls inside.
> We have to make subqueries non-const to prevent unnecessary
> subquery evaluation. At the moment we have different methods
> for this. Here is a list of these modes:
>
> 1. PREPARE stmt;
> We use UNCACHEABLE_PREPARE flag.
> It is set during parsing in sql_parse.cc, mysql_new_select() for
> each SELECT_LEX object and cleared at the end of PREPARE in
> sql_prepare.cc, init_stmt_after_parse(). If this flag is set
> subquery becomes non-const and evaluation does not happen.
>
> 2. CREATE|ALTER VIEW, SHOW CREATE VIEW, I_S tables which
> process FRM files
> We use LEX::view_prepare_mode field. We set it before
> view preparation and check this flag in
> ::fix_fields(), ::fix_length_and_dec().
> Some of bugs are fixed using this approach,
"of" is not needed.
> some are not(Bug#57352, Bug#57703). The problem here is.
Stray dot above: "The problem here is."
> that we have a lot of ::fix_fields(), ::fix_length_and_dec()
> where we use Item::val_...() calls for const items.
>
> 3. Derived tables with subquery = wrong result(Bug19077)
> The reason of this bug is too early subquery evaluation.
> It's fixed by adding Item::with_subselect field
I think you mean: "It was fixed by".
> The check of this field in appropriate places prevents
> const item evaluation if the item have subquery.
> The fix for Bug19077 fixes only the problem with.
Stray dot above.
> convert_constant_item() function and does not cover
> other places(::fix_fields(), ::fix_length_and_dec() again)
> where subqueries could be evaluated.
>
> Example:
> CREATE TABLE t1 (i INT, j BIGINT);
> INSERT INTO t1 VALUES (1, 2), (2, 2), (3, 2);
> SELECT * FROM (SELECT MIN(i) FROM t1
> WHERE j = SUBSTRING('12', (SELECT * FROM (SELECT MIN(j) FROM t1) t2))) t3;
> DROP TABLE t1;
>
> 4. Derived tables with subquery where subquery
> is evaluated before table locking(Bug#54475, Bug#52157)
>
> Suggested solution is following:
>
> -Introduce new field LEX::context_analysis_only with the following
> possible flags:
> #define CONTEXT_ANALYSIS_ONLY_PREPARE 1
> #define CONTEXT_ANALYSIS_ONLY_VIEW 2
> #define CONTEXT_ANALYSIS_ONLY_DERIVED 4
> -Set/clean these flags when we perform
> context analysis operation
> -Item_subselect::const_item() returns
> result depending on LEX::context_analysis_only.
> If context_analysis_only is set then we return
> FALSE that means that subquery is non-const.
> As all subquery types are wrapped by Item_subselect
> it allow as to make subquery non-const when
> it's necessary.
Some comments below.
[..]
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2010-11-18 13:11:18 +0000
> +++ b/sql/item.cc 2010-12-07 14:32:55 +0000
> @@ -1712,16 +1712,7 @@ bool agg_item_set_converter(DTCollation
>
> if (!(conv= (*arg)->safe_charset_converter(coll.collation))&&
> ((*arg)->collation.repertoire == MY_REPERTOIRE_ASCII))
> - {
> - /*
> - We should disable const subselect item evaluation because
> - subselect transformation does not happen in view_prepare_mode
> - and thus val_...() methods can not be called for const items.
> - */
> - bool resolve_const= ((*arg)->type() == Item::SUBSELECT_ITEM&&
> - thd->lex->view_prepare_mode) ? FALSE : TRUE;
> - conv= new Item_func_conv_charset(*arg, coll.collation, resolve_const);
> - }
> + conv= new Item_func_conv_charset(*arg, coll.collation, 1);
The third parameter is now always constant.
More unnecessary code that you can remove.
> if (!conv)
> {
>
> === modified file 'sql/item_cmpfunc.cc'
> --- a/sql/item_cmpfunc.cc 2010-09-09 12:48:06 +0000
> +++ b/sql/item_cmpfunc.cc 2010-12-07 14:32:55 +0000
> @@ -401,7 +401,7 @@ static bool convert_constant_item(THD *t
> Field *field= field_item->field;
> int result= 0;
>
> - if (!(*item)->with_subselect&& (*item)->const_item())
> + if ((*item)->const_item())
> {
> TABLE *table= field->table;
> ulong orig_sql_mode= thd->variables.sql_mode;
> @@ -497,7 +497,9 @@ void Item_bool_func2::fix_length_and_dec
> }
>
> thd= current_thd;
> - if (!thd->is_context_analysis_only())
> + if (!(thd->lex->context_analysis_only&
> + (CONTEXT_ANALYSIS_ONLY_PREPARE |
> + CONTEXT_ANALYSIS_ONLY_VIEW)))
Let's still use a wrapper function, I suggest we simply rename
context_analysis_only() to is_ps_or_view_context_analysis().
[..]
> === modified file 'sql/item_subselect.cc'
> --- a/sql/item_subselect.cc 2010-10-18 12:12:27 +0000
> +++ b/sql/item_subselect.cc 2010-12-07 14:32:55 +0000
> @@ -123,20 +123,6 @@ void Item_subselect::cleanup()
> }
>
>
> -/*
> - We cannot use generic Item::safe_charset_converter() because
> - Subselect transformation does not happen in view_prepare_mode
> - and thus we can not evaluate val_...() for const items.
> -*/
> -
> -Item *Item_subselect::safe_charset_converter(CHARSET_INFO *tocs)
> -{
> - Item_func_conv_charset *conv=
> - new Item_func_conv_charset(this, tocs, thd->lex->view_prepare_mode ? 0 :
> 1);
> - return conv->safe ? conv : NULL;
> -}
> -
> -
> void Item_singlerow_subselect::cleanup()
> {
> DBUG_ENTER("Item_singlerow_subselect::cleanup");
> @@ -271,6 +257,7 @@ bool Item_subselect::exec()
> if (thd->is_error() || thd->killed)
> return 1;
>
> + DBUG_ASSERT(!thd->lex->context_analysis_only);
> /*
> Simulate a failure in sub-query execution. Used to test e.g.
> out of memory or query being killed conditions.
> @@ -307,6 +294,8 @@ table_map Item_subselect::used_tables()
>
> bool Item_subselect::const_item() const
> {
> + if (thd->lex->context_analysis_only)
> + return FALSE;
> return const_item_cache;
Suggestion:
return thd->lex->context_analysis_only ? FALSE : const_item_cache;
> }
>
> @@ -1638,7 +1627,8 @@ bool Item_in_subselect::fix_fields(THD *
> {
> bool result = 0;
>
> - if (thd_arg->lex->view_prepare_mode&& left_expr&&
> !left_expr->fixed)
> + if ((thd_arg->lex->context_analysis_only&
> CONTEXT_ANALYSIS_ONLY_VIEW)&&
> + left_expr&& !left_expr->fixed)
> result = left_expr->fix_fields(thd_arg,&left_expr);
>
> return result || Item_subselect::fix_fields(thd_arg, ref);
>
> === modified file 'sql/item_subselect.h'
> --- a/sql/item_subselect.h 2010-04-06 07:26:59 +0000
> +++ b/sql/item_subselect.h 2010-12-07 14:32:55 +0000
> @@ -126,7 +126,6 @@ public:
> virtual void reset_value_registration() {}
> enum_parsing_place place() { return parsing_place; }
> bool walk(Item_processor processor, bool walk_subquery, uchar *arg);
> - Item *safe_charset_converter(CHARSET_INFO *tocs);
>
> /**
> Get the SELECT_LEX structure associated with this Item.
>
> === modified file 'sql/mysql_priv.h'
> --- a/sql/mysql_priv.h 2010-10-07 08:13:11 +0000
> +++ b/sql/mysql_priv.h 2010-12-07 14:32:55 +0000
> @@ -566,17 +566,25 @@ protected:
>
> #define MY_CHARSET_BIN_MB_MAXLEN 1
>
> +/*
> + These flags are set when we perform
> + context analysis of the statement and make
> + subqueries non-const. It prevents subquery
> + evaluation at context analysis stage.
> +*/
> +#define CONTEXT_ANALYSIS_ONLY_PREPARE 1
> +#define CONTEXT_ANALYSIS_ONLY_VIEW 2
> +#define CONTEXT_ANALYSIS_ONLY_DERIVED 4
> +
> // uncachable cause
> #define UNCACHEABLE_DEPENDENT 1
> #define UNCACHEABLE_RAND 2
> #define UNCACHEABLE_SIDEEFFECT 4
> /// forcing to save JOIN for explain
> #define UNCACHEABLE_EXPLAIN 8
> -/** Don't evaluate subqueries in prepare even if they're not correlated */
> -#define UNCACHEABLE_PREPARE 16
> /* For uncorrelated SELECT in an UNION with some correlated SELECTs */
> -#define UNCACHEABLE_UNITED 32
> -#define UNCACHEABLE_CHECKOPTION 64
> +#define UNCACHEABLE_UNITED 16
> +#define UNCACHEABLE_CHECKOPTION 32
>
[..]
> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h 2010-08-18 04:56:06 +0000
> +++ b/sql/sql_lex.h 2010-12-07 14:32:55 +0000
> @@ -1715,14 +1715,8 @@ typedef struct st_lex : public Query_tab
> bool verbose, no_write_to_binlog;
>
> bool tx_chain, tx_release;
> - /*
> - Special JOIN::prepare mode: changing of query is prohibited.
> - When creating a view, we need to just check its syntax omitting
> - any optimizations: afterwards definition of the view will be
> - reconstructed by means of ::print() methods and written to
> - to an .frm file. We need this definition to stay untouched.
> - */
Please move this comment to above where CONTEXT_ANALYSIS_ONLY_VIEW is
declared.
> - bool view_prepare_mode;
> +
> + uint8 context_analysis_only;
> bool safe_to_cache_query;
> bool subqueries, ignore;
> st_parsing_options parsing_options;
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2010-10-23 12:55:44 +0000
> +++ b/sql/sql_parse.cc 2010-12-07 14:32:55 +0000
> @@ -5865,13 +5865,6 @@ mysql_new_select(LEX *lex, bool move_dow
> DBUG_RETURN(1);
> }
> select_lex->nest_level= lex->nest_level;
> - /*
> - Don't evaluate this subquery during statement prepare even if
> - it's a constant one. The flag is switched off in the end of
> - mysqld_stmt_prepare.
> - */
Please move this comment to above where CONTEXT_ANALYSIS_ONLY_PREPARE is
declared.
> - if (thd->stmt_arena->is_stmt_prepare())
> - select_lex->uncacheable|= UNCACHEABLE_PREPARE;
> if (move_down)
> {
> SELECT_LEX_UNIT *unit;
>
Overall, looks good. Thanks for working on this!
Regards,
Davi
