List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:May 29 2012 3:57pm
Subject:bzr push into mysql-trunk branch (guilhem.bichot:3908 to 3909) Bug#14005353
View as plain text  
 3909 Guilhem Bichot	2012-05-29
      Follow-up after the fix for
      Bug#14005353 COLUMN CANNOT BE NULL ERROR WHEN EXECUTING TRIGGER WITH PK IN SUBQUERY
      We were reading uninitialized memory.
     @ sql/sql_lex.cc
        The fix for Bug#14005353 added into Item_field::fix_fields() some
        logic:
            if (thd->lex->current_select->with_sum_func && // (1)
                !thd->lex->current_select->group_list.elements) // (2)
              maybe_null= true;
        But fix_fields() is not only used in SELECT. LOAD DATA, ALTER TABLE
        use it (via setup_fields() or setup_order()). Those statements, like
        many others, and unlike SELECT, have
        thd->lex->current_select==&thd->lex->select_lex, and don't fully
        initialize the content of thd->lex->select_lex: they don't call
        mysql_init_select() or st_select_lex::init_select().
        Thus my new code was reading an uninitialized member (with_sum_func).
        Surely, it is bad design for a class (LEX) to let other pieces of code
        access a half-initialized object. If *(lex->current_select) is random
        then lex->current_select should not point there. It should rather
        point to NULL, and be set to non-NULL only when the SELECT_LEX is
        fully ready.
        But it seems to be intentional design that a SELECT_LEX can be
        half-initialized or fully initialized, see how we have:
        st_select_lex::init_query() and st_select_lex::init_select(). The
        former seems to be "what's needed for all queries" and the latter
        "what's additionally needed for SELECT-like queries". Some statements
        (like DELETE) do access a SELECT_LEX which has only gone through
        st_select_lex::init_query() (DELETE stores its table
        is current_select->tables_list).
        And note that lex_start() (always called) does some partial
        initializations of lex->select_lex too (for example it resets group_list);
        it is probably not its job.
        The fast fix is: reset with_sum_func in st_select_lex::init_query()
        (which is always called, by lex_start()).
        A better fix would be to redesign:
        - decide if we really have to skip st_select_lex::init_select() for
        some statements (i.e. produce a half-initialized object) (the reason
        might be performance)
        - if no, move st_select_lex::init_select() into
        st_select_lex::init_query()
        - if yes, split the object in two sub-objects
        - if something is not reliable, don't point to it; rather use a NULL
        pointer, so the rest of code knows that there is nothing to be found
        there.
        I will file a bug# or wl# for that.

    modified:
      sql/sql_lex.cc
 3908 Nuno Carvalho	2012-05-29
      Added waiting conditions to rpl_semi_sync test when semi sync master 
      status depend on round trip to slave to avoid test failures (result
      content mismatch) on slow platforms.

    modified:
      mysql-test/suite/rpl/t/rpl_semi_sync.test
=== modified file 'sql/sql_lex.cc'
--- a/sql/sql_lex.cc	2012-05-28 09:09:33 +0000
+++ b/sql/sql_lex.cc	2012-05-29 15:56:12 +0000
@@ -1787,6 +1787,7 @@ void st_select_lex::init_query()
   select_list_tables= 0;
   m_non_agg_field_used= false;
   m_agg_func_used= false;
+  with_sum_func= false;
 }
 
 void st_select_lex::init_select()

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (guilhem.bichot:3908 to 3909) Bug#14005353Guilhem Bichot29 May