database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-29 05:21:33 +03:00
Code Activity

Fixed valgrind error when storing db_name_length in query_cache.

Browse Source 
- Changed storage to be 2 bytes instead of sizeof(size_t) (simple optimization)
- Fixed bug when using query_cache_strip_comments and query that started with '('
- Fixed DBUG_PRINT() that used wrong (not initialized) variables.


mysql-test/mysql-test-run.pl:
  Added some space to make output more readable.
mysql-test/r/query_cache.result:
  Updated test results
mysql-test/t/query_cache.test:
  Added test with query_cache_strip_comments
sql/mysql_priv.h:
  Added QUERY_CACHE_DB_LENGTH_SIZE
sql/sql_cache.cc:
  Fixed bug when using query_cache_strip_comments and query that started with '('
  Store db length in 2 characters instead of size_t.
  Get db length from correct position (earlier we had an error when query started with ' ')
  Fixed DBUG_PRINT() that used wrong (not initialized) variables.
This commit is contained in:
Michael Widenius
2011-12-13 14:00:20 +02:00
parent 63d32c115d
commit b653115c8e
7 changed files with 81 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
mysql-test/mysql-test-run.pl
View File

@ -5145,10 +5145,10 @@ sub mysqld_start ($$) {
# Write a message about this to the normal log file # Write a message about this to the normal log file
my $trace_name= "$opt_vardir/log/".$mysqld->name().".trace"; my $trace_name= "$opt_vardir/log/".$mysqld->name().".trace";
mtr_tofile($output, mtr_tofile($output,
"NOTE: When running with --valgrind --debug the output from", "NOTE: When running with --valgrind --debug the output from ",
"mysqld(where the valgrind messages shows up) is stored ", "mysqld (where the valgrind messages shows up) is stored ",
"together with the trace file to make it ", "together with the trace file to make it ",
"easier to find the exact position of valgrind errors.", "easier to find the exact position of valgrind errors. ",
"See trace file $trace_name.\n"); "See trace file $trace_name.\n");
$output= $trace_name; $output= $trace_name;

33
mysql-test/r/query_cache.result
View File

@ -912,13 +912,13 @@ a
a a
show status like "Qcache_queries_in_cache"; show status like "Qcache_queries_in_cache";
Variable_name Value Variable_name Value
Qcache_queries_in_cache 0 Qcache_queries_in_cache 1
show status like "Qcache_inserts"; show status like "Qcache_inserts";
Variable_name Value Variable_name Value
Qcache_inserts 18 Qcache_inserts 19
show status like "Qcache_hits"; show status like "Qcache_hits";
Variable_name Value Variable_name Value
Qcache_hits 6 Qcache_hits 7
DROP TABLE t1; DROP TABLE t1;
SET GLOBAL query_cache_size=0; SET GLOBAL query_cache_size=0;
SET SESSION query_cache_type = 2; SET SESSION query_cache_type = 2;
@ -1070,10 +1070,10 @@ flush status;
a a
show status like "Qcache_queries_in_cache"; show status like "Qcache_queries_in_cache";
Variable_name Value Variable_name Value
Qcache_queries_in_cache 0 Qcache_queries_in_cache 1
show status like "Qcache_inserts"; show status like "Qcache_inserts";
Variable_name Value Variable_name Value
Qcache_inserts 0 Qcache_inserts 1
show status like "Qcache_hits"; show status like "Qcache_hits";
Variable_name Value Variable_name Value
Qcache_hits 0 Qcache_hits 0
@ -1081,13 +1081,30 @@ Qcache_hits 0
a a
show status like "Qcache_queries_in_cache"; show status like "Qcache_queries_in_cache";
Variable_name Value Variable_name Value
Qcache_queries_in_cache 0 Qcache_queries_in_cache 1
show status like "Qcache_inserts"; show status like "Qcache_inserts";
Variable_name Value Variable_name Value
Qcache_inserts 0 Qcache_inserts 1
show status like "Qcache_hits"; show status like "Qcache_hits";
Variable_name Value Variable_name Value
Qcache_hits 0 Qcache_hits 1
set global query_cache_strip_comments=1;
(select a from t1) union (select a from t1);
a
(select a from t1) /* */union (select a from t1);
a
set global query_cache_strip_comments=0;
(select a from t1) union (select a from t1);
a
show status like "Qcache_queries_in_cache";
Variable_name Value
Qcache_queries_in_cache 1
show status like "Qcache_inserts";
Variable_name Value
Qcache_inserts 1
show status like "Qcache_hits";
Variable_name Value
Qcache_hits 4
drop table t1; drop table t1;
create table t1 (a int); create table t1 (a int);
insert into t1 values (1),(2); insert into t1 values (1),(2);

12
mysql-test/t/query_cache.test
View File

@ -776,7 +776,19 @@ show status like "Qcache_hits";
show status like "Qcache_queries_in_cache"; show status like "Qcache_queries_in_cache";
show status like "Qcache_inserts"; show status like "Qcache_inserts";
show status like "Qcache_hits"; show status like "Qcache_hits";
set global query_cache_strip_comments=1;
(select a from t1) union (select a from t1);
(select a from t1) /* */union (select a from t1);
set global query_cache_strip_comments=0;
(select a from t1) union (select a from t1);
show status like "Qcache_queries_in_cache";
show status like "Qcache_inserts";
show status like "Qcache_hits";
drop table t1; drop table t1;
#
# SP cursors and selects with query cache (BUG#9715) # SP cursors and selects with query cache (BUG#9715)
# #
create table t1 (a int); create table t1 (a int);

1
sql/mysql_priv.h
View File

@ -1045,6 +1045,7 @@ struct Query_cache_query_flags
MY_LOCALE *lc_time_names; MY_LOCALE *lc_time_names;
}; };
#define QUERY_CACHE_FLAGS_SIZE sizeof(Query_cache_query_flags) #define QUERY_CACHE_FLAGS_SIZE sizeof(Query_cache_query_flags)
#define QUERY_CACHE_DB_LENGTH_SIZE 2
#include "sql_cache.h" #include "sql_cache.h"
#define query_cache_store_query(A, B) query_cache.store_query(A, B) #define query_cache_store_query(A, B) query_cache.store_query(A, B)
#define query_cache_destroy() query_cache.destroy() #define query_cache_destroy() query_cache.destroy()

