List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:January 12 2011 1:47pm
Subject:bzr push into mysql-next-mr-bugfixing branch (guilhem:3248 to 3249)
View as plain text  
 3249 Guilhem Bichot	2011-01-12
      Review comments. In DBUG, "opt" instead of "opt_trace".
     @ WL4800_TODO.txt
        updates to TODO list:
        - first sentence was wrong
        - bug with views has been fixed recently
     @ libmysqld/CMakeLists.txt
        no .h is needed in this list
     @ sql/CMakeLists.txt
        no .h is needed in this list
     @ sql/opt_range.cc
        avoid goto
     @ sql/opt_trace.cc
        use "opt" in DBUG instead of "opt_trace"

    modified:
      WL4800_TODO.txt
      libmysqld/CMakeLists.txt
      mysys/my_getopt.c
      sql/CMakeLists.txt
      sql/opt_range.cc
      sql/opt_trace.cc
      sql/opt_trace.h
      sql/opt_trace2server.cc
 3248 Jorgen Loland	2011-01-11
      WL4800 - review comment
      
      enum variable does not need a typename

    modified:
      sql/sql_select.cc
=== modified file 'WL4800_TODO.txt'
--- a/WL4800_TODO.txt	2011-01-03 13:20:46 +0000
+++ b/WL4800_TODO.txt	2011-01-12 13:44:58 +0000
@@ -1,9 +1,6 @@
 Short-term TODO list to remember for WL#4800
 ============================================
 
-void QUICK_RANGE_SELECT::dbug_dump(int indent, bool verbose)
-doesn't use "verbose".
-
 see all added @todo
 
 My command to update the HTML Doxygen is:
@@ -44,9 +41,6 @@ disable tracing there? see optimizer_tra
 maybe looked at this in his in-review subquery tracing patch.
 
 Push already written patches for
-- when view is merged, its WHERE clause is not seen in expanded query
-(solution: don't print expanded query in open_tables() but in
-JOIN::prepare(), when the clause has finally been merged)
 - some LEFT JOIN trigcond conditions may not be shown
 
 The optimizer may have second thoughts about which access method to
@@ -59,5 +53,3 @@ range access anyway.
 
 Make --opt-trace-protocol dump traces to a separate file so that mtr
 can run with it without failing all tests.
-
-dbug tags: replace opt_trace with opt

=== modified file 'libmysqld/CMakeLists.txt'
--- a/libmysqld/CMakeLists.txt	2010-10-21 15:59:05 +0000
+++ b/libmysqld/CMakeLists.txt	2011-01-12 13:44:58 +0000
@@ -53,7 +53,7 @@ SET(SQL_EMBEDDED_SOURCES emb_qcache.cc l
            ../sql/item_xmlfunc.cc ../sql/key.cc ../sql/lock.cc ../sql/log.cc 
            ../sql/log_event.cc ../sql/mf_iocache.cc ../sql/my_decimal.cc 
            ../sql/net_serv.cc ../sql/opt_range.cc ../sql/opt_sum.cc 
-           ../sql/opt_trace.cc ../sql/opt_trace.h ../sql/opt_trace2server.cc
+           ../sql/opt_trace.cc ../sql/opt_trace2server.cc
            ../sql/parse_file.cc ../sql/procedure.cc ../sql/protocol.cc 
            ../sql/records.cc ../sql/rpl_filter.cc
            ../sql/rpl_record.cc ../sql/sha2.cc ../sql/des_key_file.cc

=== modified file 'mysys/my_getopt.c'
--- a/mysys/my_getopt.c	2010-10-21 20:24:51 +0000
+++ b/mysys/my_getopt.c	2011-01-12 13:44:58 +0000
@@ -877,6 +877,11 @@ static longlong getopt_ll(char *arg, con
 }
 
 
+/**
+  Maximum possible value for an integer GET_* variable type
+  @param  var_type  type of integer variable (GET_*)
+  @returns  maximum possible value for this type
+ */
 ulonglong max_of_int_range(int var_type)
 {
   switch (var_type)

=== modified file 'sql/CMakeLists.txt'
--- a/sql/CMakeLists.txt	2010-09-27 09:26:39 +0000
+++ b/sql/CMakeLists.txt	2011-01-12 13:44:58 +0000
@@ -49,7 +49,7 @@ SET (SQL_SOURCE
                message.h mf_iocache.cc my_decimal.cc ../sql-common/my_time.c
                mysqld.cc net_serv.cc  keycaches.cc
                opt_range.cc opt_range.h opt_sum.cc 
-               opt_trace.cc opt_trace.h opt_trace2server.cc
+               opt_trace.cc opt_trace2server.cc
                ../sql-common/pack.c parse_file.cc password.c procedure.cc 
                protocol.cc records.cc set_var.cc 
                sp.cc sp_cache.cc sp_head.cc sp_pcontext.cc 

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2011-01-10 14:55:06 +0000
+++ b/sql/opt_range.cc	2011-01-12 13:44:58 +0000
@@ -10111,23 +10111,23 @@ get_best_group_min_max(PARAM *param, SEL
   /* Perform few 'cheap' tests whether this access method is applicable. */
   if (!join)
   {
-    cause= "no_join";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "no_join");
+    DBUG_RETURN(NULL);
   }
   if (join->tables != 1)   /* The query must reference one table. */
   {
-    cause= "not_single_table";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "not_single_table");
+    DBUG_RETURN(NULL);
   }
   if (join->select_lex->olap == ROLLUP_TYPE) /* Check (B3) for ROLLUP */
   {
-    cause= "rollup";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "rollup");
+    DBUG_RETURN(NULL);
   }
   if (table->s->keys == 0)        /* There are no indexes to use. */
   {
-    cause= "no_index";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "no_index");
+    DBUG_RETURN(NULL);
   }
 
   /* Check (SA1,SA4) and store the only MIN/MAX argument - the C attribute.*/
