List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:March 31 2011 3:10pm
Subject:[Resend] bzr commit into mysql-trunk branch (guilhem.bichot:3285)
View as plain text  
[This commit e-mail is a repeat.]

#At file:///home/mysql_src/bzrrepos_new/new_stuf_wl4800/ based on
revid:guilhem.bichot@stripped

 3285 Guilhem Bichot	2011-03-24
      Fix for the following bug.
      If a SQL command is not traced because it's not part of the set of
      trace-able commands, we wanted to disable tracing of its substatements
      (see opt_trace2server.cc).
      But this didn't work, because of the optimization that if
      @@optimizer_trace has enabled=off, we should not create any
      Opt_trace_context/Opt_trace_stmt, for efficiency reasons. Remembering that
      "tracing of children statements of this top SQL command should be disabled"
      requires an Opt_trace_stmt for the top SQL command, this Opt_trace_stmt
      having support_I_S==NO_FOR_THIS_AND_CHILDREN. But in the optimized case,
      we have no Opt_trace_stmt to put this information...
      We here fix the bug and still keep the optimization.
      The fix is based on the observation that if we have some nested
      Opt_trace_stmt-s (like, routine having substatements), and write the
      value of support_I_S for each of them, we have a sequence of
      [X1 - X2 - ... Xn ] - [NO_FOR_THIS_AND_CHILDREN repeated m times]
      where Xi is either YES_FOR_THIS or NO_FOR_THIS, and
      n and m may be zero. So instead of the m-long tail, it's enough to remember m.
      Thus, we do those changes:
      - the property "I_S is disabled for children" is not tracked by
      having support_I_S==NO_FOR_THIS_AND_CHILDREN anymore but rather by a global
      counter: each time we want to disable I_S for children, we increment it.
      We decrement the counter in the corresponding "re-enabling" code.
      - we test this counter instead of testing
      current_stmt_in_gen->support_I_S==NO_FOR_THIS_AND_CHILDREN
      - because this global counter exists at all time, this solves
      the bug.
      Of course, a global counter is not fully correct as it is
      not encapsulated, and not thread-safe; it's used here as it made the patch
      very simple to understand. In a next patch, we will thus move the global counter
      closer to the Opt_trace_context class.
     @ mysql-test/include/optimizer_trace2.inc
        Test for the bug.
     @ mysql-test/r/optimizer_trace2_no_prot.result
        testcase for bug: with and without the code change, we see that
        substatements of "DO f1()" are traced even though DO is not.
        PREPARE and EXECUTE are not traced anymore (but the
        statement which they prepare or execute, is traced).
     @ sql/sql_parse.cc
        As a bonus consequence we don't need to have CF_OPTIMIZER_TRACE
        for SQL PREPARE/EXECUTE. This is good because now we have only
        one trace for "PREPARE SELECT", just like we always had only
        one trace for preparing a SELECT with the C client API.

    modified:
      mysql-test/include/optimizer_trace2.inc
      mysql-test/r/optimizer_trace2_no_prot.result
      mysql-test/r/optimizer_trace2_ps_prot.result
      sql/opt_trace.cc
      sql/opt_trace.h
      sql/opt_trace2server.cc
      sql/sql_parse.cc
      sql/sql_prepare.cc
=== modified file 'mysql-test/include/optimizer_trace2.inc'
--- a/mysql-test/include/optimizer_trace2.inc	2011-03-14 14:33:12 +0000
+++ b/mysql-test/include/optimizer_trace2.inc	2011-03-24 10:33:38 +0000
@@ -219,3 +219,26 @@ select * from t1;
 select @a,@b;
 drop procedure p1;
 drop table t1;
+
+--echo
+--echo # Test that in a non-traced SQL command, substatements are never
+--echo # traced
+--echo
+
+create table t1(a int);
+insert into t1 values(1),(2);
+delimiter |;
+create function f1() returns int
+begin
+  declare b int;
+  select 48 into b from dual;
+  select a into b from t1 limit 1;
+  insert into t1 values(b*10);
+  return 36;
+end|
+delimiter ;|
+set optimizer_trace_offset=0, optimizer_trace_limit=100;
+do f1(); # DO is never traced
+select * from information_schema.OPTIMIZER_TRACE;
+drop function f1;
+drop table t1;