11
sql/sp_head.cc
View File

@ -1024,17 +1024,18 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
buffer :== buffer :==
<statement> The input statement(s) <statement> The input statement(s)
'\0' Terminating null char '\0' Terminating null char
<length> Length of following current database name (size_t) <length> Length of following current database name 2
<db_name> Name of current database <db_name> Name of current database
<flags> Flags struct <flags> Flags struct
*/ */
buf_len= qbuf.length() + 1 + sizeof(size_t) + thd->db_length + buf_len= (qbuf.length() + 1 + QUERY_CACHE_DB_LENGTH_SIZE + thd->db_length +
QUERY_CACHE_FLAGS_SIZE + 1; QUERY_CACHE_FLAGS_SIZE + 1);
if ((pbuf= (char *) alloc_root(thd->mem_root, buf_len))) if ((pbuf= (char *) alloc_root(thd->mem_root, buf_len)))
{ {
char *ptr= pbuf + qbuf.length();
memcpy(pbuf, qbuf.ptr(), qbuf.length()); memcpy(pbuf, qbuf.ptr(), qbuf.length());
pbuf[qbuf.length()]= 0; *ptr= 0;
memcpy(pbuf+qbuf.length()+1, (char *) &thd->db_length, sizeof(size_t)); int2store(ptr+1, thd->db_length);
} }
else else
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);

45
sql/sql_cache.cc
View File