@@ -10140,8 +10140,9 @@ get_best_group_min_max(PARAM *param, SEL
       (!join->select_distinct) &&
       !is_agg_distinct)
   {
-    cause= "not_group_by_or_distinct";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).
+      add_alnum("cause", "not_group_by_or_distinct");
+    DBUG_RETURN(NULL);
   }
   /* Analyze the query in more detail. */
 
@@ -10161,8 +10162,9 @@ get_best_group_min_max(PARAM *param, SEL
         continue;
       else
       {
-        cause= "not_applicable_aggregate_function";
-        goto return_null_not_chosen_cause;
+        trace_group.add("chosen", false).
+          add_alnum("cause", "not_applicable_aggregate_function");
+        DBUG_RETURN(NULL);
       }
 
       /* The argument of MIN/MAX. */
@@ -10196,8 +10198,9 @@ get_best_group_min_max(PARAM *param, SEL
   {
     if ((*tmp_group->item)->real_item()->type() != Item::FIELD_ITEM)
     {
-      cause= "group_field_is_expression";
-      goto return_null_not_chosen_cause;
+      trace_group.add("chosen", false).
+        add_alnum("cause", "group_field_is_expression");
+      DBUG_RETURN(NULL);
     }
   }
 
@@ -10546,8 +10549,9 @@ get_best_group_min_max(PARAM *param, SEL
                                       (index_info->flags & HA_SPATIAL) ?
                                       Field::itMBR : Field::itRAW))
   {
-    cause= "unsupported_predicate_on_agg_attribute";
-    goto return_null_not_usable_cause;
+    trace_group.add("usable", false).
+      add_alnum("cause", "unsupported_predicate_on_agg_attribute");
+    DBUG_RETURN(NULL);
   }
   /* The query passes all tests, so construct a new TRP object. */
   read_plan= new (param->mem_root)
@@ -10579,16 +10583,6 @@ get_best_group_min_max(PARAM *param, SEL
   }
 
   DBUG_RETURN(read_plan);
-
-return_null_not_chosen_cause:
-  DBUG_ASSERT(cause);
-  trace_group.add("chosen", false).add_alnum("cause", cause);
-  DBUG_RETURN(NULL);
-
-return_null_not_usable_cause:
-  DBUG_ASSERT(cause);
-  trace_group.add("usable", false).add_alnum("cause", cause);
-  DBUG_RETURN(NULL);
 }
 
 

