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 Bichot | 12 Jan |