@ -462,6 +462,7 @@ static void make_base_query(String *new_query,
DBUG_ASSERT(query[query_length] == 0); DBUG_ASSERT(query[query_length] == 0);
DBUG_ASSERT(!is_white_space(query[0])); DBUG_ASSERT(!is_white_space(query[0]));
new_query->length(0); // Don't copy anything from old buffer
if (new_query->realloc(query_length + additional_length)) if (new_query->realloc(query_length + additional_length))
{ {
/* /*
@ -540,8 +541,11 @@ insert_space:
} }
if (buffer == last_space) if (buffer == last_space)
buffer--; // Remove the last space buffer--; // Remove the last space
*buffer= 0; *buffer= 0; // End zero after query
new_query->length((size_t) (buffer - new_query->ptr())); new_query->length((size_t) (buffer - new_query->ptr()));
/* Copy db_length */
memcpy(buffer+1, query_end+1, QUERY_CACHE_DB_LENGTH_SIZE);
} }
@ -1473,8 +1477,8 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
/* Key is query + database + flag */ /* Key is query + database + flag */
if (thd->db_length) if (thd->db_length)
{ {
memcpy((char*) (query + query_length + 1 + sizeof(size_t)), thd->db, memcpy((char*) (query + query_length + 1 + QUERY_CACHE_DB_LENGTH_SIZE),
thd->db_length); thd->db, thd->db_length);
DBUG_PRINT("qcache", ("database: %s length: %u", DBUG_PRINT("qcache", ("database: %s length: %u",
thd->db, (unsigned) thd->db_length)); thd->db, (unsigned) thd->db_length));
} }
@ -1482,8 +1486,8 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
{ {
DBUG_PRINT("qcache", ("No active database")); DBUG_PRINT("qcache", ("No active database"));
} }
tot_length= query_length + thd->db_length + 1 + sizeof(size_t) + tot_length= (query_length + thd->db_length + 1 +
QUERY_CACHE_FLAGS_SIZE; QUERY_CACHE_DB_LENGTH_SIZE + QUERY_CACHE_FLAGS_SIZE);
/* /*
We should only copy structure (don't use it location directly) We should only copy structure (don't use it location directly)
because of alignment issue because of alignment issue
@ -1642,7 +1646,7 @@ Query_cache::send_result_to_client(THD *thd, char *org_sql, uint query_length)
Query_cache_block_table *block_table, *block_table_end; Query_cache_block_table *block_table, *block_table_end;
ulong tot_length; ulong tot_length;
Query_cache_query_flags flags; Query_cache_query_flags flags;
const char *sql, *sql_end; const char *sql, *sql_end, *found_brace= 0;
DBUG_ENTER("Query_cache::send_result_to_client"); DBUG_ENTER("Query_cache::send_result_to_client");
/* /*
@ -1716,9 +1720,16 @@ Query_cache::send_result_to_client(THD *thd, char *org_sql, uint query_length)
case '\n': case '\n':
case '\t': case '\t':
case ' ': case ' ':
case '(': // To handle (select a from t1) union (select a from t1);
sql++; sql++;
continue; continue;
case '(': // To handle (select a from t1) union (select a from t1);
if (!found_brace)
{
found_brace= sql;
sql++;
continue;
}
/* fall trough */
default: default:
break; break;
} }
@ -1752,8 +1763,7 @@ Query_cache::send_result_to_client(THD *thd, char *org_sql, uint query_length)
sure the new current database has a name with the same length sure the new current database has a name with the same length
as the previous one. as the previous one.
*/ */
size_t db_len; size_t db_len= uint2korr(sql_end+1);
memcpy((char *) &db_len, (sql + query_length + 1), sizeof(size_t));
if (thd->db_length != db_len) if (thd->db_length != db_len)
{ {
/* /*
@ -1788,8 +1798,11 @@ Query_cache::send_result_to_client(THD *thd, char *org_sql, uint query_length)
Query_cache_block *query_block; Query_cache_block *query_block;
if (opt_query_cache_strip_comments) if (opt_query_cache_strip_comments)
{ {
if (found_brace)
sql= found_brace;
make_base_query(&thd->base_query, sql, (size_t) (sql_end - sql), make_base_query(&thd->base_query, sql, (size_t) (sql_end - sql),
thd->db_length + 1 + QUERY_CACHE_FLAGS_SIZE); thd->db_length + 1 + QUERY_CACHE_DB_LENGTH_SIZE +
QUERY_CACHE_FLAGS_SIZE);
sql= thd->base_query.ptr(); sql= thd->base_query.ptr();
query_length= thd->base_query.length(); query_length= thd->base_query.length();
} }
@ -1799,15 +1812,15 @@ Query_cache::send_result_to_client(THD *thd, char *org_sql, uint query_length)
thd->base_query.set(sql, query_length, system_charset_info); thd->base_query.set(sql, query_length, system_charset_info);
} }
tot_length= query_length + 1 + sizeof(size_t) + tot_length= (query_length + 1 + QUERY_CACHE_DB_LENGTH_SIZE +
thd->db_length + QUERY_CACHE_FLAGS_SIZE; thd->db_length + QUERY_CACHE_FLAGS_SIZE);
if (thd->db_length) if (thd->db_length)
{ {
memcpy((char*) (sql+query_length+1+ sizeof(size_t)), thd->db, memcpy((uchar*) sql + query_length + 1 + QUERY_CACHE_DB_LENGTH_SIZE,
thd->db_length); thd->db, thd->db_length);
DBUG_PRINT("qcache", ("database: '%s' length: %u", DBUG_PRINT("qcache", ("database: '%s' length: %u",
thd->db, (unsigned)thd->db_length)); thd->db, (uint) thd->db_length));
} }
else else
{ {
@ -1929,7 +1942,7 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
{ {
DBUG_PRINT("qcache", DBUG_PRINT("qcache",
("Temporary table detected: '%s.%s'", ("Temporary table detected: '%s.%s'",
table_list.db, table_list.alias)); tmptable->s->db.str, tmptable->alias.c_ptr()));
unlock(); unlock();
/* /*
We should not store result of this query because it contain We should not store result of this query because it contain

10
sql/sql_parse.cc
View File

@ -508,10 +508,10 @@ static void handle_bootstrap_impl(THD *thd)
query= (char *) thd->memdup_w_gap(buff, length + 1, query= (char *) thd->memdup_w_gap(buff, length + 1,
thd->db_length + 1 + thd->db_length + 1 +
QUERY_CACHE_DB_LENGTH_SIZE +
QUERY_CACHE_FLAGS_SIZE); QUERY_CACHE_FLAGS_SIZE);
size_t db_len= 0;
memcpy(query + length + 1, (char *) &db_len, sizeof(size_t));
thd->set_query(query, length); thd->set_query(query, length);
int2store(query + length + 1, 0); // No db in bootstrap
DBUG_PRINT("query",("%-.4096s", thd->query())); DBUG_PRINT("query",("%-.4096s", thd->query()));
#if defined(ENABLED_PROFILING) && defined(COMMUNITY_SERVER) #if defined(ENABLED_PROFILING) && defined(COMMUNITY_SERVER)
thd->profiling.start_new_query(); thd->profiling.start_new_query();
@ -1880,7 +1880,8 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length)
*/ */
if (! (query= (char*) thd->memdup_w_gap(packet, if (! (query= (char*) thd->memdup_w_gap(packet,
packet_length, packet_length,
1 + sizeof(size_t) + thd->db_length + 1 + thd->db_length +
QUERY_CACHE_DB_LENGTH_SIZE +
QUERY_CACHE_FLAGS_SIZE))) QUERY_CACHE_FLAGS_SIZE)))
return TRUE; return TRUE;
query[packet_length]= '\0'; query[packet_length]= '\0';
@ -1889,8 +1890,7 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length)
also store this length, in case current database is changed during also store this length, in case current database is changed during
execution. We might need to reallocate the 'query' buffer execution. We might need to reallocate the 'query' buffer
*/ */
char *len_pos = (query + packet_length + 1); int2store(query + packet_length + 1, thd->db_length);
memcpy(len_pos, (char *) &thd->db_length, sizeof(size_t));
thd->set_query(query, packet_length); thd->set_query(query, packet_length);