=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc	2010-11-24 18:54:26 +0000
+++ b/sql/opt_trace.cc	2011-01-12 13:44:58 +0000
@@ -37,7 +37,7 @@ bool Opt_trace_struct::dbug_assert_on_sy
 
 void Opt_trace_struct::syntax_error(const char *key)
 {
-  DBUG_PRINT("opt_trace", ("syntax error key: %s", key));
+  DBUG_PRINT("opt", ("syntax error key: %s", key));
   DBUG_ASSERT(stmt->support_I_S);
   DBUG_ASSERT(!dbug_assert_on_syntax_error);
   /*
@@ -66,7 +66,7 @@ void Opt_trace_struct::do_construct(Opt_
   saved_key= key;
   requires_key= requires_key_arg;
   /* DBUG_PRINT supports NULL in %s */
-  DBUG_PRINT("opt_trace", ("%s: starting %s", key, types[requires_key]));
+  DBUG_PRINT("opt", ("%s: starting %s", key, types[requires_key]));
   stmt= stmt_arg;
   parent= parent_arg;
   DBUG_ASSERT(parent == NULL || stmt == parent->stmt);
@@ -180,7 +180,7 @@ void Opt_trace_struct::add_struct(const 
 
 void Opt_trace_struct::do_destruct(void)
 {
-  DBUG_PRINT("opt_trace", ("%s: ending %s", saved_key, types[requires_key]));
+  DBUG_PRINT("opt", ("%s: ending %s", saved_key, types[requires_key]));
   DBUG_ASSERT(started);
   /*
     Note:
@@ -215,7 +215,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
                                            bool escape)
 {
   DBUG_ASSERT(started);
-  DBUG_PRINT("opt_trace", ("%s: \"%.*s\"", key, (int)val_length, val));
+  DBUG_PRINT("opt", ("%s: \"%.*s\"", key, (int)val_length, val));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -285,7 +285,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
 Opt_trace_struct& Opt_trace_struct::do_add(const char *key, bool val)
 {
   DBUG_ASSERT(started);
-  DBUG_PRINT("opt_trace", ("%s: %d", key, (int)val));
+  DBUG_PRINT("opt", ("%s: %d", key, (int)val));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -303,7 +303,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
   DBUG_ASSERT(started);
   char buf[22];
   llstr(val, buf);
-  DBUG_PRINT("opt_trace", ("%s: %s", key, buf));
+  DBUG_PRINT("opt", ("%s: %s", key, buf));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -318,7 +318,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
   DBUG_ASSERT(started);
   char buf[22];
   ullstr(val, buf);
-  DBUG_PRINT("opt_trace", ("%s: %s", key, buf));
+  DBUG_PRINT("opt", ("%s: %s", key, buf));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -333,7 +333,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
   DBUG_ASSERT(started);
   char buf[32];
   my_snprintf(buf, sizeof(buf), "%g", val);
-  DBUG_PRINT("opt_trace", ("%s: %s", key, buf));
+  DBUG_PRINT("opt", ("%s: %s", key, buf));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -345,7 +345,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
 
 Opt_trace_struct& Opt_trace_struct::do_add_null(const char *key)
 {
-  DBUG_PRINT("opt_trace", ("%s: null", key));
+  DBUG_PRINT("opt", ("%s: null", key));
   if (!stmt->support_I_S)
     return *this;
   stmt->separator();
@@ -413,7 +413,7 @@ Opt_trace_struct *Opt_trace_context::get
 
 void Opt_trace_context::link_to_shown(Opt_trace_stmt *stmt)
 {
-  DBUG_PRINT("opt_trace", ("Opt_trace_context::link_to_shown %p", stmt));
+  DBUG_PRINT("opt", ("Opt_trace_context::link_to_shown %p", stmt));
   stmt->prev= stmt->next= NULL;
   if (oldest_stmt_to_show == NULL)
   {
@@ -431,7 +431,7 @@ void Opt_trace_context::link_to_shown(Op
 
 void Opt_trace_context::unlink_from_shown(Opt_trace_stmt *stmt)
 {
-  DBUG_PRINT("opt_trace", ("Opt_trace_context::unlink_from_shown %p", stmt));
+  DBUG_PRINT("opt", ("Opt_trace_context::unlink_from_shown %p", stmt));
   if (stmt == oldest_stmt_to_show)
     oldest_stmt_to_show= stmt->next;
   else
@@ -445,7 +445,7 @@ void Opt_trace_context::unlink_from_show
 
 void Opt_trace_context::link_to_del(Opt_trace_stmt *stmt)
 {
-  DBUG_PRINT("opt_trace", ("Opt_trace_context::link_to_del %p", stmt));
+  DBUG_PRINT("opt", ("Opt_trace_context::link_to_del %p", stmt));
   stmt->prev= stmt->next= NULL;
   if (stmt_to_del != NULL)
   {
@@ -458,7 +458,7 @@ void Opt_trace_context::link_to_del(Opt_
 
 void Opt_trace_context::unlink_from_del(Opt_trace_stmt *stmt)
 {
-  DBUG_PRINT("opt_trace", ("Opt_trace_context::unlink_from_del %p", stmt));
+  DBUG_PRINT("opt", ("Opt_trace_context::unlink_from_del %p", stmt));
   if (stmt->prev != NULL)
     stmt->prev->next= stmt->next;
   if (stmt == stmt_to_del)
@@ -598,15 +598,15 @@ bool Opt_trace_context::start(bool suppo
     /* If outside the offset/limit window, no need to support I_S */
     if (since_offset_0 < (ulong)offset)
     {
-      DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%llu) < offset(%lu)",
-                               (ulonglong)since_offset_0, offset));
+      DBUG_PRINT("opt", ("disabled: since_offset_0(%llu) < offset(%lu)",
+                         (ulonglong)since_offset_0, offset));
       new_stmt_support_I_S= false;
     }
     else if (since_offset_0 >= (ulong)(offset + limit))
     {
-      DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%llu) >="
-                               " offset(%lu) + limit(%lu)",
-                               (ulonglong)since_offset_0, offset, limit));
+      DBUG_PRINT("opt", ("disabled: since_offset_0(%llu) >="
+                         " offset(%lu) + limit(%lu)",
+                         (ulonglong)since_offset_0, offset, limit));
       new_stmt_support_I_S= false;
     }
     since_offset_0++;