=== modified file 'mysql-test/r/optimizer_trace2_no_prot.result'
--- a/mysql-test/r/optimizer_trace2_no_prot.result	2011-03-21 17:55:41 +0000
+++ b/mysql-test/r/optimizer_trace2_no_prot.result	2011-03-24 10:33:38 +0000
@@ -1477,3 +1477,23 @@ select @a,@b;
 3	2
 drop procedure p1;
 drop table t1;
+
+# Test that in a non-traced SQL command, substatements are never
+# traced
+
+create table t1(a int);
+insert into t1 values(1),(2);
+create function f1() returns int
+begin
+declare b int;
+select 48 into b from dual;
+select a into b from t1 limit 1;
+insert into t1 values(b*10);
+return 36;
+end|
+set optimizer_trace_offset=0, optimizer_trace_limit=100;
+do f1();
+select * from information_schema.OPTIMIZER_TRACE;
+QUERY	TRACE	MISSING_BYTES_BEYOND_MAX_MEM_SIZE
+drop function f1;
+drop table t1;

=== modified file 'mysql-test/r/optimizer_trace2_ps_prot.result'
--- a/mysql-test/r/optimizer_trace2_ps_prot.result	2011-03-21 17:55:41 +0000
+++ b/mysql-test/r/optimizer_trace2_ps_prot.result	2011-03-24 10:33:38 +0000
@@ -1497,3 +1497,23 @@ select @a,@b;
 3	2
 drop procedure p1;
 drop table t1;
+
+# Test that in a non-traced SQL command, substatements are never
+# traced
+
+create table t1(a int);
+insert into t1 values(1),(2);
+create function f1() returns int
+begin
+declare b int;
+select 48 into b from dual;
+select a into b from t1 limit 1;
+insert into t1 values(b*10);
+return 36;
+end|
+set optimizer_trace_offset=0, optimizer_trace_limit=100;
+do f1();
+select * from information_schema.OPTIMIZER_TRACE;
+QUERY	TRACE	MISSING_BYTES_BEYOND_MAX_MEM_SIZE
+drop function f1;
+drop table t1;

=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc	2011-03-22 10:08:27 +0000
+++ b/sql/opt_trace.cc	2011-03-24 10:33:38 +0000
@@ -26,6 +26,8 @@
 
 #ifdef OPTIMIZER_TRACE
 
+int glob_recursive_disable_I_S= 0;
+
 /**
   @class Opt_trace_stmt
 
@@ -214,7 +216,8 @@ private:
       */
       return true;
     }
-    support_I_S= NO_FOR_THIS_AND_CHILDREN;
+    support_I_S= NO_FOR_THIS;
+    ++glob_recursive_disable_I_S;
     return false;
   }
   /**
@@ -228,6 +231,7 @@ private:
   void restore_I_S()
   {
     support_I_S= stack_of_values_of_support_I_S.pop();
+    --glob_recursive_disable_I_S;
   }
 
   /// A wrapper of class String, for storing query or trace.
@@ -584,7 +588,8 @@ bool Opt_trace_stmt::open_struct(const c
       }
       if (unlikely(stack_of_values_of_support_I_S.append(support_I_S)))
         goto err;
-      support_I_S= NO_FOR_THIS_AND_CHILDREN;
+      support_I_S= NO_FOR_THIS;
+      ++glob_recursive_disable_I_S;
       has_disabled_I_S= true;
     }
     else
@@ -648,7 +653,10 @@ void Opt_trace_stmt::close_struct(const 
     }
   }
   if (d.has_disabled_I_S)
+  {
     support_I_S= stack_of_values_of_support_I_S.pop();
+    --glob_recursive_disable_I_S;
+  }
 }
 
 
@@ -1011,15 +1019,14 @@ bool Opt_trace_context::start(enum enum_
     starting execution of a sub-statement of a stored routine (CALL has
     tracing enabled too).
   */
