List:Commits« Previous MessageNext Message »
From:Jørgen Løland Date:December 2 2009 11:42am
Subject:Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508
View as plain text  
Evgeny,

The patch looks correct and is approved, although I still don't think 
the function name reflects the behavior. See comment further down.

Evgeny Potemkin wrote:
> #At file:///work/bzrroot/48508-bug-5.0-bugteam/ based on
> revid:alexey.kopytov@stripped
> 
>  2850 Evgeny Potemkin	2009-12-01
>       Bug#48508: Crash on prepared statement re-execution.
>       
>       Actually there is two different bugs.
>       The first one caused crash on queries with WHERE condition over views
>       containing WHERE condition. A wrong check for prepared statement phase led
>       to items for view fields being allocated in the execution memory and freed
>       at the end of execution. Thus the optimized WHERE condition refers to
>       unallocated memory on the second execution and server crashed.
>       The second one caused by the Item_cond::compile function not saving changes
>       it made to the item tree. Thus on the next execution changes weren't
>       reverted and server crashed on dereferencing of unallocated space.
>       
>       The new helper function called is_stmt_prepare_or_first_stmt_execute
>       is added to the Query_arena class.
>       The find_field_in_view function now uses
>       is_stmt_prepare_or_first_stmt_execute() to check whether
>       newly created view items should be freed at the end of the query execution.
>       The Item_cond::compile function now saves changes it makes to item tree.
>      @ mysql-test/r/ps.result
>         Added a test case for the bug#48508.
>      @ mysql-test/t/ps.test
>         Added a test case for the bug#48508.
>      @ sql/item_cmpfunc.cc
>         Bug#48508: Crash on prepared statement re-execution.
>         The Item_cond::compile function now saves changes it makes to item tree.
>      @ sql/sql_base.cc
>         Bug#48508: Crash on prepared statement re-execution.
>         The find_field_in_view function now uses
>         is_stmt_prepare_or_first_stmt_execute() to check whether
>         newly created view items should be freed at the end of the query execution.
>      @ sql/sql_class.h
>         Bug#48508: Crash on prepared statement re-execution.
>         The Query_arena::is_stmt_prepare_or_first_sp_execute function now correctly
>         do its check.
(...)
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h	2009-10-20 04:42:10 +0000
> +++ b/sql/sql_class.h	2009-12-01 18:28:45 +0000
> @@ -759,6 +759,8 @@ public:
>    { return state == INITIALIZED_FOR_SP; }
>    inline bool is_stmt_prepare_or_first_sp_execute() const
>    { return (int)state < (int)PREPARED; }
> +  inline bool is_stmt_prepare_or_first_stmt_execute() const
> +  { return (int)state <= (int)PREPARED; }


This is nitpicking, but I still don't think the function name reflects 
the behavior. The name gives the impression that it returns true for PS 
prepare or PS first execute only, but it also returns true for SP first 
execute. However, I don't have a good suggestion for another name.


-- 
Jørgen Løland
Thread
bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin1 Dec
  • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Jørgen Løland2 Dec
    • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Øystein Grøvlen2 Dec
      • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin2 Dec
        • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Øystein Grøvlen2 Dec
          • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin2 Dec