#At file:///home/mysql_src/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800-applying-Tor-s-review/ based on revid:tor.didriksen@stripped
3244 Guilhem Bichot 2011-01-26
Pulled the entire big commit of Tor's review comments; here doing
additional edits and a few reverts.
modified:
sql/opt_trace.cc
sql/opt_trace.h
sql/opt_trace2server.cc
unittest/gunit/opt_trace-t.cc
=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc 2011-01-11 08:15:46 +0000
+++ b/sql/opt_trace.cc 2011-01-26 10:57:25 +0000
@@ -31,8 +31,6 @@
/* Opt_trace_struct class */
-const char Opt_trace_struct::brackets[]= { '[', '{', ']', '}' };
-const char *Opt_trace_struct::types[]= { "array", "object" };
bool Opt_trace_struct::dbug_assert_on_syntax_error= true;
void Opt_trace_struct::syntax_error(const char *key)
@@ -46,14 +44,26 @@ void Opt_trace_struct::syntax_error(cons
*/
if (key != NULL)
{
- stmt->buffer.append(STRING_WITH_LEN("** invalid JSON"
- " (unexpected key \""));
- stmt->buffer.append(key);
- stmt->buffer.append(STRING_WITH_LEN("\") ** "));
+ stmt->trace_buffer.append(STRING_WITH_LEN("** invalid JSON"
+ " (unexpected key \""));
+ stmt->trace_buffer.append(key);
+ stmt->trace_buffer.append(STRING_WITH_LEN("\") ** "));
}
else
- stmt->buffer.append(STRING_WITH_LEN("** invalid JSON"
- " (missing key) ** "));
+ stmt->trace_buffer.append(STRING_WITH_LEN("** invalid JSON"
+ " (missing key) ** "));
+}
+
+
+/** opening and closing symbols for arrays ([])and objects ({}) */
+static const char brackets[]= { '[', '{', ']', '}' };
+static inline char opening_bracket(bool requires_key)
+{
+ return brackets[requires_key];
+}
+static inline char closing_bracket(bool requires_key)
+{
+ return brackets[requires_key + 2];
}
@@ -66,7 +76,7 @@ void Opt_trace_struct::do_construct(Opt_
saved_key= key;
requires_key= requires_key_arg;
- DBUG_PRINT("opt_trace", ("%s: starting %s", key, types[requires_key]));
+ DBUG_PRINT("opt_trace", ("%s: starting struct", key));
stmt= stmt_arg;
parent= parent_arg;
DBUG_ASSERT(parent == NULL || stmt == parent->stmt);
@@ -92,48 +102,13 @@ void Opt_trace_struct::do_construct(Opt_
stmt->support_I_S= false; // disable tracing for this struct and children
}
if (stmt->support_I_S)
- {
- const size_t alloced= stmt->buffer.alloced_length();
- const size_t increment= Opt_trace_stmt::buffer_alloc_increment;
- if ((alloced - stmt->buffer.length()) < (increment / 3))
- {
- /*
- Support for I_S will produce long strings, and there is little free
- space left in the allocated buffer, so it looks like
- realloc is soon unavoidable; so let's get many bytes at a time.
- Note that if this re-allocation fails, or any String::append(), we
- will get a weird trace; either truncated if the server stops, or maybe
- with a hole if there is later memory again for the trace's
- continuation. That will be visible in the OS_MALLOC_ERROR column.
- */
- size_t new_size= alloced + increment;
- size_t max_size= stmt->buffer.allowed_mem_size;
- /*
- Determine a safety margin:
- (A) String::realloc() adds at most ALIGN_SIZE(1) bytes to requested
- length, so we need to decrement max_size by this amount, to be sure that
- we don't allocate more than max_size
- (B) We need to stay at least one byte under that max_size, or the next
- append() would trigger up-front truncation, which is potentially wrong
- for a "pre-emptive allocation" as we do here.
- */
- const size_t safety_margin= ALIGN_SIZE(1) /* (A) */ + 1 /* (B) */;
- if (max_size >= safety_margin)
- {
- max_size-= safety_margin;
- if (new_size > max_size) // Don't pre-allocate more than the limit.
- new_size= max_size;
- if (new_size >= alloced) // Never shrink string.
- stmt->buffer.realloc(new_size);
- }
- }
- }
+ stmt->trace_buffer.prealloc();
if (likely(parent != NULL))
parent->add_struct(key);
stmt->current_struct= this;
if (!stmt->support_I_S)
return;
- stmt->buffer.append(brackets[requires_key]);
+ stmt->trace_buffer.append(opening_bracket(requires_key));
stmt->push();
}
@@ -160,9 +135,9 @@ void Opt_trace_struct::add_key_name(cons
strncpy(previous_key, key, sizeof(previous_key) - 1);
previous_key[sizeof(previous_key) - 1]= 0;
#endif
- stmt->buffer.append('"');
- stmt->buffer.append(key);
- stmt->buffer.append(STRING_WITH_LEN("\": "));
+ stmt->trace_buffer.append('"');
+ stmt->trace_buffer.append(key);
+ stmt->trace_buffer.append(STRING_WITH_LEN("\": "));
}
}
@@ -179,7 +154,7 @@ void Opt_trace_struct::add_struct(const
void Opt_trace_struct::do_destruct()
{
- DBUG_PRINT("opt_trace", ("%s: ending %s", saved_key, types[requires_key]));
+ DBUG_PRINT("opt_trace", ("%s: ending struct", saved_key));
DBUG_ASSERT(started);
/*
Note:
@@ -196,12 +171,12 @@ void Opt_trace_struct::do_destruct()
{
stmt->pop();
stmt->next_line();
- stmt->buffer.append(brackets[requires_key + 2]);
+ stmt->trace_buffer.append(closing_bracket(requires_key));
if (stmt->ctx->end_marker && saved_key != NULL)
{
- stmt->buffer.append(STRING_WITH_LEN(" /* "));
- stmt->buffer.append(saved_key);
- stmt->buffer.append(STRING_WITH_LEN(" */"));
+ stmt->trace_buffer.append(STRING_WITH_LEN(" /* "));
+ stmt->trace_buffer.append(saved_key);
+ stmt->trace_buffer.append(STRING_WITH_LEN(" */"));
}
}
stmt->support_I_S= save_stmt_support_I_S;
@@ -209,6 +184,11 @@ void Opt_trace_struct::do_destruct()
}
+/**
+ @note add() has an up-front if(), hopefully inlined, so that in the common
+ case - tracing run-time disabled - we have no function call. If tracing is
+ enabled, we call do_add().
+*/
Opt_trace_struct& Opt_trace_struct::do_add(const char *key, const char *val,
const size_t val_length,
bool escape)
@@ -219,7 +199,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- stmt->buffer.append('"');
+ stmt->trace_buffer.append('"');
/*
Objects' keys use "normal" characters (A-Za-z0-9_), no escaping
needed. Same for numeric/bool values. Only string values may need
@@ -235,7 +215,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. Even worse, a single bad character in the expanded query makes
+ character. So just 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
@@ -246,14 +226,14 @@ Opt_trace_struct& Opt_trace_struct::do_a
*/
CHARSET_INFO *query_charset= stmt->query_buffer.charset();
if (my_charset_same(query_charset, system_charset_info))
- stmt->buffer.append_escaped(val, val_length);
+ stmt->trace_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)
{
- stmt->buffer.malloc_error= true;
+ stmt->trace_buffer.malloc_error= true;
return *this;
}
uint errors;
@@ -269,13 +249,13 @@ Opt_trace_struct& Opt_trace_struct::do_a
query_charset as target character set, so escaping would be
impossible.
*/
- stmt->buffer.append_escaped(utf8_str, new_length);
+ stmt->trace_buffer.append_escaped(utf8_str, new_length);
my_free(utf8_str);
}
}
else
- stmt->buffer.append(val, val_length);
- stmt->buffer.append('"');
+ stmt->trace_buffer.append(val, val_length);
+ stmt->trace_buffer.append('"');
return *this;
}
@@ -293,7 +273,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
stmt->separator();
add_key_name(key);
const LEX_CSTRING *readable= &readables[(int)val];
- stmt->buffer.append(readable->str, readable->length);
+ stmt->trace_buffer.append(readable->str, readable->length);
return *this;
}
@@ -308,7 +288,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- stmt->buffer.append(buf);
+ stmt->trace_buffer.append(buf);
return *this;
}
@@ -323,7 +303,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- stmt->buffer.append(buf);
+ stmt->trace_buffer.append(buf);
return *this;
}
@@ -338,7 +318,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- stmt->buffer.append(buf);
+ stmt->trace_buffer.append(buf);
return *this;
}
@@ -350,7 +330,7 @@ Opt_trace_struct& Opt_trace_struct::do_a
return *this;
stmt->separator();
add_key_name(key);
- stmt->buffer.append(STRING_WITH_LEN("null"));
+ stmt->trace_buffer.append(STRING_WITH_LEN("null"));
return *this;
}
@@ -597,18 +577,17 @@ bool Opt_trace_context::start(bool suppo
if (new_stmt_support_I_S && offset >= 0)
{
/* If outside the offset/limit window, no need to support I_S */
- if (since_offset_0 < (ulong)offset)
+ if (since_offset_0 < offset)
{
- DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%llu) < offset(%lu)",
- (ulonglong)since_offset_0, offset));
+ DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%ld) < offset(%ld)",
+ since_offset_0, offset));
new_stmt_support_I_S= false;
}
- else if (since_offset_0 >= (ulong)(offset + limit))
+ else if (since_offset_0 >= (offset + limit))
{
- 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));
+ DBUG_PRINT("opt_trace", ("disabled: since_offset_0(%ld) >="
+ " offset(%ld) + limit(%ld)",
+ since_offset_0, offset, limit));
new_stmt_support_I_S= false;
}
since_offset_0++;
@@ -640,9 +619,9 @@ bool Opt_trace_context::start(bool suppo
*/
purge(false);
/* This purge may have freed space, compute max allowed size: */
- stmt->buffer.allowed_mem_size= allowed_mem_size(stmt);
+ stmt->trace_buffer.allowed_mem_size= allowed_mem_size(stmt);
/* Trace is always in UTF8, it's "all" that JSON accepts */
- stmt->buffer.set_charset(system_charset_info);
+ stmt->trace_buffer.set_charset(system_charset_info);
DBUG_ASSERT(system_charset_info == &my_charset_utf8_general_ci);
DBUG_RETURN(false);
}
@@ -661,7 +640,7 @@ void Opt_trace_context::end()
Parent regains control, now it needs to be told that its child has
used space, and thus parent's allowance has shrunk.
*/
- parent->buffer.allowed_mem_size= allowed_mem_size(parent);
+ parent->trace_buffer.allowed_mem_size= allowed_mem_size(parent);
}
}
/*
@@ -721,7 +700,7 @@ const char *Opt_trace_context::get_tail(
{
if (current_stmt_in_gen == NULL)
return "";
- Opt_trace_stmt::Buffer *buffer= &(current_stmt_in_gen->buffer);
+ Opt_trace_stmt::Buffer *buffer= &(current_stmt_in_gen->trace_buffer);
size_t buffer_len= buffer->length();
const char *ptr= buffer->c_ptr_safe();
if (buffer_len > size)
@@ -746,7 +725,7 @@ bool Opt_trace_context::set_query(const
/* Opt_trace_iterator class */
-void Opt_trace_iterator::operator++(int)
+void Opt_trace_iterator::next()
{
cursor= cursor->next;
if (row_count >= limit)
@@ -770,18 +749,18 @@ void Opt_trace_iterator::operator++(int)
to manipulate in a debugger, or to compare in unit tests with
EXPECT_STREQ).
c_ptr_safe() may realloc an empty String from 0 bytes to 8 bytes,
- when it adds the closing NULL.
+ when it adds the closing NUL.
*/
-Opt_trace_info Opt_trace_iterator::operator*()
+Opt_trace_info Opt_trace_iterator::get_value()
{
Opt_trace_info ret;
- ret.trace_ptr= cursor->buffer.c_ptr_safe();
- ret.trace_length= cursor->buffer.length();
+ ret.trace_ptr= cursor->trace_buffer.c_ptr_safe();
+ ret.trace_length= cursor->trace_buffer.length();
ret.query_ptr= cursor->query_buffer.ptr();
ret.query_length= cursor->query_buffer.length();
ret.query_charset= cursor->query_buffer.charset();
- ret.missing_bytes= cursor->buffer.missing_bytes;
- ret.malloc_error= cursor->buffer.malloc_error ||
+ ret.missing_bytes= cursor->trace_buffer.missing_bytes;
+ ret.malloc_error= cursor->trace_buffer.malloc_error ||
cursor->query_buffer.malloc_error;
return ret;
}
@@ -790,15 +769,13 @@ Opt_trace_info Opt_trace_iterator::opera
/* Opt_trace_stmt class */
const size_t Opt_trace_stmt::empty_trace_len= 4; // magic number
-const size_t Opt_trace_stmt::buffer_alloc_increment= 1024;
-
Opt_trace_stmt::Opt_trace_stmt(Opt_trace_context *ctx_arg,
Opt_trace_stmt *parent_arg,
bool support_I_S_arg):
started(true), ctx(ctx_arg), parent(parent_arg),
current_struct(NULL), support_I_S(support_I_S_arg),
- depth(0), max_depth(0)
+ current_depth(0), max_depth(0)
{
DBUG_ASSERT(parent == NULL || ctx == parent->ctx);
int dummy= 0;
@@ -809,16 +786,16 @@ Opt_trace_stmt::Opt_trace_stmt(Opt_trace
void Opt_trace_stmt::end()
{
- DBUG_ASSERT(depth == 0);
- depth= 0;
+ DBUG_ASSERT(current_depth == 0);
+ current_depth= 0;
started= false;
/* Send the full nice trace to DBUG */
DBUG_EXECUTE("opt_trace",
- if (buffer.length() <= empty_trace_len)
+ if (trace_buffer.length() <= empty_trace_len)
{ /* don't spam DBUG with empty trace */ }
else
{
- const char *trace= buffer.c_ptr_safe();
+ const char *trace= trace_buffer.c_ptr_safe();
DBUG_LOCK_FILE;
fputs("Complete optimizer trace:", DBUG_FILE);
fputs(trace, DBUG_FILE);
@@ -842,44 +819,44 @@ void Opt_trace_stmt::next_line()
{
if (ctx->one_line)
return;
- buffer.append('\n');
+ trace_buffer.append('\n');
// You can get away with a single append here:
- int ix= sizeof(my_spaces) - 2*depth - 1;
+ int ix= sizeof(my_spaces) - 2*current_depth - 1;
if (ix < 0)
ix= 0;
- buffer.append(&my_spaces[ix]);
+ trace_buffer.append(&my_spaces[ix]);
}
void Opt_trace_stmt::push()
{
DBUG_ASSERT(support_I_S);
- depth++;
- if (depth > max_depth)
+ current_depth++;
+ if (current_depth > max_depth)
{
int dummy= 0;
level_empty.append(dummy);
- max_depth= depth;
+ max_depth= current_depth;
}
- level_empty.at(depth)= true;
+ level_empty.at(current_depth)= true;
}
void Opt_trace_stmt::pop()
{
DBUG_ASSERT(support_I_S);
- depth--;
+ current_depth--;
}
void Opt_trace_stmt::separator()
{
// Put a comma first, if we have already written an object at this level.
- if (!level_empty.at(depth))
- buffer.append(',');
+ if (!level_empty.at(current_depth))
+ trace_buffer.append(',');
next_line();
- level_empty.at(depth)= false;
+ level_empty.at(current_depth)= false;
}
@@ -889,33 +866,70 @@ bool Opt_trace_stmt::set_query(const cha
// Should be called only once per statement.
DBUG_ASSERT(query_buffer.ptr() == NULL);
query_buffer.set_charset(charset);
- // We are taking a bit of space from 'buffer'.
- if (buffer.alloced_length() >= buffer.allowed_mem_size)
+ // We are taking a bit of space from 'trace_buffer'.
+ if (trace_buffer.alloced_length() >= trace_buffer.allowed_mem_size)
{
// Trace buffer already occupies all space.
- buffer.missing_bytes+= length;
+ trace_buffer.missing_bytes+= length;
return true;
}
- const size_t available= buffer.allowed_mem_size - buffer.alloced_length();
+ const size_t available= trace_buffer.allowed_mem_size -
+ trace_buffer.alloced_length();
query_buffer.allowed_mem_size= available;
/* 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)
- buffer.allowed_mem_size= 0;
+ if (query_buffer.alloced_length() >= trace_buffer.allowed_mem_size)
+ trace_buffer.allowed_mem_size= 0;
else
- buffer.allowed_mem_size-= query_buffer.alloced_length();
+ trace_buffer.allowed_mem_size-= query_buffer.alloced_length();
return rc;
}
/* Opt_trace_stmt::Buffer class */
-bool Opt_trace_stmt::Buffer::realloc(uint32 arg_length)
+bool Opt_trace_stmt::Buffer::prealloc()
{
- bool rc= string_buf.realloc(arg_length);
- malloc_error|= rc;
- return rc;
+ const size_t alloced= alloced_length();
+ const size_t increment= 1024;
+ if ((alloced - length()) < (increment / 3))
+ {
+ /*
+ Support for I_S will produce long strings, and there is little free
+ space left in the allocated buffer, so it looks like
+ realloc is soon unavoidable; so let's get many bytes at a time.
+ Note that if this re-allocation fails, or any String::append(), we
+ will get a weird trace; either truncated if the server stops, or maybe
+ with a hole if there is later memory again for the trace's
+ continuation. That will be visible in the OS_MALLOC_ERROR column.
+ */
+ size_t new_size= alloced + increment;
+ size_t max_size= allowed_mem_size;
+ /*
+ Determine a safety margin:
+ (A) String::realloc() adds at most ALIGN_SIZE(1) bytes to requested
+ length, so we need to decrement max_size by this amount, to be sure that
+ we don't allocate more than max_size
+ (B) We need to stay at least one byte under that max_size, or the next
+ append() would trigger up-front truncation, which is potentially wrong
+ for a "pre-emptive allocation" as we do here.
+ */
+ const size_t safety_margin= ALIGN_SIZE(1) /* (A) */ + 1 /* (B) */;
+ if (max_size >= safety_margin)
+ {
+ max_size-= safety_margin;
+ if (new_size > max_size) // Don't pre-allocate more than the limit.
+ new_size= max_size;
+ if (new_size >= alloced) // Never shrink string.
+ {
+ bool rc= string_buf.realloc(new_size);
+ malloc_error|= rc;
+ return rc;
+ }
+ }
+ }
+ return false;
}
@@ -1070,7 +1084,7 @@ Opt_trace_disable_I_S::Opt_trace_disable
stmt(ctx ? ctx->current_stmt_in_gen : NULL),
disable(disable_arg)
{
- if (likely(!disable))
+ if (!disable)
return;
if (ctx != NULL)
@@ -1093,7 +1107,7 @@ Opt_trace_disable_I_S::Opt_trace_disable
Opt_trace_disable_I_S::~Opt_trace_disable_I_S()
{
- if (likely(!disable))
+ if (!disable)
return;
if (ctx != NULL)
{
=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h 2011-01-11 08:15:46 +0000
+++ b/sql/opt_trace.h 2011-01-26 10:57:25 +0000
@@ -289,10 +289,8 @@ struct TABLE_LIST;
can and should often be used. Having a restricted vocabulary helps
consistency.
- @li Use only simple characters for key names: a-ZA-Z_# (to denote a
- number), and no space.
-
-??? to denote a number ???
+ @li Use only simple characters for key names: a-ZA-Z_#, and no space. '#'
+ serves to denote a number, like in "select#" .
@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
@@ -369,9 +367,8 @@ public:
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 {
+ enum {
FLAG_DEFAULT= 0, FLAG_ENABLED= 1, FLAG_END_MARKER= 2, FLAG_ONE_LINE= 4
};
@@ -386,11 +383,9 @@ public:
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...
+ GREEDY_SEARCH= (1 << 0),
+ RANGE_OPTIMIZER= (1 << 1), ///@todo join cache, semijoin...
/*
If you add here, update feature_value of empty implementation
and default_features!
@@ -401,7 +396,7 @@ public:
This feature cannot be disabled by the user; for this it is important
that it always has biggest flag; flag's value itself does not matter
*/
- MISC= 128
+ MISC= (1 << 7)
};
static const feature_value default_features;
@@ -487,8 +482,7 @@ private:
Number of statements traced so far since "offset 0", for comparison with
OFFSET/LIMIT
*/
- ha_rows since_offset_0;
- // ha_rows seems strange, why not int/long?
+ long since_offset_0;
/**
Trace which is currently being generated, where structures are being
@@ -524,13 +518,13 @@ private:
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
feature_value features; ///< copy of parameter of Opt_trace_context::start
- /** Whether the settings above may be changed for a new trace. */
- // comment is positive, variable name is negative.
+ /** Whether the settings above may not be changed for a new trace. */
bool cannot_change_settings;
/**
@@ -568,7 +562,6 @@ private:
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);
};
@@ -621,7 +614,7 @@ private: // Except other classes in this
Buffer() : allowed_mem_size(0), missing_bytes(0), malloc_error(false) {}
uint32 alloced_length() const { return string_buf.alloced_length(); }
uint32 length() const { return string_buf.length(); }
- bool realloc(uint32 arg_length);
+ bool prealloc(); ///< pro-actively extend buffer if soon short of space
char *c_ptr_safe()
{
/* Alas, String::c_ptr_safe() does no realloc error checking */
@@ -629,7 +622,7 @@ private: // Except other classes in this
}
const char *ptr() const { return string_buf.ptr(); }
- CHARSET_INFO *charset() { return string_buf.charset(); }
+ CHARSET_INFO *charset() const { return string_buf.charset(); }
void set_charset(CHARSET_INFO *charset) { string_buf.set_charset(charset); }
/**
@@ -653,16 +646,14 @@ private: // Except other classes in this
friend class Opt_trace_iterator;
};
- // Suggest renaming to trace_buffer: much easier to search for it.
- Buffer buffer; ///< Where the trace is accumulated
+ Buffer trace_buffer; ///< Where the trace is accumulated
Buffer query_buffer; ///< Where the query is put
/**
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;
+ int current_depth;
/** maximum nesting level so far */
int max_depth;
/** whether current structure is empty; one such info per nesting level */
@@ -682,10 +673,6 @@ private: // Except other classes in this
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;
@@ -693,7 +680,7 @@ public:
const Opt_trace_stmt *prev_() const { return prev; }
size_t alloced_length() const
- { return buffer.alloced_length() + query_buffer.alloced_length(); }
+ { return trace_buffer.alloced_length() + query_buffer.alloced_length(); }
/** Turn this on only in unit tests for out-of-memory testing */
static bool simulate_oom;
@@ -702,7 +689,6 @@ 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);
};
@@ -712,7 +698,7 @@ public:
struct Opt_trace_info
{
/**
- String containing trace. It is NULL-terminated, only to aid debugging or
+ String containing trace. It is NUL-terminated, only to aid debugging or
unit testing; this property is not relied upon in normal server usage.
*/
const char *trace_ptr;
@@ -745,23 +731,19 @@ public:
positioned either:
- on the first element (row_count=1 is then correct)
- or on the end, in which case row_count will not be used, as the user
- should compare with the 'end' iterator before taking the content with the
- '*' operator.
+ should test @c at_end() before reading the content with @c get_value().
*/
Opt_trace_iterator(Opt_trace_context *ctx) :
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);
+ void next(); ///< Advances iterator to next trace.
/**
- Returns information about the trace on which the iterator is
+ Returns information, by value, about the trace on which the iterator is
positioned.
- @note Returns object by value!!!
*/
- Opt_trace_info operator*();
+ Opt_trace_info get_value();
bool at_end() const { return cursor == NULL; }
@@ -777,8 +759,7 @@ private:
Opt_trace_struct is a base class for them.
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
- ^^^ ???
+ add(), then the destructor closes the structure (we use RAII, Resource
Acquisition Is Initialization).
*/
class Opt_trace_struct
@@ -841,35 +822,6 @@ public:
@return a reference to the structure, useful for chaining like this:
@verbatim add(x,y).add(z,t).add(u,v) @endverbatim
- 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() 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.
- In calls like
- @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
- degraded performance (from 2.0 to 2.1 secs). Adding if() in one single
- 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
- 2.0 secs, but it is hoped that the used testcase (20-table plan) is
- unusually intensive on the optimizer and thus real-life cases should have
- a smaller penalty. This will be benchmarked with the QA teams.
-
String-related add() variants are named add_[something]():
- to avoid confusing the compiler between:
add(const char *value, size_t val_length) and
@@ -1085,12 +1037,12 @@ 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
- argument of type @c Opt_trace_array* to the adder. Then the @c add(key,val)
- call would not compile as Opt_trace_array wouldn't feature it.
+ 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 adder received, as function
+ parameter, the type of the structure (like @c Opt_trace_array*). Then
+ the @c add(key,val) call would not compile as Opt_trace_array wouldn't
+ feature it.
But as explained in comment of class Opt_trace_context we
cannot pass down the object, have to maintain a "current object or
@@ -1131,23 +1083,10 @@ Sentence below is very hard to read, too
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 :-)
+ (returning 0). But Opt_trace_object::requires_key() would not be accessible
+ from Opt_trace_struct::construct() (which would complicate coding), whereas
+ the int8 is.
*/
- // 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) */
@@ -1170,10 +1109,6 @@ C++ has member functions, not methods.
*/
bool save_stmt_support_I_S;
- /** opening and closing symbols for arrays ([])and objects ({}) */
- static const char brackets[];
- /** human-readable names of structure types */
- static const char* types[];
public:
/**
Whether a JSON syntax error should cause an assertion in debug binaries;
@@ -1354,11 +1289,10 @@ private:
@param sql_command SQL command being prepared or executed
@return whether this function decided to trace (and thus the corresponding
opt_trace_end() should end the trace).
- @note if tracing was already started (by a top statement above the present
- sub-statement in the call chain), and this function decides to trace
+ @note if tracing was already started by a top statement above the present
+ 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,
=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc 2011-01-11 08:15:46 +0000
+++ b/sql/opt_trace2server.cc 2011-01-26 10:57:25 +0000
@@ -138,7 +138,7 @@ start_trace:
delete thd->opt_trace;
thd->opt_trace= NULL;
}
- goto disable;
+ DBUG_RETURN(false);
}
DBUG_RETURN(true); // started all ok
@@ -158,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; // Ouch, criss-cross goto!
+ goto start_trace;
}
DBUG_RETURN(false);
}
@@ -234,9 +234,9 @@ 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.at_end() ; it++)
+ for (Opt_trace_iterator it(thd->opt_trace) ; !it.at_end() ; it.next())
{
- const Opt_trace_info info= *it;
+ const Opt_trace_info info= it.get_value();
restore_record(table, s->default_values);
/*
We will put the query, which is in character_set_client, into a column
@@ -267,12 +267,11 @@ int fill_optimizer_trace_info(THD *thd,
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 "" ^
+ {"QUERY", 65535, MYSQL_TYPE_STRING, 0, false, NULL, SKIP_OPEN_TABLE},
+ {"TRACE", 65535, MYSQL_TYPE_STRING, 0, false, NULL, SKIP_OPEN_TABLE},
{"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},
+ 0, false, NULL, SKIP_OPEN_TABLE},
+ {"OS_MALLOC_ERROR", 1, MYSQL_TYPE_TINY, 0, false, NULL, SKIP_OPEN_TABLE},
{NULL, 0, MYSQL_TYPE_STRING, 0, true, NULL, 0}
};
=== modified file 'unittest/gunit/opt_trace-t.cc'
--- a/unittest/gunit/opt_trace-t.cc 2011-01-11 08:15:46 +0000
+++ b/unittest/gunit/opt_trace-t.cc 2011-01-26 10:57:25 +0000
@@ -25,15 +25,22 @@
#ifdef OPTIMIZER_TRACE
-/* 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 ...
namespace {
const ulonglong all_features= Opt_trace_context::default_features;
+/**
+ @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_json_compliance(str, length) \
+ { \
+ SCOPED_TRACE(""); \
+ do_check_json_compliance(str, length); \
+ }
/**
Checks compliance of a trace with JSON syntax rules.
@@ -44,7 +51,7 @@ const ulonglong all_features= Opt_trace_
@param str pointer to trace
@param length trace's length
*/
-void check_json_compliance(const char *str, size_t length)
+void do_check_json_compliance(const char *str, size_t length)
{
return;
/*
@@ -60,15 +67,11 @@ 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 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);
+ EXPECT_EQ(0, rc);
#endif
}
@@ -99,8 +102,12 @@ TEST_F(TraceContentTest, empty)
trace.end();
/* And verify trace's content */
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ /*
+ ASSERT here, because a failing EXPECT_FALSE would continue into
+ it.get_value() and segfault.
+ */
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]= "{\n}";
EXPECT_STREQ(expected, info.trace_ptr);
EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
@@ -108,8 +115,8 @@ TEST_F(TraceContentTest, empty)
EXPECT_EQ(0U, info.missing_bytes);
EXPECT_FALSE(info.malloc_error);
/* Should be no more traces */
- it++;
- EXPECT_TRUE(it.at_end());
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -139,8 +146,8 @@ TEST_F(TraceContentTest, NormalUsage)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -160,13 +167,13 @@ TEST_F(TraceContentTest, NormalUsage)
" 4\n"
" ] /* another array */\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);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -199,8 +206,8 @@ TEST_F(TraceContentTest, tail)
const char *tail= trace.get_tail(40);
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -220,15 +227,15 @@ TEST_F(TraceContentTest, tail)
" 4\n"
" ] /* another array */\n"
"}";
- ASSERT_STREQ(expected, info.trace_ptr);
- ASSERT_EQ(sizeof(expected) - 1, info.trace_length);
- ASSERT_EQ((void *)0, info.query_ptr);
- ASSERT_EQ((size_t)0, info.query_length);
- ASSERT_EQ((size_t)0, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- ASSERT_EQ(0, strcmp(expected + sizeof(expected) - 1 - 40, tail));
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_STREQ(expected, info.trace_ptr);
+ EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
+ EXPECT_EQ((void *)0, info.query_ptr);
+ EXPECT_EQ((size_t)0, info.query_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ EXPECT_EQ(0, strcmp(expected + sizeof(expected) - 1 - 40, tail));
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -258,8 +265,8 @@ TEST_F(TraceContentTest, BuggyObject)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -278,12 +285,12 @@ TEST_F(TraceContentTest, BuggyObject)
" 4\n"
" ] /* another array */\n"
"}";
- ASSERT_STREQ(expected, info.trace_ptr);
- ASSERT_EQ(sizeof(expected) - 1, info.trace_length);
- ASSERT_EQ((size_t)0, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_STREQ(expected, info.trace_ptr);
+ EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -308,8 +315,8 @@ TEST_F(TraceContentTest, BuggyArray)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -323,12 +330,12 @@ TEST_F(TraceContentTest, BuggyArray)
" 4\n"
" ] /* another array */\n"
"}";
- ASSERT_STREQ(expected, info.trace_ptr);
- ASSERT_EQ(sizeof(expected) - 1, info.trace_length);
- ASSERT_EQ((size_t)0, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_STREQ(expected, info.trace_ptr);
+ EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -367,8 +374,8 @@ TEST_F(TraceContentTest, DisableIS)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -388,27 +395,26 @@ TEST_F(TraceContentTest, DisableIS)
" 4\n"
" ] /* another array */\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);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_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,
+static void make_one_trace(Opt_trace_context *trace, const char *name,
long offset, long limit)
{
- ASSERT_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);
+ Opt_trace_object oto(trace);
oto.add(name, 0LL);
}
- trace.end();
+ trace->end();
}
@@ -434,16 +440,16 @@ void do_check(Opt_trace_context *trace,
Opt_trace_iterator it(trace);
for (const char **name= names ; *name != NULL ; name++)
{
- EXPECT_FALSE(it.at_end());
- Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ Opt_trace_info info= it.get_value();
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_EQ(name_len + 11, info.trace_length);
+ EXPECT_EQ(0, strncmp(info.trace_ptr + 5, *name, name_len));
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
}
- EXPECT_TRUE(it.at_end());
+ ASSERT_TRUE(it.at_end());
}
@@ -451,51 +457,51 @@ void do_check(Opt_trace_context *trace,
TEST(TraceSettingsTest, offset)
{
Opt_trace_context trace;
- make_one_trace(trace, "100", -1 /* offset */, 1 /* limit */);
+ make_one_trace(&trace, "100", -1 /* offset */, 1 /* limit */);
const char *expected_traces0[]= {"100", NULL};
check(trace, expected_traces0);
- make_one_trace(trace, "101", -1, 1);
+ make_one_trace(&trace, "101", -1, 1);
/* 101 should have overwritten 100 */
const char *expected_traces1[]= {"101", NULL};
check(trace, expected_traces1);
- make_one_trace(trace, "102", -1, 1);
+ make_one_trace(&trace, "102", -1, 1);
const char *expected_traces2[]= {"102", NULL};
check(trace, expected_traces2);
check(trace, expected_traces2);
trace.reset();
const char *expected_traces_empty[]= {NULL};
check(trace, expected_traces_empty);
- make_one_trace(trace, "103", -3, 2);
- make_one_trace(trace, "104", -3, 2);
- make_one_trace(trace, "105", -3, 2);
- make_one_trace(trace, "106", -3, 2);
- make_one_trace(trace, "107", -3, 2);
- make_one_trace(trace, "108", -3, 2);
- make_one_trace(trace, "109", -3, 2);
+ make_one_trace(&trace, "103", -3, 2);
+ make_one_trace(&trace, "104", -3, 2);
+ make_one_trace(&trace, "105", -3, 2);
+ make_one_trace(&trace, "106", -3, 2);
+ make_one_trace(&trace, "107", -3, 2);
+ make_one_trace(&trace, "108", -3, 2);
+ make_one_trace(&trace, "109", -3, 2);
const char *expected_traces3[]= {"107", "108", NULL};
check(trace, expected_traces3);
trace.reset();
check(trace, expected_traces_empty);
- make_one_trace(trace, "110", 3, 2);
- make_one_trace(trace, "111", 3, 2);
- make_one_trace(trace, "112", 3, 2);
- make_one_trace(trace, "113", 3, 2);
- make_one_trace(trace, "114", 3, 2);
- make_one_trace(trace, "115", 3, 2);
- make_one_trace(trace, "116", 3, 2);
+ make_one_trace(&trace, "110", 3, 2);
+ make_one_trace(&trace, "111", 3, 2);
+ make_one_trace(&trace, "112", 3, 2);
+ make_one_trace(&trace, "113", 3, 2);
+ make_one_trace(&trace, "114", 3, 2);
+ make_one_trace(&trace, "115", 3, 2);
+ make_one_trace(&trace, "116", 3, 2);
const char *expected_traces10[]= {"113", "114", NULL};
check(trace, expected_traces10);
trace.reset();
check(trace, expected_traces_empty);
- make_one_trace(trace, "117", 0, 1);
- make_one_trace(trace, "118", 0, 1);
- make_one_trace(trace, "119", 0, 1);
+ make_one_trace(&trace, "117", 0, 1);
+ make_one_trace(&trace, "118", 0, 1);
+ make_one_trace(&trace, "119", 0, 1);
const char *expected_traces17[]= {"117", NULL};
check(trace, expected_traces17);
trace.reset();
- make_one_trace(trace, "120", 0, 0);
- make_one_trace(trace, "121", 0, 0);
- make_one_trace(trace, "122", 0, 0);
+ make_one_trace(&trace, "120", 0, 0);
+ make_one_trace(&trace, "121", 0, 0);
+ make_one_trace(&trace, "122", 0, 0);
const char *expected_traces20[]= {NULL};
check(trace, expected_traces20);
}
@@ -518,8 +524,8 @@ TEST(TraceSettingsTest, MaxMemSize)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -530,12 +536,12 @@ TEST(TraceSettingsTest, MaxMemSize)
Without truncation the trace would take:
2+17+3+1+20*100 = 2023
*/
- ASSERT_EQ((size_t)996, info.trace_length);
- ASSERT_EQ((size_t)1027, info.missing_bytes); // 996+1027=2023
- ASSERT_EQ(false, info.malloc_error);
- ASSERT_EQ(0, strncmp(expected, info.trace_ptr, sizeof(expected) - 1));
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ((size_t)996, info.trace_length);
+ EXPECT_EQ((size_t)1027, info.missing_bytes); // 996+1027=2023
+ EXPECT_EQ(false, info.malloc_error);
+ EXPECT_EQ(0, strncmp(expected, info.trace_ptr, sizeof(expected) - 1));
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -560,19 +566,19 @@ TEST(TraceSettingsTest, MaxMemSize2)
}
trace.end();
Opt_trace_iterator it(&trace);
- 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);
- ASSERT_EQ(false, info.malloc_error);
- it++;
- info= *it;
+ ASSERT_FALSE(it.at_end());
+ Opt_trace_info info= it.get_value();
+ EXPECT_EQ((size_t)17, info.trace_length);
+ EXPECT_EQ((size_t)16, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ info= it.get_value();
/* 2nd trace completely empty as first trace left no room */
- ASSERT_EQ((size_t)0, info.trace_length);
- ASSERT_EQ((size_t)33, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ((size_t)0, info.trace_length);
+ EXPECT_EQ((size_t)33, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_TRUE(it.at_end());
/*
3rd trace; the first one should automatically be purged, thus the 3rd
should have a bit of room.
@@ -584,22 +590,22 @@ TEST(TraceSettingsTest, MaxMemSize2)
}
trace.end();
Opt_trace_iterator it2(&trace);
- EXPECT_FALSE(it2.at_end());
- info= *it2;
- ASSERT_EQ((size_t)0, info.trace_length);
- ASSERT_EQ((size_t)33, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- it2++;
- info= *it2;
+ ASSERT_FALSE(it2.at_end());
+ info= it2.get_value();
+ EXPECT_EQ((size_t)0, info.trace_length);
+ EXPECT_EQ((size_t)33, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it2.next();
+ info= it2.get_value();
/*
3rd one had room. A bit less than first, because just reading the second
with the iterator has reallocated the second from 0 to 8 bytes...
*/
- ASSERT_EQ((size_t)14, info.trace_length);
- ASSERT_EQ((size_t)19, info.missing_bytes);
- ASSERT_EQ(false, info.malloc_error);
- it2++;
- EXPECT_TRUE(it2.at_end());
+ EXPECT_EQ((size_t)14, info.trace_length);
+ EXPECT_EQ((size_t)19, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it2.next();
+ ASSERT_TRUE(it2.at_end());
}
@@ -620,20 +626,20 @@ TEST_F(TraceContentTest, OutOfMemory)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
"\n" // 200.4 missing
" ]\n"
"}";
- ASSERT_STREQ(expected, info.trace_ptr);
- ASSERT_EQ(sizeof(expected) - 1, info.trace_length);
- ASSERT_EQ((size_t)0, info.missing_bytes);
- ASSERT_EQ(true, info.malloc_error); // malloc error reported
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_STREQ(expected, info.trace_ptr);
+ EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(true, info.malloc_error); // malloc error reported
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
#endif // !DBUG_OFF
@@ -671,8 +677,8 @@ TEST_F(TraceContentTest, FilteringByFeat
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
const char expected[]=
"{\n"
" \"one array\": [\n"
@@ -689,13 +695,13 @@ TEST_F(TraceContentTest, FilteringByFeat
" 4\n"
" ]\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);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -709,8 +715,8 @@ TEST_F(TraceContentTest, 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; // warning on windows: truncation of constant value
- all_chars[129]= 0xa4; // warning on windows: truncation of constant value
+ all_chars[128]= static_cast<char>(0xc3);
+ all_chars[129]= static_cast<char>(0xa4);
// all_chars is used both as query...
trace.set_query(all_chars, sizeof(all_chars), system_charset_info);
{
@@ -720,24 +726,24 @@ TEST_F(TraceContentTest, escaping)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
// we get the trace escaped, JSON-compliant:
const char expected[]=
"{\n"
" \"somekey\": \"\\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\u0008\\t\\n\\u000b\\u000c\\r\\u000e\\u000f\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001a\\u001b\\u001c\\u001d\\u001e\\u001f !\\\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~ä\"\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);
- ASSERT_EQ(sizeof(all_chars), info.query_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ EXPECT_EQ(sizeof(all_chars), info.query_length);
// we get the query unescaped, verbatim, not 0-terminated:
- ASSERT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
- ASSERT_EQ(system_charset_info, info.query_charset);
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
+ EXPECT_EQ(system_charset_info, info.query_charset);
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
@@ -781,23 +787,23 @@ TEST_F(TraceContentTest, NonUtf8)
}
trace.end();
Opt_trace_iterator it(&trace);
- EXPECT_FALSE(it.at_end());
- const Opt_trace_info info= *it;
+ ASSERT_FALSE(it.at_end());
+ const Opt_trace_info info= it.get_value();
// This is UTF8 and thus JSON-compliant; ABC is present
const char expected[]=
"{\n"
" \"somekey\": \"????ABC\"\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);
- ASSERT_EQ(sizeof(all_chars), info.query_length);
+ EXPECT_EQ((size_t)0, info.missing_bytes);
+ EXPECT_EQ(false, info.malloc_error);
+ EXPECT_EQ(sizeof(all_chars), info.query_length);
// we get the query unescaped, verbatim, not 0-terminated:
- ASSERT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
- it++;
- EXPECT_TRUE(it.at_end());
+ EXPECT_EQ(0, memcmp(all_chars, info.query_ptr, sizeof(all_chars)));
+ it.next();
+ ASSERT_TRUE(it.at_end());
}
} // namespace
Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110126105725-16cdxy0mok0fo9d4.bundle
| Thread |
|---|
| • bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3244) | Guilhem Bichot | 26 Jan |
