#At file:///export/home/didrik/repo/next-mr-opt-backporting-wl4800/ based on revid:jorgen.loland@stripped21073433-nqg9a60zdwbb0tmy
3243 Tor Didriksen 2011-01-11
WL#5257 Review comments.
More comments in followup email ...
@ sql/opt_range.cc
@ sql/opt_trace.cc
added:
unittest/gunit/opt_notrace-t.cc
modified:
sql/opt_range.cc
sql/opt_trace.cc
sql/opt_trace.h
sql/opt_trace2server.cc
sql/sys_vars.cc
unittest/gunit/CMakeLists.txt
unittest/gunit/opt_trace-t.cc
=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc 2010-12-21 07:34:33 +0000
+++ b/sql/opt_range.cc 2011-01-11 08:15:46 +0000
@@ -793,8 +793,10 @@ static
TRP_GROUP_MIN_MAX *get_best_group_min_max(PARAM *param, SEL_TREE *tree,
double read_time);
#if !defined(DBUG_OFF) || defined(OPTIMIZER_TRACE)
+// print_sel_tree declared 'static' but never defined
static void print_sel_tree(PARAM *param, SEL_TREE *tree, key_map *tree_map,
const char *msg);
+// print_ror_scans_arr declared 'static' but never defined
static void print_ror_scans_arr(TABLE *table, const char *msg,
struct st_ror_scan_info **start,
struct st_ror_scan_info **end);
@@ -12393,6 +12395,7 @@ print_multiple_key_values(KEY_PART *key_
dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets);
}
+// print_quick defined but not used.
static void print_quick(QUICK_SELECT_I *quick, const key_map *needed_reg)
{
char buf[MAX_KEY/8+1];
=== 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-11 08:15:46 +0000
@@ -65,7 +65,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]));
stmt= stmt_arg;
parent= parent_arg;
@@ -74,8 +74,7 @@ void Opt_trace_struct::do_construct(Opt_
#ifndef DBUG_OFF
previous_key[0]= 0;
#endif
- save_stmt_support_I_S=
- stmt->support_I_S;
+ save_stmt_support_I_S= stmt->support_I_S;
if (save_stmt_support_I_S &&
(feature & stmt->ctx->features) == 0)
{
@@ -122,9 +121,9 @@ void Opt_trace_struct::do_construct(Opt_
if (max_size >= safety_margin)
{
max_size-= safety_margin;
- if (new_size > max_size) // don't pre-allocate more than the limit
+ if (new_size > max_size) // Don't pre-allocate more than the limit.
new_size= max_size;
- if (new_size >= alloced) // never shrink string
+ if (new_size >= alloced) // Never shrink string.
stmt->buffer.realloc(new_size);
}
}
@@ -141,7 +140,7 @@ void Opt_trace_struct::do_construct(Opt_
void Opt_trace_struct::add_key_name(const char *key)
{
- /* user should always add to the innermost open object, not outside */
+ // User should always add to the innermost open object, not outside.
DBUG_ASSERT(stmt->current_struct == this);
bool has_key= key != NULL;
if (unlikely(has_key != requires_key))
@@ -178,18 +177,18 @@ void Opt_trace_struct::add_struct(const
}
-void Opt_trace_struct::do_destruct(void)
+void Opt_trace_struct::do_destruct()
{
DBUG_PRINT("opt_trace", ("%s: ending %s", saved_key, types[requires_key]));
DBUG_ASSERT(started);
/*
Note:
- the Server code invokes Opt_trace_struct's constructor/destructor, and so
- stmt's current_struct is necessarily set by Opt_trace_struct's
- constructor/destructor
+ stmt's current_struct is necessarily set by Opt_trace_struct's
+ constructor/destructor
- the Server code invokes Opt_trace_context start/end() functions, and so
- context's current_stmt_in_gen is set in start()/end() (which is clean: object
- sets _its_ variables, does not let its child do it).
+ context's current_stmt_in_gen is set in start()/end() (which is clean:
+ object sets _its_ variables, does not let its child do it).
*/
stmt->current_struct= parent;
DBUG_ASSERT(parent == NULL || stmt == parent->stmt);
@@ -236,7 +235,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
("select" and other language keywords). If we directly store that mix in a
UTF8 column, the query_charset piece causes an issue:
Field_blob::store() will truncate the trace at first unrecognized
- character. At worse, a single bad character in the expanded query makes
+ character. Even worse, a single bad character in the expanded query makes
all the rest of the trace be lost.
To work around the bug, we force a conversion from UTF8 to UTF8, which
will replace any non-UTF8 characters with '?'. Some query_charset
@@ -250,7 +249,6 @@ Opt_trace_struct& Opt_trace_struct::do_a
stmt->buffer.append_escaped(val, val_length);
else
{
-
uint32 new_length= system_charset_info->mbmaxlen * val_length;
char *utf8_str= (char *)my_malloc(new_length, MYF(0));
if (utf8_str == NULL)
@@ -281,6 +279,10 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
}
+namespace {
+LEX_CSTRING readables[]= { { STRING_WITH_LEN("false") },
+ { STRING_WITH_LEN("true") } };
+}
Opt_trace_struct& Opt_trace_struct::do_add(const char *key, bool val)
{
@@ -290,8 +292,6 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- LEX_CSTRING readables[]= { { STRING_WITH_LEN("false") },
- { STRING_WITH_LEN("true") } };
const LEX_CSTRING *readable= &readables[(int)val];
stmt->buffer.append(readable->str, readable->length);
return *this;
@@ -301,7 +301,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
Opt_trace_struct& Opt_trace_struct::do_add(const char *key, longlong val)
{
DBUG_ASSERT(started);
- char buf[22];
+ char buf[22]; // magic constant
llstr(val, buf);
DBUG_PRINT("opt_trace", ("%s: %s", key, buf));
if (!stmt->support_I_S)
@@ -331,7 +331,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
Opt_trace_struct& Opt_trace_struct::do_add(const char *key, double val)
{
DBUG_ASSERT(started);
- char buf[32];
+ char buf[32]; // magic constant
my_snprintf(buf, sizeof(buf), "%g", val);
DBUG_PRINT("opt_trace", ("%s: %s", key, buf));
if (!stmt->support_I_S)
@@ -384,17 +384,18 @@ const char *Opt_trace_context::feature_n
"greedy_search", "range_optimizer", "default", NullS
};
-const Opt_trace_context::feature_value Opt_trace_context::FEATURES_DEFAULT=
+const Opt_trace_context::feature_value
+Opt_trace_context::default_features=
Opt_trace_context::feature_value(Opt_trace_context::GREEDY_SEARCH |
Opt_trace_context::RANGE_OPTIMIZER);
-Opt_trace_context::Opt_trace_context(void):
+Opt_trace_context::Opt_trace_context():
oldest_stmt_to_show(NULL), newest_stmt_to_show(NULL), stmt_to_del(NULL),
since_offset_0(0), current_stmt_in_gen(NULL), cannot_change_settings(false)
{}
-Opt_trace_context::~Opt_trace_context(void)
+Opt_trace_context::~Opt_trace_context()
{
/* There may well be some few ended traces left: */
purge(true);
@@ -405,7 +406,7 @@ Opt_trace_context::~Opt_trace_context(vo
}
-Opt_trace_struct *Opt_trace_context::get_current_struct(void) const
+Opt_trace_struct *Opt_trace_context::get_current_struct() const
{
return current_stmt_in_gen->current_struct;
}
@@ -476,13 +477,13 @@ void Opt_trace_context::purge(bool purge
/* This case is managed in @c Opt_trace_context::start() */
DBUG_VOID_RETURN;
}
- ulong i= 0; // distance to the newest trace (unit: 'list elements')
+ long i= 0; // distance to the newest trace (unit: 'list elements')
Opt_trace_stmt *stmt;
/* Start from the newest traces, scroll back in time */
for (stmt= newest_stmt_to_show ; stmt != NULL ; )
{
i++;
- if (!purge_all && i <= (ulong)(-offset))
+ if (!purge_all && i <= (-offset))
{
/* OFFSET mandates that this trace should be kept; move to previous */
stmt= stmt->prev;
@@ -606,6 +607,7 @@ bool Opt_trace_context::start(bool suppo
{
DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%llu) >="
" offset(%lu) + limit(%lu)",
+ // Both offset and limit are signed.
(ulonglong)since_offset_0, offset, limit));
new_stmt_support_I_S= false;
}
@@ -616,6 +618,7 @@ bool Opt_trace_context::start(bool suppo
new_stmt_support_I_S);
DBUG_PRINT("opt_trace",("new stmt %p support_I_S %d", stmt,
(int)new_stmt_support_I_S));
+ // new cannot return NULL.
if (stmt == NULL)
DBUG_RETURN(true);
@@ -645,7 +648,7 @@ bool Opt_trace_context::start(bool suppo
}
-void Opt_trace_context::end(void)
+void Opt_trace_context::end()
{
if (current_stmt_in_gen != NULL)
{
@@ -683,26 +686,30 @@ void Opt_trace_context::end(void)
}
-size_t Opt_trace_context::allowed_mem_size(const Opt_trace_stmt *for_stmt)
- const
+namespace {
+size_t allowed_mem_size(const Opt_trace_stmt *stmt_start,
+ const Opt_trace_stmt *for_stmt)
{
- DBUG_ENTER("Opt_trace_context::allowed_mem_size");
size_t mem_size= 0;
- /* Even to-be-deleted traces use memory, so consider them in sum */
- for (Opt_trace_stmt *stmt_start= newest_stmt_to_show ;
- ;
- stmt_start= stmt_to_del)
+ for (const Opt_trace_stmt *stmt= stmt_start ;
+ stmt != NULL ; stmt= stmt->prev_())
{
- for (Opt_trace_stmt *stmt= stmt_start ; stmt != NULL ; )
- {
- if (stmt != for_stmt)
- mem_size+= stmt->buffer.alloced_length() +
- stmt->query_buffer.alloced_length();
- stmt= stmt->prev;
- }
- if (stmt_start == stmt_to_del)
- break;
+ if (stmt != for_stmt)
+ mem_size+= stmt->alloced_length();
}
+ return mem_size;
+}
+}
+
+
+size_t Opt_trace_context::allowed_mem_size(const Opt_trace_stmt *for_stmt) const
+{
+ DBUG_ENTER("Opt_trace_context::allowed_mem_size");
+ /* Even to-be-deleted traces use memory, so consider them in sum */
+ size_t mem_size=
+ ::allowed_mem_size(newest_stmt_to_show, for_stmt) +
+ ::allowed_mem_size(stmt_to_del, for_stmt);
+
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));
@@ -723,7 +730,7 @@ const char *Opt_trace_context::get_tail(
}
-void Opt_trace_context::reset(void)
+void Opt_trace_context::reset()
{
purge(true);
since_offset_0= 0;
@@ -750,20 +757,20 @@ void Opt_trace_iterator::operator++(int)
'limit'; if offset>=0, we stopped producing traces when 'limit' was
reached.
*/
- cursor= NULL; // like the 'end' iterator has
+ cursor= NULL;
}
row_count++;
}
/**
- @note because allocation is done in big chunks, buffer->Ptr[str_length]
+ @note Because allocation is done in big chunks, buffer->Ptr[str_length]
may be uninitialized while buffer->Ptr[allocated length] is 0, so we
must use c_ptr_safe() as we want a NUL-terminated string (which is easier
to manipulate in a debugger, or to compare in unit tests with
- ASSERT_STREQ).
+ EXPECT_STREQ).
c_ptr_safe() may realloc an empty String from 0 bytes to 8 bytes,
- when it adds the closing NUL.
+ when it adds the closing NULL.
*/
Opt_trace_info Opt_trace_iterator::operator*()
{
@@ -780,12 +787,9 @@ Opt_trace_info Opt_trace_iterator::opera
}
-Opt_trace_iterator Opt_trace_iterator::end;
-
-
/* Opt_trace_stmt class */
-const size_t Opt_trace_stmt::empty_trace_len= 4;
+const size_t Opt_trace_stmt::empty_trace_len= 4; // magic number
const size_t Opt_trace_stmt::buffer_alloc_increment= 1024;
@@ -803,7 +807,7 @@ Opt_trace_stmt::Opt_trace_stmt(Opt_trace
}
-void Opt_trace_stmt::end(void)
+void Opt_trace_stmt::end()
{
DBUG_ASSERT(depth == 0);
depth= 0;
@@ -825,22 +829,30 @@ void Opt_trace_stmt::end(void)
}
-void Opt_trace_stmt::next_line(void)
+static const char my_spaces[] =
+ " "
+ " "
+ " "
+ " "
+ " "
+ " "
+ " "
+ ; // Extend as desired, to max supported indentation level.
+void Opt_trace_stmt::next_line()
{
if (ctx->one_line)
return;
buffer.append('\n');
- char spaces[]= " ";
- // 2 spaces per nesting level
- const int nb= depth * 2, spaces_len= sizeof(spaces) - 1;
- // add spaces in chunks of spaces_len, better than many append(' ')
- for (int i= 0; i < nb/spaces_len; i++)
- buffer.append(spaces, spaces_len);
- buffer.append(spaces, (nb % spaces_len));
+
+ // You can get away with a single append here:
+ int ix= sizeof(my_spaces) - 2*depth - 1;
+ if (ix < 0)
+ ix= 0;
+ buffer.append(&my_spaces[ix]);
}
-void Opt_trace_stmt::push(void)
+void Opt_trace_stmt::push()
{
DBUG_ASSERT(support_I_S);
depth++;
@@ -854,19 +866,16 @@ void Opt_trace_stmt::push(void)
}
-void Opt_trace_stmt::pop(void)
+void Opt_trace_stmt::pop()
{
DBUG_ASSERT(support_I_S);
depth--;
}
-void Opt_trace_stmt::separator(void)
+void Opt_trace_stmt::separator()
{
- /*
- If we've already written an object at this level, first put a
- comma.
- */
+ // Put a comma first, if we have already written an object at this level.
if (!level_empty.at(depth))
buffer.append(',');
next_line();
@@ -877,19 +886,19 @@ void Opt_trace_stmt::separator(void)
bool Opt_trace_stmt::set_query(const char *query, size_t length,
CHARSET_INFO *charset)
{
- /* Should be called only once per statement */
+ // Should be called only once per statement.
DBUG_ASSERT(query_buffer.ptr() == NULL);
query_buffer.set_charset(charset);
- /* We're taking a bit of space from 'buffer' */
+ // We are taking a bit of space from 'buffer'.
if (buffer.alloced_length() >= buffer.allowed_mem_size)
{
- /* trace buffer already occupies all space */
+ // Trace buffer already occupies all space.
buffer.missing_bytes+= length;
return true;
}
const size_t available= buffer.allowed_mem_size - buffer.alloced_length();
query_buffer.allowed_mem_size= available;
- /* No need to escape query, this is not for JSON */
+ /* No need to escape query, this is not for JSON. */
const bool rc= query_buffer.append(query, length);
/* Space which query took is taken out of the trace: */
if (query_buffer.alloced_length() >= buffer.allowed_mem_size)
@@ -904,7 +913,7 @@ bool Opt_trace_stmt::set_query(const cha
bool Opt_trace_stmt::Buffer::realloc(uint32 arg_length)
{
- bool rc= String::realloc(arg_length);
+ bool rc= string_buf.realloc(arg_length);
malloc_error|= rc;
return rc;
}
@@ -931,7 +940,8 @@ bool Opt_trace_stmt::Buffer::append_esca
{
rc= false;
const char *pstr, *pstr_end;
- char buf[128] /* temporary output buffer*/, *pbuf= buf;
+ char buf[128]; // Temporary output buffer.
+ char *pbuf= buf;
for (pstr= str, pstr_end= (str + length) ; pstr < pstr_end ; pstr++)
{
char esc;
@@ -939,8 +949,8 @@ bool Opt_trace_stmt::Buffer::append_esca
/*
JSON syntax says that control characters must be escaped. Experience
confirms that this means ASCII 0->31 and " and \ . A few of
- them are accepted with a short escaping syntax (using \ : like
- \n) but for most of them, only \uXXXX works, where XXXX is an
+ them are accepted with a short escaping syntax (using \ : like \n)
+ but for most of them, only \uXXXX works, where XXXX is a
hexadecimal value for the code point.
Rules also mention escaping / , but Python's and Perl's json modules
do not require it, and somewhere on Internet someone said JSON
@@ -948,7 +958,7 @@ bool Opt_trace_stmt::Buffer::append_esca
*/
switch (c)
{
- // don't use \u when possible for common chars, \ is easier to read:
+ // Don't use \u when possible for common chars, \ is easier to read:
case '\\':
esc= '\\';
break;
@@ -968,7 +978,7 @@ bool Opt_trace_stmt::Buffer::append_esca
esc= 0;
break;
}
- if (esc != 0) // escaping with backslash
+ if (esc != 0) // Escaping with backslash.
{
*pbuf++= '\\';
*pbuf++= esc;
@@ -976,7 +986,7 @@ bool Opt_trace_stmt::Buffer::append_esca
else
{
uint ascii_code= (uint)c;
- if (ascii_code < 32) // escaping with \u
+ if (ascii_code < 32) // Escaping with \u
{
*pbuf++= '\\';
*pbuf++= 'u';
@@ -994,17 +1004,17 @@ bool Opt_trace_stmt::Buffer::append_esca
*pbuf++= _dig_vec_lower[ascii_code];
}
else
- *pbuf++= c; // normal character, no escaping needed
+ *pbuf++= c; // Normal character, no escaping needed.
}
- if (pbuf > buf + (sizeof(buf) - 6))
+ if (pbuf > buf + (sizeof(buf) - 6)) // magic number
{
- /* no room in 'buf' for next char, so flush it */
- rc|= String::append(buf, pbuf - buf);
+ // No room in 'buf' for next char, so flush it.
+ rc|= string_buf.append(buf, pbuf - buf);
pbuf= buf; // back to buf's start
}
}
- /* flush any chars left in 'buf' */
- rc|= String::append(buf, pbuf - buf);
+ // Flush any chars left in 'buf'.
+ rc|= string_buf.append(buf, pbuf - buf);
}
malloc_error|= rc;
}
@@ -1027,7 +1037,7 @@ bool Opt_trace_stmt::Buffer::append(cons
rc= true;
else
#endif
- rc= String::append(str, length);
+ rc= string_buf.append(str, length);
malloc_error|= rc;
}
return rc;
@@ -1044,8 +1054,8 @@ bool Opt_trace_stmt::Buffer::append(char
}
else
{
- // no need for escaping chr, given how this function is used */
- rc= String::append(chr);
+ // No need for escaping chr, given how this function is used.
+ rc= string_buf.append(chr);
malloc_error|= rc;
}
return rc;
@@ -1056,11 +1066,13 @@ bool Opt_trace_stmt::Buffer::append(char
Opt_trace_disable_I_S::Opt_trace_disable_I_S(Opt_trace_context *ctx_arg,
bool disable_arg) :
+ ctx(ctx_arg),
+ stmt(ctx ? ctx->current_stmt_in_gen : NULL),
disable(disable_arg)
{
if (likely(!disable))
return;
- ctx= ctx_arg;
+
if (ctx != NULL)
{
/* Disable in to-be-created future traces */
@@ -1069,7 +1081,6 @@ Opt_trace_disable_I_S::Opt_trace_disable
saved_ctx_cannot_change_settings= ctx->cannot_change_settings;
/* Mark that this disabling is mandatory for all: */
ctx->cannot_change_settings= true;
- stmt= ctx->current_stmt_in_gen;
if (stmt != NULL)
{
/* Disable also in the current trace */
@@ -1080,7 +1091,7 @@ Opt_trace_disable_I_S::Opt_trace_disable
}
-Opt_trace_disable_I_S::~Opt_trace_disable_I_S(void)
+Opt_trace_disable_I_S::~Opt_trace_disable_I_S()
{
if (likely(!disable))
return;
=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h 2010-12-19 14:24:03 +0000
+++ b/sql/opt_trace.h 2011-01-11 08:15:46 +0000
@@ -18,12 +18,14 @@
#include "my_config.h" // OPTIMIZER_TRACE
#include "sql_array.h" // Dynamic_array
-class THD;
#include "my_base.h" // ha_rows
#include "sql_string.h" // String
#include "sql_list.h" // because sql_cmd.h needs it
#include "sql_cmd.h" // for enum_sql_command
+struct st_schema_table;
+struct TABLE_LIST;
+
/**
@file
API for the Optimizer trace (WL#5257)
@@ -56,17 +58,17 @@ class THD;
dictionary or Perl's associative array or hash or STL's hash_map.
@li "arrays" (ordered set of values); equivalent to Python's and Perl's list
or STL's vector.
- @li "values": a value can be a string, number, boolean, null, which we all call
- "scalars", or be an object, array.
+ @li "values": a value can be a string, number, boolean, null,
+ which we all call "scalars", or be an object, array.
- For example (after "<<" are explanations which are not part of output):
+ For example (explanations after "<<" are not part of output):
@verbatim
{ << start of top object
"first_name": "Gustave", << key/value pair (value is string)
"last_name": "Eiffel", << key/value pair (value is string)
"born": 1832, << key/value pair (value is integer)
"contributions_to": [ << key/value pair (value is array)
- { << 1st item of array is an object (here a building)
+ { << 1st item of array is an object (a building)
"name": "Eiffel tower",
"location": Paris
}, << end of 1st item of array
@@ -190,21 +192,21 @@ class THD;
Opt_trace_array ota(trace, "semijoin_strategy_choice");
@endverbatim
- this creates an array for key "semijoin_strategy_choice". We are going to
+ This creates an array for key "semijoin_strategy_choice". We are going to
list possible semijoin strategy choices.
@verbatim
Opt_trace_object oto(trace);
@endverbatim
- this creates an object without key (normal, it's in an array). This
+ This creates an object without key (normal, it's in an array). This
object will describe one single strategy choice.
@verbatim
oto.add("strategy", "FirstMatch");
@endverbatim
- this adds a key/value pair to the just-created object: key is "strategy",
+ This adds a key/value pair to the just-created object: key is "strategy",
value is "FirstMatch". This is the strategy to be described in the
just-created object.
@@ -214,7 +216,7 @@ class THD;
oto.add("chosen", (pos->sj_strategy == SJ_OPT_FIRST_MATCH));
@endverbatim
- this adds 3 key/value pairs: cost of strategy, number of records produced
+ This adds 3 key/value pairs: cost of strategy, number of records produced
by this strategy, and whether this strategy is chosen.
After that, there is similar code for other semijoin strategies.
@@ -290,8 +292,10 @@ class THD;
@li Use only simple characters for key names: a-ZA-Z_# (to denote a
number), and no space.
+??? to denote a number ???
+
@li Keep in mind than in an object, keys are not ordered; an application may
- parse the JSON output and output it again with keys' order changed; thus
+ parse the JSON output and output it again with keys order changed; thus
when order matters, use an array (which may imply having anonymous objects
as items of the array, with keys inside the anonymous objects, see how it's
done in JOIN::optimize_steps()). Keep in mind that in an object keys should
@@ -321,8 +325,8 @@ class Opt_trace_stmt;
@li When adding a value (scalar or array or object) to an array, or adding a
key/value pair to an object, we need this outer object or array (from now
- on, we will call "structure" an "object or array", as both are structured
- types).
+ on, we will use the term "structure" for "object or array",
+ as both are structured types).
@li The most natural way would be that the outer object would be passed in
argument to the adder (the function which wants to add the value or
@@ -334,7 +338,8 @@ class Opt_trace_stmt;
modifying many function prototypes.
Example (in gdb "backtrace" notation: inner frames first):
@verbatim
- #0 Item_in_subselect::single_value_transformer - opens an object for key "transformation"
+ #0 Item_in_subselect::single_value_transformer
+ - opens an object for key "transformation"
#1 Item_in_subselect::select_in_like_transformer - does no tracing
#2 Item_allany_subselect::select_transformer - does no tracing
#3 JOIN::prepare - opens an object for key "join_preparation"
@@ -344,28 +349,32 @@ class Opt_trace_stmt;
@li So, as we cannot practically pass the object down, we rather maintain a
"current object or array" in the Opt_trace_context context; it's a pointer
- to an instance of Opt_trace_struct, and the function deep down (frame #0) grabs
- it from the context, where it was depositted by the function high up (frame
- #13 in the last example).
+ to an instance of Opt_trace_struct, and the function deep down (frame #0)
+ grabs it from the context, where it was depositted by the function high up
+ (frame #13 in the last example).
*/
class Opt_trace_context
{
public:
- Opt_trace_context(void);
- ~Opt_trace_context(void);
+ Opt_trace_context();
+ ~Opt_trace_context();
/**
- Flags' names for @@@@optimizer_trace variable of @c sys_vars.cc :
+ Names of flags for @@@@optimizer_trace variable of @c sys_vars.cc :
@li "enabled" = tracing enabled
@li "end_marker" = see parameter of @ref Opt_trace_context::start
@li "one_line"= see parameter of @ref Opt_trace_context::start
@li "default".
*/
static const char *flag_names[];
+
/** Flags' numeric values for @@@@optimizer_trace variable */
+ // the style guide says enum enum_flag_value { ... }
+ // the style guide does *not* mandate uppercase enum values .....
enum flag_value {
FLAG_DEFAULT= 0, FLAG_ENABLED= 1, FLAG_END_MARKER= 2, FLAG_ONE_LINE= 4
};
+
/**
Features' names for @@@@optimizer_trace_features variable of
@c sys_vars.cc:
@@ -375,13 +384,16 @@ public:
@li "default".
*/
static const char *feature_names[];
+
/** Features' numeric values for @@@@optimizer_trace_features variable */
+ // These are actually decimal bit values(?)
+ // so maybe write 1 << 0, 1 << 1, and so on???
enum feature_value {
GREEDY_SEARCH= 1,
RANGE_OPTIMIZER= 2, ///@todo join cache, semijoin...
/*
- if you add here, update feature_value of empty implementation
- and FEATURES_DEFAULT!
+ If you add here, update feature_value of empty implementation
+ and default_features!
*/
/**
Anything unclassified, including the top object (thus, by "inheritance
@@ -391,15 +403,15 @@ public:
*/
MISC= 128
};
- static const feature_value FEATURES_DEFAULT;
+ static const feature_value default_features;
/**
Starts a new trace.
- @param need_it_for_I_S should trace produce output suitable for
- information_schema, or only send to DBUG
- @param end_marker for a key/(object|array) pair, should the key be
+ @param need_it_for_I_S Should trace produce output suitable for
+ information_schema, or only send to DBUG?
+ @param end_marker For a key/(object|array) pair, should the key be
repeated in a comment when the object|array
- closes, like
+ closes? like
@verbatim
"key_foo": {
multi-line blah
@@ -409,14 +421,14 @@ public:
JSON. Note that YAML supports #-prefixed
comments (we would just need to put the next
item's "," before the current item's "#").
- @param one_line should the trace be on a single line without
- indentation (more compact for network transfer
- to programs, less human-readable)
- @param offset offset for restricting trace production
- @param limit limit for restricting trace production
- @param max_mem_size maximum allowed for cumulated size of all
- remembered traces
- @param features only those optimizer features should be traced
+ @param one_line Should the trace be on a single line without
+ indentation? (More compact for network transfer
+ to programs, less human-readable.)
+ @param offset Offset for restricting trace production.
+ @param limit Limit for restricting trace production.
+ @param max_mem_size Maximum allowed for cumulated size of all
+ remembered traces.
+ @param features Only these optimizer features should be traced.
@retval false ok
@retval true error (OOM)
@@ -425,22 +437,25 @@ public:
long offset, long limit, ulong max_mem_size,
ulonglong features);
/** Ends the current trace */
- void end(void);
+ void end();
+
/** Returns whether there is a current trace */
- bool is_started(void) const { return current_stmt_in_gen != NULL; }
+ bool is_started() const { return current_stmt_in_gen != NULL; }
+
/** Returns the current open Object Or Array */
- Opt_trace_struct *get_current_struct(void) const;
+ Opt_trace_struct *get_current_struct() const;
+
/**
Returns a pointer to the last bytes of the current trace, 0-terminated.
@param size how many last bytes are wanted
*/
const char *get_tail(size_t size);
+
/**
Brainwash: deletes all remembered traces and resets counters regarding
- OFFSET/LIMIT (so that the next statement is considered as "at offset
- 0").
+ OFFSET/LIMIT (so that the next statement is considered as "at offset 0").
*/
- void reset(void);
+ void reset();
/**
Set the "original query" for the current statement.
@@ -462,7 +477,7 @@ private:
- to output traces "oldest first" in OPTIMIZER_TRACE
- to preserve traces "newest first" when optimizer-trace-offset<0
- to delete a trace in the middle of the list when it is permanently out
- of the offset/limit showable window.
+ of the offset/limit showable window.
*/
Opt_trace_stmt *oldest_stmt_to_show;
Opt_trace_stmt *newest_stmt_to_show; ///< Newest remembered statement trace
@@ -473,6 +488,7 @@ private:
OFFSET/LIMIT
*/
ha_rows since_offset_0;
+ // ha_rows seems strange, why not int/long?
/**
Trace which is currently being generated, where structures are being
@@ -506,15 +522,17 @@ private:
false: they should support only DBUG (or nothing, if non-debug binary)
*/
bool support_I_S;
- bool end_marker; ///< copy of parameter of Opt_trace_context::start
- bool one_line; ///< copy of parameter of Opt_trace_context::start
- long offset; ///< copy of parameter of Opt_trace_context::start
- long limit; ///< copy of parameter of Opt_trace_context::start
- size_t max_mem_size; ///< copy of parameter of Opt_trace_context::start
+ bool end_marker; ///< copy of parameter of Opt_trace_context::start
+ bool one_line; ///< copy of parameter of Opt_trace_context::start
+ long offset; ///< copy of parameter of Opt_trace_context::start
+ long limit; ///< copy of parameter of Opt_trace_context::start
+ size_t max_mem_size; ///< copy of parameter of Opt_trace_context::start
feature_value features; ///< copy of parameter of Opt_trace_context::start
- /** Whether the settings above may be changed for a new trace */
+ /** Whether the settings above may be changed for a new trace. */
+ // comment is positive, variable name is negative.
bool cannot_change_settings;
+
/**
Find and delete unneeded traces.
For example if OFFSET=-1,LIMIT=1, only the last trace is needed. When a
@@ -523,22 +541,34 @@ private:
@param all if true, ignore OFFSET and thus delete everything
*/
void purge(bool purge_all); ///< find and delete unneeded traces
- /** put trace in list of traces to show in OPTIMIZER_TRACE */
+
+ /** Put trace in list of traces to show in OPTIMIZER_TRACE. */
void link_to_shown(Opt_trace_stmt *stmt);
- /** remove trace from list of traces to show in OPTIMIZER_TRACE */
+
+ /** Remove trace from list of traces to show in OPTIMIZER_TRACE. */
void unlink_from_shown(Opt_trace_stmt *stmt);
- /** put trace in list of unneeded traces */
+
+ /** Put trace in list of unneeded traces. */
void link_to_del(Opt_trace_stmt *stmt);
- /** remove trace from list of unneeded traces */
+
+ /** Remove trace from list of unneeded traces. */
void unlink_from_del(Opt_trace_stmt *stmt);
+
/** compute maximum allowed memory size for trace 'for_stmt'*/
size_t allowed_mem_size(const Opt_trace_stmt *for_stmt) const;
+ // Having too many friends is asking for trouble ....
+ // Opt_trace_stmt needs only a const getter for one_line.
+ // Opt_trace_struct needs a couple of const getters, plus current_stmt_in_gen
+ // Opt_trace_disable_I_S actually changes the state of the context object!
+ // Opt_trace_iterator needs only oldest_stmt_to_show and limit
+ // and finally: the free function does not need friendship.
+
friend class Opt_trace_stmt;
friend class Opt_trace_struct;
friend class Opt_trace_disable_I_S;
friend class Opt_trace_iterator;
- friend void opt_trace_print_expanded_query(THD *thd);
+ //friend void opt_trace_print_expanded_query(THD *thd);
};
@@ -565,7 +595,8 @@ private: // Except other classes in this
Opt_trace_stmt(Opt_trace_context *ctx_arg, Opt_trace_stmt *parent,
bool support_I_S_arg);
/** Ends a trace; destruction may not be possible immediately though */
- void end(void);
+ void end();
+
bool started;
Opt_trace_context *ctx; ///< context
Opt_trace_stmt *parent; ///< parent trace
@@ -577,30 +608,30 @@ private: // Except other classes in this
bool support_I_S;
/**
- Extension of class String, for storing query or trace.
-
- We want to prevent users from calling String functions which allocate
- memory, because we want to record their malloc error status. So we make
- them accessible only through our wrappers, thanks to private
- inheritance.
+ A wrapper of class String, for storing query or trace.
*/
- class Buffer: private String
+ class Buffer
{
-private:
+ private:
size_t allowed_mem_size; ///< allowed memory size for this String
size_t missing_bytes; ///< how many bytes could not be added
bool malloc_error; ///< whether there was a malloc/realloc() error
-public:
+ String string_buf;
+ public:
Buffer() : allowed_mem_size(0), missing_bytes(0), malloc_error(false) {}
- uint32 alloced_length(void) const { return String::alloced_length(); }
- uint32 length(void) const { return String::length(); }
+ uint32 alloced_length() const { return string_buf.alloced_length(); }
+ uint32 length() const { return string_buf.length(); }
bool realloc(uint32 arg_length);
- char *c_ptr_safe(void)
+ char *c_ptr_safe()
{
/* Alas, String::c_ptr_safe() does no realloc error checking */
- return String::c_ptr_safe();
+ return string_buf.c_ptr_safe();
}
- inline const char *ptr(void) const { return String::ptr(); }
+ const char *ptr() const { return string_buf.ptr(); }
+
+ CHARSET_INFO *charset() { return string_buf.charset(); }
+ void set_charset(CHARSET_INFO *charset) { string_buf.set_charset(charset); }
+
/**
@param str String, in this instance's charset
@param length length of string
@@ -622,6 +653,7 @@ public:
friend class Opt_trace_iterator;
};
+ // Suggest renaming to trace_buffer: much easier to search for it.
Buffer buffer; ///< Where the trace is accumulated
Buffer query_buffer; ///< Where the query is put
@@ -629,19 +661,21 @@ public:
Nesting level of the current structure.
The more nested ("deep"), the more indentation spaces we add on the left.
*/
+ // how about current_depth: much easier to search for it.
int depth;
/** maximum nesting level so far */
int max_depth;
/** whether current structure is empty; one such info per nesting level */
Dynamic_array<int> level_empty;
+
/** enter a deeper nesting level */
- void push(void);
+ void push();
/** leave current nesting level and go back one level up */
- void pop(void);
+ void pop();
/** put comma, newline and indentation */
- void separator(void);
+ void separator();
/** put newline and indentation */
- void next_line(void);
+ void next_line();
/** @sa Opt_trace_context::set_query() */
bool set_query(const char* query, size_t length, CHARSET_INFO *charset);
@@ -649,11 +683,18 @@ public:
Opt_trace_stmt *prev, *next; ///< list management
/** By how much we should increase buffer's size when it's becoming full */
+ // At first sight it looks strange that Buffer doesn't handle this itself.
+ // Do we add to the length, or do we multiply (which is more common I think)
static const size_t buffer_alloc_increment;
/** Length of an empty trace */
static const size_t empty_trace_len;
public:
+ const Opt_trace_stmt *prev_() const { return prev; }
+
+ size_t alloced_length() const
+ { return buffer.alloced_length() + query_buffer.alloced_length(); }
+
/** Turn this on only in unit tests for out-of-memory testing */
static bool simulate_oom;
@@ -661,7 +702,7 @@ public:
friend class Opt_trace_struct;
friend class Opt_trace_disable_I_S;
friend class Opt_trace_iterator;
- friend void opt_trace_print_expanded_query(THD *thd);
+// friend void opt_trace_print_expanded_query(THD *thd);
};
@@ -671,7 +712,7 @@ public:
struct Opt_trace_info
{
/**
- String containing trace. It is NUL-terminated, only to aid debugging or
+ String containing trace. It is NULL-terminated, only to aid debugging or
unit testing; this property is not relied upon in normal server usage.
*/
const char *trace_ptr;
@@ -708,20 +749,23 @@ public:
'*' operator.
*/
Opt_trace_iterator(Opt_trace_context *ctx) :
- cursor(ctx->oldest_stmt_to_show), row_count(1), limit(ctx->limit) {}
- void operator++(int); ///< advances iterator to next trace
+ cursor(ctx->oldest_stmt_to_show), row_count(1), limit(ctx->limit)
+ {}
+
+ ///< Advances iterator to next trace.
+ // A bit strange to implement postfix iterator, rather than prefix.
+ void operator++(int);
+
/**
Returns information about the trace on which the iterator is
positioned.
+ @note Returns object by value!!!
*/
Opt_trace_info operator*();
- /** Needed to compare the iterator with the list's end */
- bool operator!=(const Opt_trace_iterator &other) const
- { return cursor != other.cursor; }
- static Opt_trace_iterator end; ///< List's end
+
+ bool at_end() const { return cursor == NULL; }
+
private:
- /** Only to construct the 'end' iterator */
- Opt_trace_iterator(void) : cursor(NULL) {}
Opt_trace_stmt *cursor; ///< trace which the iterator is positioned on
ha_rows row_count; ///< how many traces yielded so far
ha_rows limit; ///< yield "empty" after yielding that many traces
@@ -734,13 +778,13 @@ private:
When you want to add a structure to the trace, you create an instance of
Opt_trace_object or Opt_trace_array, then you add information to it with
add(), then the destructor closes the OOA (we use RAII, Resource
+ ^^^ ???
Acquisition Is Initialization).
*/
class Opt_trace_struct
{
protected:
/**
- Constructor.
@param ctx_arg Optimizer trace context for this structure
@param requires_key_arg whether this structure requires/forbids keys
for values put inside it (an object requires them, an
@@ -761,7 +805,7 @@ protected:
/* A first inlined test */
if (unlikely(ctx_arg != NULL) && ctx_arg->is_started())
{
- /* tracing enabled: must fully initialize the structure */
+ // Tracing enabled: must fully initialize the structure.
do_construct(ctx_arg->current_stmt_in_gen,
ctx_arg->current_stmt_in_gen->current_struct,
requires_key_arg, key, feature);
@@ -771,7 +815,7 @@ protected:
dummy.
*/
}
- ~Opt_trace_struct(void) { if (unlikely(started)) do_destruct(); }
+ ~Opt_trace_struct() { if (unlikely(started)) do_destruct(); }
public:
@@ -799,7 +843,7 @@ public:
In the most performance-critical case (release binary with
tracing compiled in but not enabled at runtime), we don't want function
- calls, which is why we have an inline if() in each add() method below.
+ calls, which is why we have an inline if() in each add() function below.
In an optimizer-intensive test (as in BUG#50595 with a 20-table plan
search, see mysql-test/t/bug50595.test), this inlining took
@c greedy_search()'s clock time from 2.6 secs down to 2.0 secs.
@@ -807,13 +851,18 @@ public:
@verbatim <some Opt_trace_struct>.add().add().add() @endverbatim
which have 3 if()s (one per @c add()), it has been measured that wrapping
that inside a single if(), like
- @verbatim if(started) { <some Opt_trace_struct>.add().add().add() } @endverbatim
+ @verbatim if(started) { <some Opt_trace_struct>.add().add().add() }
+ @endverbatim
degraded performance (from 2.0 to 2.1 secs). Adding if() in one single
- code line caused de-inlining of add() methods in several unrelated places
+ code line caused de-inlining of add() functions in several unrelated places
and the performance degraded. So we don't do it. The performance impact of
multiple chained add() seems comparable to a single add() anyway.
Removing @c likely() also increases from 2.0 to 2.1, so we leave
them.
+
+ // I don't need to know all these details about milliseconds here ....
+ // This should be a documentation about the interface.
+
The same test against a vanilla tree without this WL, took 1.9 secs; so
does the WL tree with optimizer trace compiled out.
So the important figure is that this WL takes the common case from 1.9 to
@@ -838,6 +887,7 @@ public:
return *this;
return do_add(key, value, strlen(value), false);
}
+
/**
Adds a value (of string type) to the structure. No key is specified, so
it adds only the value (the structure must thus be an array).
@@ -850,6 +900,7 @@ public:
return *this;
return do_add(NULL, value, strlen(value), false);
}
+
/**
Like add_alnum() but supports any UTF8 characters in 'value'.
Will "escape" 'value' to be JSON-compliant.
@@ -864,6 +915,7 @@ public:
return *this;
return do_add(key, value, val_length, true);
}
+
/** Variant of add_utf8() for adding to an array (no key) */
Opt_trace_struct& add_utf8(const char *value, size_t val_length)
{
@@ -871,6 +923,7 @@ public:
return *this;
return do_add(NULL, value, val_length, true);
}
+
/** Variant of add_utf8() where 'value' is 0-terminated */
Opt_trace_struct& add_utf8(const char *key,
const char *value)
@@ -879,6 +932,7 @@ public:
return *this;
return do_add(key, value, strlen(value), true);
}
+
/** Variant of add_utf8() where 'value' is 0-terminated */
Opt_trace_struct& add_utf8(const char *value)
{
@@ -886,6 +940,7 @@ public:
return *this;
return do_add(NULL, value, strlen(value), true);
}
+
/**
Add a value (of Item type) to the structure. The Item should be a
condition (like a WHERE clause) which will be pretty-printed into the
@@ -990,7 +1045,7 @@ public:
structure before it goes out of scope. Don't use it unless RAII mandates
a new scope which mandates re-indenting lots of code lines.
*/
- void end(void) { if (unlikely(started)) do_destruct(); }
+ void end() { if (unlikely(started)) do_destruct(); }
private:
/** Full initialization. @sa Opt_trace_struct::Opt_trace_struct */
@@ -1000,7 +1055,7 @@ private:
const char *key,
Opt_trace_context::feature_value feature);
/** Really does destruction */
- void do_destruct(void);
+ void do_destruct();
/**
Really adds to the object. @sa add().
@param escape do JSON-compliant escaping of 'value'.
@@ -1022,6 +1077,7 @@ private:
NULL otherwise.
*/
void add_struct(const char *key);
+
/**
Emits a JSON syntax error.
@param key key involved in the error, NULL if there is no key.
@@ -1029,6 +1085,7 @@ private:
When adding a value (or array or object) to an array, or a key/value pair
to an object, we need this outer array or object.
+Sentence below is very hard to read, too many ()()()
It would be possible, when trying to add a key to an array (which is wrong
in JSON) (or similarly when trying to add a value without any key to an
object), to catch it at compilation time, if the outer object was passed an
@@ -1041,13 +1098,13 @@ private:
Opt_trace_struct), and the adder grabs it from the context.
As this current structure is of type "object or array", we cannot do
- compile-time checks that only suitable methods are used. A call to @c
+ compile-time checks that only suitable functions are used. A call to @c
add(key,value) is necessarily legal for the compiler as the structure may
be an object, though it will be wrong in case the structure is actually
an array at run-time. Thus we have the risk of an untested particular
situation where the current structure is not an object (but an array)
though the code expected it to be one. We catch that at run-time:
- Opt_trace_struct methods detect wrong usage, like adding a value to an
+ Opt_trace_struct functions detect wrong usage, like adding a value to an
object without specifying a key, and then they:
@li in debug build, assert
@li in release builds, emit a warning string in the trace and should not
@@ -1063,22 +1120,34 @@ private:
Opt_trace_struct& operator=(const Opt_trace_struct&);
bool started; ///< Whether the structure does tracing
+
/**
Whether the structure requires/forbids keys for values inside it.
1: this is an object. 0: this is an array. Other: incorrect.
+ it is unclear at this point why it is int8 rather than bool
+
@note The canonical way would be to not have such int8 per instance, but
rather have a pure virtual method Opt_trace_struct::requires_key(),
+C++ has member functions, not methods.
overloaded by Opt_trace_object (returning 1) and by Opt_trace_array
(returning 0). But this would have drawbacks:
@li it would add a vtbl pointer to each instance which takes even more
space than int8
+-- this depends on alignment (you happen to have a bool as a neighbour..)
@li it would add requires_key() function calls which cost more than
reading one int8
+-- you don't know that, until measured on different platforms.
+-- reading an int8 may require masking operations, unless
+-- your platform is byte addressable
@li Opt_trace_object::requires_key() would not be accessible from
Opt_trace_struct::construct() (which would complicate coding), whereas the
int8 is.
+-- virtual function not accessible to CTOR/DTOR: true
+-- OK, so I buy this argument, but not the others :-)
*/
+ // Hand-coded typechecking is generally a bad idea,
+ // but since you need the information in the base CTOR/DTOR: OK.
int8 requires_key;
Opt_trace_stmt *stmt; ///< Trace owning the structure
/** Parent structure ("outer structure" of the structure) */
@@ -1129,8 +1198,9 @@ public:
*/
Opt_trace_object(Opt_trace_context *ctx, const char *key,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) :
- Opt_trace_struct(ctx, true, key, feature) {}
+ Opt_trace_context::MISC)
+ : Opt_trace_struct(ctx, true, key, feature)
+ {}
/**
Constructs an object. No key is specified, so the object is just a value
(serves for the single root object, or for adding the object to an array).
@@ -1138,8 +1208,9 @@ public:
*/
Opt_trace_object(Opt_trace_context *ctx,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) :
- Opt_trace_struct(ctx, true, NULL, feature) {}
+ Opt_trace_context::MISC)
+ : Opt_trace_struct(ctx, true, NULL, feature)
+ {}
};
@@ -1159,8 +1230,9 @@ public:
*/
Opt_trace_array(Opt_trace_context *ctx, const char *key,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) :
- Opt_trace_struct(ctx, false, key, feature) {}
+ Opt_trace_context::MISC)
+ : Opt_trace_struct(ctx, false, key, feature)
+ {}
/**
Constructs an array. No key is specified, so the array is just a value
(serves for adding the object to an array).
@@ -1168,16 +1240,17 @@ public:
*/
Opt_trace_array(Opt_trace_context *ctx,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) :
- Opt_trace_struct(ctx, false, NULL, feature) {}
+ Opt_trace_context::MISC)
+ : Opt_trace_struct(ctx, false, NULL, feature)
+ {}
};
/**
Instantiate an instance of this class for specific, punctual cases where
optimizer trace should write only to DBUG and not I_S. Example:
- @c QUICK_RANGE_SELECT::dbug_dump() writes to the I_S trace and DBUG a
- "range_select" object. This is good when called from
+ @c QUICK_RANGE_SELECT::dbug_dump() writes a "range_select" object
+ to the I_S trace and DBUG. This is good when called from
@c SQL_SELECT::test_quick_select(). But it is also called by @c TEST_join()
(from @c JOIN::optimize()), and there the produced I_S trace is
undesirable, so it is silenced with such object below (trace only goes to
@@ -1191,12 +1264,12 @@ class Opt_trace_disable_I_S
public:
/**
Constructor
- @param ctx_arg context
- @param disable_arg whether the instance should really disable
+ @param ctx_arg Context.
+ @param disable_arg Whether the instance should really disable
anything. If false, the object is dummy. If true,
tracing-to-I_S is disabled at construction and
re-enabled at destruction.
- @details a dummy instance is there only for RAII reasons. Imagine we want
+ @details A dummy instance is there only for RAII reasons. Imagine we want
to do this:
@verbatim
{
@@ -1224,6 +1297,7 @@ public:
Opt_trace_disable_I_S(Opt_trace_context *ctx_arg, bool disable_arg);
/** Destructor. Re-enables tracing. */
~Opt_trace_disable_I_S();
+
private:
Opt_trace_context *ctx; ///< context to disable/re-enable
Opt_trace_stmt *stmt; ///< trace to disable/re-enable
@@ -1236,13 +1310,18 @@ private:
};
+// I believe the macros below are gone in newer versions of the code.
+// Anyways: the names aren't very good, and the formatting was bad ...
+
/**
Grabs the current structure from the context and call its methods.
Most of the time, there is a direct reference to this structure available
(because it has been instantianted a few lines earlier), then prefer using
it than this macro.
*/
-#define OPT_TRACE(trace, action) do { \
+#define OPT_TRACE(trace, action) \
+ do \
+ { \
if (unlikely((trace) != NULL)) \
if (unlikely((trace)->is_started())) \
(trace)->get_current_struct()->action; \
@@ -1252,7 +1331,9 @@ private:
/**
Executes an action only if tracing is started.
*/
-#define OPT_TRACE2(trace, action) do { \
+#define OPT_TRACE2(trace, action) \
+ do \
+ { \
if (unlikely((trace) != NULL)) \
if (unlikely((trace)->is_started())) \
action; \
@@ -1266,7 +1347,6 @@ private:
//@{
-struct TABLE_LIST;
/**
Start tracing a THD's actions (generally at a statement's start).
@param thd the THD
@@ -1278,6 +1358,7 @@ struct TABLE_LIST;
sub-statement in the call chain), and this function decides to trace
(respectively not trace) the sub-statement, it returns "true"
(resp. false). Each sub-statement is responsible for ending the trace which it
+()()()()()
has started.
*/
bool opt_trace_start(THD *thd, const TABLE_LIST *tbl,
@@ -1328,7 +1409,6 @@ static inline bool opt_trace_set_query(O
*/
int fill_optimizer_trace_info(THD *thd, TABLE_LIST *tables, Item *cond);
-struct st_schema_table;
/**
Create fields' descriptions of information_schema.OPTIMIZER_TRACE
@retval 0 ok
@@ -1361,10 +1441,12 @@ class Opt_trace_object
public:
Opt_trace_object(Opt_trace_context *ctx, const char *key,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) {}
+ Opt_trace_context::MISC)
+ {}
Opt_trace_object(Opt_trace_context *ctx,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) {}
+ Opt_trace_context::MISC)
+ {}
Opt_trace_object& add_alnum(const char *key, const char *value)
{ return *this; }
Opt_trace_object& add_utf8(const char *key,
@@ -1379,7 +1461,7 @@ public:
Opt_trace_object& add(const char *key, longlong value) { return *this; }
Opt_trace_object& add(const char *key, ulonglong value) { return *this; }
Opt_trace_object& add(const char *key, double value) { return *this; }
- void end(void) {}
+ void end() {}
};
/** Empty implementation used when optimizer trace is not compiled in */
@@ -1388,10 +1470,12 @@ class Opt_trace_array
public:
Opt_trace_array(Opt_trace_context *ctx, const char *key,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) {}
+ Opt_trace_context::MISC)
+ {}
Opt_trace_array(Opt_trace_context *ctx,
Opt_trace_context::feature_value feature=
- Opt_trace_context::MISC) {}
+ Opt_trace_context::MISC)
+ {}
Opt_trace_array& add_alnum(const char *value) { return *this; }
Opt_trace_array& add_utf8(const char *value, size_t val_length)
{ return *this; }
@@ -1404,7 +1488,7 @@ public:
Opt_trace_array& add(longlong value) { return *this; }
Opt_trace_array& add(ulonglong value) { return *this; }
Opt_trace_array& add(double value) { return *this; }
- void end(void) {}
+ void end() {}
};
/** Empty implementation used when optimizer trace is not compiled in */
@@ -1452,12 +1536,14 @@ public:
/**
Helper to put the database/table name in the trace
@param t TABLE* pointer
+why macro, why not an ordinary function?
*/
#define add_utf8_table(t) \
add_utf8("database", (t)->s->db.str, (t)->s->db.length). \
add_utf8("table", (t)->alias)
-#if !defined(DBUG_OFF) && !defined(OPTIMIZER_TRACE)
+#if !defined(DBUG_OFF) && !defined(OPTIMIZER_TRACE) \
+ && !defined(OPTIMIZER_TRACE_UNITTEST)
/*
A debug binary without optimizer trace compiled in, will miss some
=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc 2010-12-19 14:24:03 +0000
+++ b/sql/opt_trace2server.cc 2011-01-11 08:15:46 +0000
@@ -19,14 +19,10 @@
Helpers connecting the optimizer trace to THD or Information Schema. They
are dedicated "to the server" (hence the file's name).
In order to create a unit test of the optimizer trace without defining
- Item_field (and all its parent classes), st_select_lex..., those helpers
- are defined out of opt_trace.cc.
+ Item_field (and all its parent classes), st_select_lex..., these helpers
+ are defined in opt_trace.cc.
*/
-#ifdef USE_PRAGMA_IMPLEMENTATION
-#pragma implementation // gcc: Class implementation
-#endif
-
#include "opt_trace.h"
#include "sql_show.h" // schema_table_stored_record()
#include "sql_parse.h" // sql_command_flags
@@ -60,8 +56,8 @@ static bool list_has_optimizer_trace_tab
Whether a SQL command qualifies for optimizer tracing.
@param sql_command the command
*/
-static inline bool sql_command_can_be_traced
-(enum enum_sql_command sql_command)
+static inline bool
+sql_command_can_be_traced(enum enum_sql_command sql_command)
{
/*
Tracing is limited to a few SQL commands only.
@@ -71,7 +67,7 @@ static inline bool sql_command_can_be_tr
- they probably don't have anything interesting optimizer-related
- select_lex for them might be uninitialized and unprintable.
- SHOW WARNINGS would create an uninteresting trace and thus overwrite the
- previous interesting one.
+ previous interesting one.
About prepared statements: note that we don't turn on tracing for
SQLCOM_PREPARE (respectively SQLCOM_EXECUTE), because we don't know yet what
@@ -96,15 +92,17 @@ bool opt_trace_start(THD *thd, const TAB
We need an optimizer trace:
* if the user asked for it or
* if we are using --debug (because the trace serves as a relay for it, for
- optimizer debug printouts).
- Additionally, we should not be tracing if:
+ optimizer debug printouts).
+ Additionally, we should *not* be tracing if:
* command is not interesting (optimizer-wise)
* query involves I_S.OPTIMIZER_TRACE (we do not want to overwrite the
- trace while reading it with SELECT).
+ trace while reading it with SELECT).
*/
- ulonglong var= thd->variables.optimizer_trace;
+ const ulonglong var= thd->variables.optimizer_trace;
bool need_it_for_I_S= (var & Opt_trace_context::FLAG_ENABLED);
- bool need_it_for_dbug= false, allocated_here= false;
+ bool need_it_for_dbug= false;
+ bool 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;);
if (!(need_it_for_I_S || need_it_for_dbug))
@@ -113,16 +111,19 @@ bool opt_trace_start(THD *thd, const TAB
goto disable;
if (list_has_optimizer_trace_table(tbl))
goto disable;
+
/*
We don't allocate it in THD's MEM_ROOT as it must survive until a next
statement (SELECT) reads the trace.
*/
if (thd->opt_trace == NULL)
{
- if ((thd->opt_trace= new Opt_trace_context()) == NULL)
+ // (this version of) operator new can never return NULL.
+ if ((thd->opt_trace= new Opt_trace_context) == NULL)
goto disable;
allocated_here= true;
}
+
start_trace:
if (thd->opt_trace->start(need_it_for_I_S,
(var & Opt_trace_context::FLAG_END_MARKER),
@@ -140,6 +141,7 @@ start_trace:
goto disable;
}
DBUG_RETURN(true); // started all ok
+
disable:
/*
No need to create a trace for this statement.
@@ -156,7 +158,7 @@ disable:
if (need_it_for_I_S && thd->opt_trace != NULL && thd->opt_trace->is_started())
{
need_it_for_I_S= false;
- goto start_trace;
+ goto start_trace; // Ouch, criss-cross goto!
}
DBUG_RETURN(false);
}
@@ -176,6 +178,7 @@ void opt_trace_print_expanded_query(THD
Opt_trace_context * const trace= thd->opt_trace;
if (trace == NULL || !trace->is_started())
return;
+
/*
In "SELECT stored_func()" , execution of a
sub-statement of stored_func() may start by opening tables (that's the case
@@ -188,15 +191,16 @@ void opt_trace_print_expanded_query(THD
thd->lex->sql_command is SQLCOM_END in this case. So we do a check of the
command:
*/
+// Too many parens (()) in comment above, hard to read.
if (!sql_command_can_be_traced(thd->lex->sql_command))
return;
+
char buff[1024];
String str(buff,(uint32) sizeof(buff), system_charset_info);
str.length(0);
/**
- If this statement is not SELECT, what is shown here can be
- inexact. INSERT SELECT is shown as SELECT. DELETE WHERE is shown
- as SELECT WHERE.
+ If this statement is not SELECT, what is shown here can be inexact.
+ INSERT SELECT is shown as SELECT. DELETE WHERE is shown as SELECT WHERE.
This is acceptable given the audience (developers) and the goal (the
inexact parts are irrelevant for the optimizer).
*/
@@ -211,7 +215,7 @@ void opt_trace_add_select_number(Opt_tra
{
if (unlikely(select_number >= INT_MAX))
{
- /* clearer than any huge number */
+ // Clearer than any huge number.
s->add_alnum("select#", "fake");
}
else
@@ -230,9 +234,7 @@ int fill_optimizer_trace_info(THD *thd,
The list must not change during the iterator's life time. This is ok as
the life time is only the present block which cannot change the list.
*/
- for (Opt_trace_iterator it(thd->opt_trace) ;
- it != Opt_trace_iterator::end ;
- it++)
+ for (Opt_trace_iterator it(thd->opt_trace) ; !it.at_end() ; it++)
{
const Opt_trace_info info= *it;
restore_record(table, s->default_values);
@@ -242,8 +244,10 @@ int fill_optimizer_trace_info(THD *thd,
When literals with introducers are used, see "LiteralsWithIntroducers"
in this file.
*/
- table->field[0]->store(info.query_ptr, info.query_length, info.query_charset);
- table->field[1]->store(info.trace_ptr, info.trace_length, system_charset_info);
+ table->field[0]->store(info.query_ptr, info.query_length,
+ info.query_charset);
+ table->field[1]->store(info.trace_ptr, info.trace_length,
+ system_charset_info);
table->field[2]->store(info.missing_bytes, true);
table->field[3]->store(info.malloc_error, true);
if (schema_table_store_record(thd, table))
@@ -265,6 +269,7 @@ ST_FIELD_INFO optimizer_trace_info[]=
/* name, length, type, value, maybe_null, old_name, open_method */
{"QUERY", 65535, MYSQL_TYPE_STRING, 0, false, "", SKIP_OPEN_TABLE},
{"TRACE", 65535, MYSQL_TYPE_STRING, 0, false, 0, SKIP_OPEN_TABLE},
+ // why is this zero, others are "" ^
{"MISSING_BYTES_BEYOND_MAX_MEM_SIZE", 20, MYSQL_TYPE_LONG,
0, false, "", SKIP_OPEN_TABLE},
{"OS_MALLOC_ERROR", 1, MYSQL_TYPE_TINY, 0, false, "", SKIP_OPEN_TABLE},
@@ -274,13 +279,11 @@ ST_FIELD_INFO optimizer_trace_info[]=
int make_optimizer_trace_table_for_show(THD *thd, ST_SCHEMA_TABLE *schema_table)
{
- ST_FIELD_INFO *field_info;
Name_resolution_context *context= &thd->lex->select_lex.context;
- int i;
- for (i= 0; schema_table->fields_info[i].field_name != NULL; i++)
+ for (int i= 0; schema_table->fields_info[i].field_name != NULL; i++)
{
- field_info= &schema_table->fields_info[i];
+ ST_FIELD_INFO *field_info= &schema_table->fields_info[i];
Item_field *field= new Item_field(context,
NullS, NullS, field_info->field_name);
if (field)
=== modified file 'sql/sys_vars.cc'
--- a/sql/sys_vars.cc 2010-11-23 10:21:53 +0000
+++ b/sql/sys_vars.cc 2011-01-11 08:15:46 +0000
@@ -1437,7 +1437,7 @@ static Sys_var_flagset Sys_optimizer_tra
" and val is one of {on, off, default}",
SESSION_VAR(optimizer_trace_features), CMD_LINE(REQUIRED_ARG),
Opt_trace_context::feature_names,
- DEFAULT(Opt_trace_context::FEATURES_DEFAULT));
+ DEFAULT(Opt_trace_context::default_features));
/** Delete all old optimizer traces */
static bool optimizer_trace_update(sys_var *self, THD *thd,
=== modified file 'unittest/gunit/CMakeLists.txt'
--- a/unittest/gunit/CMakeLists.txt 2010-09-18 16:25:43 +0000
+++ b/unittest/gunit/CMakeLists.txt 2011-01-11 08:15:46 +0000
@@ -207,7 +207,7 @@ IF (CMAKE_CXX_COMPILER_ID STREQUAL "SunP
ENDIF()
# Add tests (link them with sql library)
-SET(TESTS sql_list mdl mdl_mytap my_regex thread_utils opt_trace)
+SET(TESTS sql_list mdl mdl_mytap my_regex thread_utils opt_trace opt_notrace)
FOREACH(test ${TESTS})
ADD_EXECUTABLE(${test}-t ${test}-t.cc)
TARGET_LINK_LIBRARIES(${test}-t gunit sqlgunitlib strings dbug regex)
=== added file 'unittest/gunit/opt_notrace-t.cc'
--- a/unittest/gunit/opt_notrace-t.cc 1970-01-01 00:00:00 +0000
+++ b/unittest/gunit/opt_notrace-t.cc 2011-01-11 08:15:46 +0000
@@ -0,0 +1,21 @@
+#include "my_config.h"
+#include <gtest/gtest.h>
+
+#if defined(OPTIMIZER_TRACE)
+#undef OPTIMIZER_TRACE
+#endif
+#define OPTIMIZER_TRACE_UNITTEST
+
+#include <opt_trace.h>
+
+namespace {
+
+TEST(Foo, Bar)
+{
+ // Fill in more here, to verify that implementations are in sync!!!
+ Opt_trace_context trace;
+ Opt_trace_object oto(&trace);
+ Opt_trace_array ota(&trace);
+}
+
+}
=== modified file 'unittest/gunit/opt_trace-t.cc'
--- a/unittest/gunit/opt_trace-t.cc 2010-11-24 18:54:26 +0000
+++ b/unittest/gunit/opt_trace-t.cc 2011-01-11 08:15:46 +0000
@@ -28,8 +28,11 @@
/* Some minimal working implementations to have a working trace... */
CHARSET_INFO *system_charset_info= &my_charset_utf8_general_ci;
/* ... end here. */
+// I didn't quite understand the comments above ...
-const ulonglong all_features= Opt_trace_context::FEATURES_DEFAULT;
+namespace {
+
+const ulonglong all_features= Opt_trace_context::default_features;
/**
@@ -43,11 +46,12 @@ const ulonglong all_features= Opt_trace_
*/
void check_json_compliance(const char *str, size_t length)
{
-#if 0
+ return;
/*
Read from stdin, eliminate comments, parse as JSON. If invalid, an exception
is thrown by Python, uncaught, which produces a non-zero error code.
*/
+#ifndef __WIN__
const char python_cmd[]=
"python -c \""
"import json, re, sys;"
@@ -56,22 +60,34 @@ void check_json_compliance(const char *s
"json.loads(s, 'utf-8')\"";
// Send the trace to this new process' stdin:
FILE *fd= popen(python_cmd, "w");
- ASSERT_NE((void*)NULL, fd);
- ASSERT_EQ((size_t)1, fwrite(str, length, 1, fd));
+// ASSERT makes sense here: if we fail to run python we must abort.
+// Most of the other uses of ASSERT should be changed to EXPECT,
+// so that we continue to run the rest of the test even if there
+// is a failed assumption.
+ ASSERT_TRUE(NULL != fd);
+ ASSERT_EQ(1U, fwrite(str, length, 1, fd));
int rc= pclose(fd);
rc= WEXITSTATUS(rc);
ASSERT_EQ(0, rc);
#endif
}
+class TraceContentTest : public ::testing::Test
+{
+public:
+ Opt_trace_context trace;
+};
+
+
+TEST_F(TraceContentTest, ConstructAndDestruct)
+{
+}
+
/** Test empty trace */
-TEST(Trace_content_test, empty)
+TEST_F(TraceContentTest, empty)
{
- /* Create a trace */
- Opt_trace_context trace;
- ASSERT_EQ(false, trace.start(true, false, false, -1, 1, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, false, false, -1, 1, ULONG_MAX, all_features));
/*
Add at least an object to it. A really empty trace ("") is not
JSON-compliant, at least Python's JSON module raises an exception.
@@ -83,26 +99,24 @@ TEST(Trace_content_test, empty)
trace.end();
/* And verify trace's content */
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]= "{\n}";
- ASSERT_STREQ(expected, info.trace_ptr);
- ASSERT_EQ(sizeof(expected) - 1, info.trace_length);
+ EXPECT_STREQ(expected, info.trace_ptr);
+ EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
check_json_compliance(info.trace_ptr, info.trace_length);
- ASSERT_EQ((size_t)0, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
+ EXPECT_EQ(0U, info.missing_bytes);
+ EXPECT_FALSE(info.malloc_error);
/* Should be no more traces */
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test normal usage */
-TEST(Trace_content_test, normal_usage)
+TEST_F(TraceContentTest, NormalUsage)
{
- Opt_trace_context trace;
- ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, true, false, -1, 1, ULONG_MAX, all_features));
{
Opt_trace_object oto(&trace);
{
@@ -125,7 +139,7 @@ TEST(Trace_content_test, normal_usage)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -152,19 +166,17 @@ TEST(Trace_content_test, normal_usage)
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/**
- Test Opt_trace_context::get_tail(). Same as Trace_content_test.normal_usage
+ Test Opt_trace_context::get_tail(). Same as TraceContentTest.NormalUsage
but with a get_tail() in the middle.
*/
-TEST(Trace_content_test, tail)
+TEST_F(TraceContentTest, tail)
{
- Opt_trace_context trace;
- ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, true, false, -1, 1, ULONG_MAX, all_features));
{
Opt_trace_object oto(&trace);
{
@@ -187,7 +199,7 @@ TEST(Trace_content_test, tail)
const char *tail= trace.get_tail(40);
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -216,16 +228,14 @@ TEST(Trace_content_test, tail)
ASSERT_EQ(false, info.malloc_error);
ASSERT_EQ(0, strcmp(expected + sizeof(expected) - 1 - 40, tail));
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test reaction to malformed JSON (object with value without key) */
-TEST(Trace_content_test, buggy_object)
+TEST_F(TraceContentTest, BuggyObject)
{
- Opt_trace_context trace;
- ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, true, false, -1, 1, ULONG_MAX, all_features));
{
Opt_trace_object oto(&trace);
{
@@ -248,7 +258,7 @@ TEST(Trace_content_test, buggy_object)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -273,14 +283,13 @@ TEST(Trace_content_test, buggy_object)
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test reaction to malformed JSON (array with value with key) */
-TEST(Trace_content_test, buggy_array)
+TEST_F(TraceContentTest, BuggyArray)
{
- Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
all_features));
{
@@ -299,7 +308,7 @@ TEST(Trace_content_test, buggy_array)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -319,16 +328,14 @@ TEST(Trace_content_test, buggy_array)
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test Opt_trace_disable_I_S */
-TEST(Trace_content_test, disable_I_S)
+TEST_F(TraceContentTest, DisableIS)
{
- Opt_trace_context trace;
- ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, true, false, -1, 1, ULONG_MAX, all_features));
{
Opt_trace_object oto(&trace);
{
@@ -360,7 +367,7 @@ TEST(Trace_content_test, disable_I_S)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -387,15 +394,16 @@ TEST(Trace_content_test, disable_I_S)
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Helper for Trace_settings_test.offset */
+// Our style guide says "do not transfer by reference".
static void make_one_trace(Opt_trace_context &trace, const char *name,
long offset, long limit)
{
- ASSERT_EQ(false, trace.start(true, true, false, offset, limit, ULONG_MAX,
- all_features));
+ ASSERT_FALSE(trace.start(true, true, false, offset, limit, ULONG_MAX,
+ all_features));
{
Opt_trace_object oto(&trace);
oto.add(name, 0LL);
@@ -407,38 +415,40 @@ static void make_one_trace(Opt_trace_con
/**
Helper for Trace_settings_test.offset
- @param names a NULL-terminated array of "names"
+ @param trace The trace context.
+ @param names A NULL-terminated array of "names".
Checks that the list of traces is as expected.
This macro checks that the first trace contains names[0], that the second
trace contains names[1], etc. That the number of traces is the same as
the number of elements in "names".
- @note it is a macro, for proper reporting of line numbers in case of
- assertion failure (if it were a function, the line number in the function
- would be reported, which is not useful).
+ @note It is a macro, for proper reporting of line numbers in case of
+ assertion failure. SCOPED_TRACE will report line number at the
+ macro expansion site.
*/
-#define check(trace, names) do \
- { \
- Opt_trace_iterator it(&trace); \
- Opt_trace_info info; \
- for (const char **name= names ; *name != NULL ; name++) \
- { \
- ASSERT_EQ(true, it != Opt_trace_iterator::end); \
- info= *it; \
- const size_t name_len= strlen(*name); \
- ASSERT_EQ(name_len + 11, info.trace_length); \
- ASSERT_EQ(0, strncmp(info.trace_ptr + 5, *name, name_len)); \
- ASSERT_EQ((size_t)0, info.missing_bytes); \
- ASSERT_EQ(false, info.malloc_error); \
- it++; \
- } \
- ASSERT_EQ(false, it != Opt_trace_iterator::end); \
- } while (0)
+#define check(trace, names) { SCOPED_TRACE(""); do_check(&trace, names); }
+
+void do_check(Opt_trace_context *trace, const char**names)
+{
+ Opt_trace_iterator it(trace);
+ for (const char **name= names ; *name != NULL ; name++)
+ {
+ EXPECT_FALSE(it.at_end());
+ Opt_trace_info info= *it;
+ const size_t name_len= strlen(*name);
+ ASSERT_EQ(name_len + 11, info.trace_length);
+ ASSERT_EQ(0, strncmp(info.trace_ptr + 5, *name, name_len));
+ ASSERT_EQ((size_t)0, info.missing_bytes);
+ ASSERT_EQ(false, info.malloc_error);
+ it++;
+ }
+ EXPECT_TRUE(it.at_end());
+}
/** Test offset/limit variables */
-TEST(Trace_settings_test, offset)
+TEST(TraceSettingsTest, offset)
{
Opt_trace_context trace;
make_one_trace(trace, "100", -1 /* offset */, 1 /* limit */);
@@ -492,7 +502,7 @@ TEST(Trace_settings_test, offset)
/** Test truncation by max_mem_size */
-TEST(Trace_settings_test, max_mem_size)
+TEST(TraceSettingsTest, MaxMemSize)
{
Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, false, false, -1, 1,
@@ -508,7 +518,7 @@ TEST(Trace_settings_test, max_mem_size)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -525,13 +535,13 @@ TEST(Trace_settings_test, max_mem_size)
ASSERT_EQ(false, info.malloc_error);
ASSERT_EQ(0, strncmp(expected, info.trace_ptr, sizeof(expected) - 1));
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test how truncation by max_mem_size affects next traces */
-TEST(Trace_settings_test, max_mem_size2)
+TEST(TraceSettingsTest, MaxMemSize2)
{
Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, false, false, -2, 2,
@@ -550,7 +560,7 @@ TEST(Trace_settings_test, max_mem_size2)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
Opt_trace_info info= *it;
ASSERT_EQ((size_t)17, info.trace_length);
ASSERT_EQ((size_t)16, info.missing_bytes);
@@ -562,7 +572,7 @@ TEST(Trace_settings_test, max_mem_size2)
ASSERT_EQ((size_t)33, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
/*
3rd trace; the first one should automatically be purged, thus the 3rd
should have a bit of room.
@@ -574,7 +584,7 @@ TEST(Trace_settings_test, max_mem_size2)
}
trace.end();
Opt_trace_iterator it2(&trace);
- ASSERT_EQ(true, it2 != Opt_trace_iterator::end);
+ EXPECT_FALSE(it2.at_end());
info= *it2;
ASSERT_EQ((size_t)0, info.trace_length);
ASSERT_EQ((size_t)33, info.missing_bytes);
@@ -589,15 +599,14 @@ TEST(Trace_settings_test, max_mem_size2)
ASSERT_EQ((size_t)19, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it2++;
- ASSERT_EQ(false, it2 != Opt_trace_iterator::end);
+ EXPECT_TRUE(it2.at_end());
}
/** Test reaction to out-of-memory condition */
#ifndef DBUG_OFF
-TEST(Trace_content_test, out_of_memory)
+TEST_F(TraceContentTest, OutOfMemory)
{
- Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, false, false, -1, 1, ULONG_MAX,
all_features));
{
@@ -611,7 +620,7 @@ TEST(Trace_content_test, out_of_memory)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -624,15 +633,14 @@ TEST(Trace_content_test, out_of_memory)
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(true, info.malloc_error); // malloc error reported
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
#endif // !DBUG_OFF
/** Test filtering by feature */
-TEST(Trace_content_test, filtering_by_feature)
+TEST_F(TraceContentTest, FilteringByFeature)
{
- Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, false, false, -1, 1, ULONG_MAX,
Opt_trace_context::MISC));
{
@@ -663,7 +671,7 @@ TEST(Trace_content_test, filtering_by_fe
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
const char expected[]=
"{\n"
@@ -687,14 +695,13 @@ TEST(Trace_content_test, filtering_by_fe
ASSERT_EQ((size_t)0, info.missing_bytes);
ASSERT_EQ(false, info.malloc_error);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test escaping of characters */
-TEST(Trace_content_test, escaping)
+TEST_F(TraceContentTest, escaping)
{
- Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
all_features));
// All ASCII 0-127 chars are valid UTF8 encodings
@@ -702,8 +709,8 @@ TEST(Trace_content_test, escaping)
for (uint c= 0; c < sizeof(all_chars) - 2 ; c++)
all_chars[c]= c;
// Now a character with a two-byte code in utf8: ä
- all_chars[128]= 0xc3;
- all_chars[129]= 0xa4;
+ all_chars[128]= 0xc3; // warning on windows: truncation of constant value
+ all_chars[129]= 0xa4; // warning on windows: truncation of constant value
// all_chars is used both as query...
trace.set_query(all_chars, sizeof(all_chars), system_charset_info);
{
@@ -713,7 +720,7 @@ TEST(Trace_content_test, escaping)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
// we get the trace escaped, JSON-compliant:
const char expected[]=
@@ -730,14 +737,13 @@ TEST(Trace_content_test, escaping)
ASSERT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
ASSERT_EQ(system_charset_info, info.query_charset);
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
/** Test how the system handles non-UTF8 characters, a violation of its API */
-TEST(Trace_content_test, non_utf8)
+TEST_F(TraceContentTest, NonUtf8)
{
- Opt_trace_context trace;
ASSERT_EQ(false, trace.start(true, true, false, -1, 1, ULONG_MAX,
all_features));
/*
@@ -775,7 +781,7 @@ TEST(Trace_content_test, non_utf8)
}
trace.end();
Opt_trace_iterator it(&trace);
- ASSERT_EQ(true, it != Opt_trace_iterator::end);
+ EXPECT_FALSE(it.at_end());
const Opt_trace_info info= *it;
// This is UTF8 and thus JSON-compliant; ABC is present
const char expected[]=
@@ -791,7 +797,9 @@ TEST(Trace_content_test, non_utf8)
// we get the query unescaped, verbatim, not 0-terminated:
ASSERT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
it++;
- ASSERT_EQ(false, it != Opt_trace_iterator::end);
+ EXPECT_TRUE(it.at_end());
}
+} // namespace
+
#endif // OPTIMIZER_TRACE
Attachment: [text/bzr-bundle] bzr/tor.didriksen@oracle.com-20110111081546-uee7eww1v6whym0h.bundle