-  if (current_stmt_in_gen != NULL &&
-      current_stmt_in_gen->get_support_I_S() == NO_FOR_THIS_AND_CHILDREN)
+  if (glob_recursive_disable_I_S != 0)
   {
     /*
       Tracing is strictly disabled by the caller. Thus don't listen to any
       request from the user for enabling tracing or changing settings (offset
       etc). Doing otherwise would surely bring a problem.
     */
-    new_stmt_support_I_S= NO_FOR_THIS_AND_CHILDREN;
+    new_stmt_support_I_S= NO_FOR_THIS;
   }
   else
   {

=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h	2011-03-22 10:08:27 +0000
+++ b/sql/opt_trace.h	2011-03-24 10:33:38 +0000
@@ -26,6 +26,8 @@ struct st_schema_table;
 struct TABLE_LIST;
 struct TABLE;
 
+extern int glob_recursive_disable_I_S;
+
 /**
    @file
    API for the Optimizer trace (WL#5257)

=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc	2011-03-22 10:08:27 +0000
+++ b/sql/opt_trace2server.cc	2011-03-24 10:33:38 +0000
@@ -110,7 +110,9 @@ bool opt_trace_start(THD *thd, const TAB
   /* This will be triggered if --debug or --debug=d:opt_trace is used */
   DBUG_EXECUTE("opt", need_it_for_dbug= true;);
   // First step, decide on what type of I_S support we want
-  if (!sql_command_can_be_traced(sql_command))
+  if (unlikely(support_I_S == YES_FOR_THIS) &&
+      (!sql_command_can_be_traced(sql_command) ||
+       list_has_optimizer_trace_table(tbl)))
   {
     /*
       We also constraint its substatements to do no tracing. This is an extra
@@ -126,18 +128,13 @@ bool opt_trace_start(THD *thd, const TAB
       This constraint forces us to enable trace for CALL because otherwise,
       execution of a stored procedure would not be traced. Same for SQL PREPARE
       and SQL EXECUTE.
-    */
-    support_I_S= NO_FOR_THIS_AND_CHILDREN;
-  }
-  else if (unlikely(support_I_S == YES_FOR_THIS &&
-                    list_has_optimizer_trace_table(tbl)))
-  {
-    /*
+
       Note that list_has_optimizer_trace_table() is an expensive function
       (scanning the list of all used tables, doing checks on their names) but
       we call it only if @@optimizer_trace has enabled=on.
     */
     support_I_S= NO_FOR_THIS;
+    ++glob_recursive_disable_I_S;
   }
   /*
     Second step, decide on optimizations possible to realize this I_S support.

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2011-03-21 17:55:41 +0000
+++ b/sql/sql_parse.cc	2011-03-24 10:33:38 +0000
@@ -2019,6 +2019,7 @@ mysql_execute_command(THD *thd)
 
   status_var_increment(thd->status_var.com_stat[lex->sql_command]);
 
+  const int saved_glob_rec= glob_recursive_disable_I_S;
   const bool started_optimizer_trace= opt_trace_start(thd, all_tables,
                                                       lex->sql_command);
   if (started_optimizer_trace)
@@ -4488,6 +4489,7 @@ finish:
   trace_command_steps.end();
   trace_command.end(); // must be closed before trace is ended below
   opt_trace_end(thd, started_optimizer_trace);
+  glob_recursive_disable_I_S= saved_glob_rec;
 
   DBUG_RETURN(res || thd->is_error());
 }

=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc	2011-03-21 17:55:41 +0000
+++ b/sql/sql_prepare.cc	2011-03-24 10:33:38 +0000
@@ -1969,6 +1969,7 @@ static bool check_prepared_statement(Pre
     For the optimizer trace, this is the symmetric, for statement preparation,
     of what is done at statement execution (in mysql_execute_command()).
   */
+  const int saved_glob_rec= glob_recursive_disable_I_S;
   const bool started_optimizer_trace= opt_trace_start(thd, tables,
                                                       sql_command);
   if (started_optimizer_trace)
@@ -2118,6 +2119,8 @@ static bool check_prepared_statement(Pre
 end:
   trace_command_steps.end();
   trace_command.end(); // must be closed before trace is ended below
+
+  glob_recursive_disable_I_S= saved_glob_rec;
   opt_trace_end(thd, started_optimizer_trace);
   DBUG_RETURN(res);
 }


Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110324103338-7qvh16uin7d7f374.bundle
Thread
[Resend] bzr commit into mysql-trunk branch (guilhem.bichot:3285) Guilhem Bichot31 Mar