@@ -614,8 +614,8 @@ bool Opt_trace_context::start(bool suppo
 
   Opt_trace_stmt *stmt= new Opt_trace_stmt(this, current_stmt_in_gen,
                                            new_stmt_support_I_S);
-  DBUG_PRINT("opt_trace",("new stmt %p support_I_S %d", stmt,
-                          (int)new_stmt_support_I_S));
+  DBUG_PRINT("opt",("new stmt %p support_I_S %d", stmt,
+                    (int)new_stmt_support_I_S));
   if (stmt == NULL)
     DBUG_RETURN(true);
 
@@ -704,8 +704,8 @@ size_t Opt_trace_context::allowed_mem_si
       break;
   }
   size_t rc= (mem_size <= max_mem_size) ? (max_mem_size - mem_size) : 0;
-  DBUG_PRINT("opt_trace", ("rc %llu max_mem_size %llu",
-                           (ulonglong)rc, (ulonglong)max_mem_size));
+  DBUG_PRINT("opt", ("rc %llu max_mem_size %llu",
+                     (ulonglong)rc, (ulonglong)max_mem_size));
   DBUG_RETURN(rc);
 }
 
@@ -809,7 +809,7 @@ void Opt_trace_stmt::end(void)
   depth= 0;
   started= false;
   /* Send the full nice trace to DBUG */
-  DBUG_EXECUTE("opt_trace",
+  DBUG_EXECUTE("opt",
                if (buffer.length() <= empty_trace_len)
                { /*  don't spam DBUG with empty trace */ }
                else

=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h	2011-01-10 14:55:06 +0000
+++ b/sql/opt_trace.h	2011-01-12 13:44:58 +0000
@@ -253,7 +253,7 @@ class st_select_lex;
 @endverbatim
 
   Thus, any optimizer trace operation, *even* if tracing is run-time disabled,
-  has an implicit DBUG_PRINT("opt_trace",...) inside. This way, only the
+  has an implicit DBUG_PRINT("opt",...) inside. This way, only the
   second line above is needed, and several DBUG_PRINT() could be removed from
   the Optimizer code.
   When tracing is run-time disabled, in a debug binary, traces are still

=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc	2011-01-03 20:36:18 +0000
+++ b/sql/opt_trace2server.cc	2011-01-12 13:44:58 +0000
@@ -106,7 +106,7 @@ bool opt_trace_start(THD *thd, const TAB
   bool need_it_for_I_S= (var & Opt_trace_context::FLAG_ENABLED);
   bool need_it_for_dbug= false, allocated_here= false;
   /* This will be triggered if --debug or --debug=d:opt_trace is used */
-  DBUG_EXECUTE("opt_trace", need_it_for_dbug= true;);
+  DBUG_EXECUTE("opt", need_it_for_dbug= true;);
   if (!(need_it_for_I_S || need_it_for_dbug))
     goto disable;
   if (!sql_command_can_be_traced(sql_command))

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-next-mr-bugfixing branch (guilhem:3248 to 3249) Guilhem Bichot12 Jan