Fixed access privilege bug in query cache.

Change tests to use database 'mysqltest' instead of 'foo'
Add option to not print access denied messages to check_table_access()


Docs/manual.texi:
  Updated query cache information.
client/mysqltest.c:
  When connection to the database '*NO-ONE*, connect without specify a database.
include/raid.h:
  Portability fix
mysql-test/r/drop.result:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/r/flush.result:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/r/innodb.result:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/r/query_cache.result:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/t/drop.test:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/t/flush.test:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/t/innodb.test:
  Change test to use database 'mysqltest' instead of 'foo'
mysql-test/t/query_cache.test:
  Change test to use database 'mysqltest' instead of 'foo'
sql/mysql_priv.h:
  Add option to not print access denied messages
sql/net_pkg.cc:
  Fix for query cache
sql/net_serv.cc:
  Speed improvment
sql/sql_cache.cc:
  Fixed access privilege bug in query cache
sql/sql_cache.h:
  Always compiled with integrity check function prototypes.
sql/sql_do.cc:
  Removed warning
sql/sql_parse.cc:
  Add option to not print access denied messages

Docs/manual.texi

@@ -35574,6 +35574,10 @@ In addition, a query may be seen as different if for instance one
 client is using a new communication protocol format or another
 character set than another client.
 
+Queries that uses different databases, uses different protocol versions
+or the uses different default character sets are considered different
+queries and cached separately.
+
 The cache does work for @code{SELECT CALC_ROWS ...} and
 @code{SELECT FOUND_ROWS() ...} type queries because the number of
 found rows is also stored in the cache.
@@ -35612,8 +35616,12 @@ of the form @code{SELECT * FROM AUTOINCREMENT_FIELD IS NULL}
 However, @code{FOUND ROWS()} will return the correct value,
 even if the preceding query was fetched from the cache.
 
-Queries that don't use any tables are not cached.
+Queries that don't use any tables or if the user has a column privilege for
+any of the involved tables are not cached.
 
+Before a query is fetched from the query cache, MySQL will check that
+the user has SELECT privilege to all the involved databases and
+tables. If this is not the case, the cached result will not be used.
 
 @node Query Cache Configuration, Query Cache in SELECT, Query Cache How, Query Cache
 @subsection Query Cache Configuration

client/mysqltest.c

@@ -1248,7 +1248,8 @@ int close_connection(struct st_query* q)
 }
 
 
-/* this one now is a hack - we may want to improve in in the
+/*
+   This one now is a hack - we may want to improve in in the
    future to handle quotes. For now we assume that anything that is not
    a comma, a space or ) belongs to the argument. space is a chopper, comma or
    ) are delimiters/terminators
@@ -1291,8 +1292,7 @@ int safe_connect(MYSQL* con, const char* host, const char* user,
   int i;
   for (i = 0; i < MAX_CON_TRIES; ++i)
   {
-    if(mysql_real_connect(con, host,user, pass,
-			  db, port, sock, 0))
+    if (mysql_real_connect(con, host,user, pass, db, port, sock, 0))
     {
       con_error = 0;
       break;
@@ -1365,6 +1365,9 @@ int do_connect(struct st_query* q)
     con_sock=fn_format(buff, con_sock, TMPDIR, "",0);
   if (!con_db[0])
     con_db=db;
+  /* Special database to allow one to connect without a database name */
+  if (!strcmp(con_db,"*NO-ONE*"))
+    con_db=0;
   if ((con_error = safe_connect(&next_con->mysql, con_host,
 				con_user, con_pass,
 				con_db, con_port, con_sock ? con_sock: 0)))

include/raid.h

@@ -22,8 +22,10 @@
 #define RAID_DEFAULT_CHUNKS 4
 #define RAID_DEFAULT_CHUNKSIZE 256*1024 /* 256kB */
 
-extern const char *raid_type_string[];
+C_MODE_START
 #define my_raid_type(raid_type)  raid_type_string[(int)(raid_type)]
+extern const char *raid_type_string[];
+C_MODE_END
 
 #if defined(USE_RAID) && !defined(DONT_USE_RAID)
 

mysql-test/r/drop.result

@@ -12,37 +12,37 @@ drop table t1;
 select * from t1;
 n
 1
-drop database if exists foo;
-create database foo;
-drop database if exists foo;
-create database foo;
-create table foo.foo (n int);
-insert into foo.foo values (4);
-select * from foo.foo;
+drop database if exists mysqltest;
+create database mysqltest;
+drop database if exists mysqltest;
+create database mysqltest;
+create table mysqltest.mysqltest (n int);
+insert into mysqltest.mysqltest values (4);
+select * from mysqltest.mysqltest;
 n
 4
-drop database if exists foo;
-create database foo;
-drop database foo;
-drop database if exists foo;
+drop database if exists mysqltest;
+create database mysqltest;
+drop database mysqltest;
+drop database if exists mysqltest;
 flush tables with read lock;
-create database foo;
+create database mysqltest;
 Got one of the listed errors
 unlock tables;
-create database foo;
+create database mysqltest;
 show databases;
 Database
-foo
 mysql
+mysqltest
 test
 flush tables with read lock;
-drop database foo;
+drop database mysqltest;
 Got one of the listed errors
 unlock tables;
-drop database foo;
+drop database mysqltest;
 show databases;
 Database
 mysql
 test
-drop database foo;
-Can't drop database 'foo'. Database doesn't exist
+drop database mysqltest;
+Can't drop database 'mysqltest'. Database doesn't exist

mysql-test/r/flush.result

@@ -11,13 +11,13 @@ drop table t2;
 Table 't2' was locked with a READ lock and can't be updated
  drop table t2;
 unlock tables;
-drop database if exists foo;
-create database foo;
-create table foo.t1(n int);
-insert into foo.t1 values (23);
+drop database if exists mysqltest;
+create database mysqltest;
+create table mysqltest.t1(n int);
+insert into mysqltest.t1 values (23);
 flush tables with read lock;
- drop database foo;
-select * from foo.t1;
+ drop database mysqltest;
+select * from mysqltest.t1;
 n
 23
 unlock tables;

mysql-test/r/grant_cache.result

+drop table if exists test.t1,mysqltest.t1,mysqltest.t2;
+reset query cache;
+flush status;
+create database if not exists mysqltest;
+create table mysqltest.t1 (a int,b int,c int);
+create table mysqltest.t2 (a int,b int,c int);
+insert into mysqltest.t1 values (1,1,1),(2,2,2);
+insert into mysqltest.t2 values (3,3,3);
+create table test.t1 (a char (10));
+insert into test.t1 values ("test.t1");
+select * from t1;
+a
+test.t1
+select * from t1;
+a	b	c
+1	1	1
+2	2	2
+select a from t1;
+a
+1
+2
+select c from t1;
+c
+1
+2
+select * from t2;
+a	b	c
+3	3	3
+select * from mysqltest.t1,test.t1;
+a	b	c	a
+1	1	1	test.t1
+2	2	2	test.t1
+show status like "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	6
+show status like "Qcache_hits%";
+Variable_name	Value
+Qcache_hits	0
+grant SELECT on mysqltest.* to mysqltest_1@localhost;
+grant SELECT on mysqltest.t1 to mysqltest_2@localhost;
+grant SELECT on test.t1 to mysqltest_2@localhost;
+grant SELECT(a) on mysqltest.t1 to mysqltest_3@localhost;
+select "user1";
+user1
+user1
+select * from t1;
+a	b	c
+1	1	1
+2	2	2
+select a from t1 ;
+a
+1
+2
+select c from t1;
+c
+1
+2
+show status like "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	6
+show status like "Qcache_hits";
+Variable_name	Value
+Qcache_hits	3
+show status like "Qcache_not_cached";
+Variable_name	Value
+Qcache_not_cached	1
+select "user2";
+user2
+user2
+select * from t1;
+a	b	c
+1	1	1
+2	2	2
+select a from t1;
+a
+1
+2
+select c from t1;
+c
+1
+2
+select * from mysqltest.t1,test.t1;
+a	b	c	a
+1	1	1	test.t1
+2	2	2	test.t1
+select * from t2;
+select command denied to user: 'mysqltest_2@localhost' for table 't2'
+show status like "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	6
+show status like "Qcache_hits";
+Variable_name	Value
+Qcache_hits	7
+show status like "Qcache_not_cached";
+Variable_name	Value
+Qcache_not_cached	3
+select "user3";
+user3
+user3
+select * from t1;
+select command denied to user: 'mysqltest_3@localhost' for column 'b' in table 't1'
+select a from t1;
+a
+1
+2
+select c from t1;
+select command denied to user: 'mysqltest_3@localhost' for column 'c' in table 't1'
+select * from t2;
+select command denied to user: 'mysqltest_3@localhost' for table 't2'
+select mysqltest.t1.c from test.t1,mysqltest.t1;
+select command denied to user: 'mysqltest_3@localhost' for column 'c' in table 't1'
+show status like "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	6
+show status like "Qcache_hits";
+Variable_name	Value
+Qcache_hits	7
+show status like "Qcache_not_cached";
+Variable_name	Value
+Qcache_not_cached	8
+select "user4";
+user4
+user4
+select a from t1;
+No Database Selected
+select * from mysqltest.t1,test.t1;
+a	b	c	a
+1	1	1	test.t1
+2	2	2	test.t1
+select a from mysqltest.t1;
+a
+1
+2
+select a from mysqltest.t1;
+a
+1
+2
+show status like "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	8
+show status like "Qcache_hits";
+Variable_name	Value
+Qcache_hits	8
+show status like "Qcache_not_cached";
+Variable_name	Value
+Qcache_not_cached	9
+delete from mysql.user where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.db where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.tables_priv where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.columns_priv where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+flush privileges;
+drop table test.t1,mysqltest.t1,mysqltest.t2;
+drop database mysqltest;

mysql-test/r/innodb.result

@@ -848,16 +848,16 @@ id	name	value	uid
 3	three	three value	103
 6	two	other value	102
 drop table t1;
-create database test_$1;
-create table test_$1.t1 (a int not null) type= innodb;
-insert into test_$1.t1 values(1);
-create table test_$1.t2 (a int not null) type= myisam;
-insert into test_$1.t2 values(1);
-create table test_$1.t3 (a int not null) type= heap;
-insert into test_$1.t3 values(1);
+create database mysqltest;
+create table mysqltest.t1 (a int not null) type= innodb;
+insert into mysqltest.t1 values(1);
+create table mysqltest.t2 (a int not null) type= myisam;
+insert into mysqltest.t2 values(1);
+create table mysqltest.t3 (a int not null) type= heap;
+insert into mysqltest.t3 values(1);
 commit;
-drop database test_$1;
-show tables from test_$1;
+drop database mysqltest;
+show tables from mysqltest;
 Got one of the listed errors
 create table t1 (a int not null) type= innodb;
 insert into t1 values(1),(2);

mysql-test/r/query_cache.result

@@ -346,19 +346,19 @@ show status like "Qcache_queries_in_cache";
 Variable_name	Value
 Qcache_queries_in_cache	0
 drop table t1,t2;
-create database foo;
-create table foo.t1 (i int not null auto_increment, a int, primary key (i));
-insert into foo.t1 (a) values (1);
-select * from foo.t1 where i is null;
+create database mysqltest;
+create table mysqltest.t1 (i int not null auto_increment, a int, primary key (i));
+insert into mysqltest.t1 (a) values (1);
+select * from mysqltest.t1 where i is null;
 i	a
 1	1
-select * from foo.t1;
+select * from mysqltest.t1;
 i	a
 1	1
 show status like "Qcache_queries_in_cache";
 Variable_name	Value
 Qcache_queries_in_cache	1
-drop database foo;
+drop database mysqltest;
 show status like "Qcache_queries_in_cache";
 Variable_name	Value
 Qcache_queries_in_cache	0

mysql-test/t/drop.test

@@ -11,33 +11,33 @@ create table t1(n int);
 drop table t1;
 select * from t1;
 
-#now test for a bug in drop database - it is important that the name
-#of the table is the same as the name of the database - in the original
-#code this triggered a bug
-drop database if exists foo;
-create database foo;
-drop database if exists foo;
-create database foo;
-create table foo.foo (n int);
-insert into foo.foo values (4);
-select * from foo.foo;
-drop database if exists foo;
-create database foo;
-drop database foo;
+# now test for a bug in drop database - it is important that the name
+# of the table is the same as the name of the database - in the original
+# code this triggered a bug
+drop database if exists mysqltest;
+create database mysqltest;
+drop database if exists mysqltest;
+create database mysqltest;
+create table mysqltest.mysqltest (n int);
+insert into mysqltest.mysqltest values (4);
+select * from mysqltest.mysqltest;
+drop database if exists mysqltest;
+create database mysqltest;
+drop database mysqltest;
 
 # test drop/create database and FLUSH TABLES WITH READ LOCK
-drop database if exists foo;
+drop database if exists mysqltest;
 flush tables with read lock;
 --error 1209,1223;
-create database foo;
+create database mysqltest;
 unlock tables;
-create database foo;
+create database mysqltest;
 show databases;
 flush tables with read lock;
 --error 1208,1223;
-drop database foo;
+drop database mysqltest;
 unlock tables;
-drop database foo;
+drop database mysqltest;
 show databases;
 --error 1008
-drop database foo;
+drop database mysqltest;

mysql-test/t/flush.test

@@ -44,15 +44,15 @@ reap;
 
 #test if drop database will wait until we release the global read lock
 connection con1;
-drop database if exists foo;
-create database foo;
-create table foo.t1(n int);
-insert into foo.t1 values (23);
+drop database if exists mysqltest;
+create database mysqltest;
+create table mysqltest.t1(n int);
+insert into mysqltest.t1 values (23);
 flush tables with read lock;
 connection con2;
-send drop database foo;
+send drop database mysqltest;
 connection con1;
-select * from foo.t1;
+select * from mysqltest.t1;
 unlock tables;
 connection con2;
 reap;

mysql-test/t/grant_cache-master.opt

+--set-variable=query_cache_size=1355776

mysql-test/t/grant_cache.test

+#
+# Test grants with query cache
+#
+drop table if exists test.t1,mysqltest.t1,mysqltest.t2;
+reset query cache;
+flush status;
+connect (root,localhost,root,,test,0,master.sock);
+connection root;
+create database if not exists mysqltest;
+
+create table mysqltest.t1 (a int,b int,c int);
+create table mysqltest.t2 (a int,b int,c int);
+insert into mysqltest.t1 values (1,1,1),(2,2,2);
+insert into mysqltest.t2 values (3,3,3);
+create table test.t1 (a char (10));
+insert into test.t1 values ("test.t1");
+select * from t1;
+connect (root2,localhost,root,,mysqltest,0,master.sock);
+connection root2;
+# put queries in cache
+select * from t1;
+select a from t1;
+select c from t1;
+select * from t2;
+select * from mysqltest.t1,test.t1;
+show status like "Qcache_queries_in_cache";
+show status like "Qcache_hits%";
+
+# Create the test users
+grant SELECT on mysqltest.* to mysqltest_1@localhost;
+grant SELECT on mysqltest.t1 to mysqltest_2@localhost;
+grant SELECT on test.t1 to mysqltest_2@localhost;
+grant SELECT(a) on mysqltest.t1 to mysqltest_3@localhost;
+
+# The following queries should be fetched from cache
+connect (user1,localhost,mysqltest_1,,mysqltest,0,master.sock);
+connection user1;
+select "user1";
+select * from t1;
+# The pre and end space are intentional
+ select a from t1 ;
+select c from t1;
+show status like "Qcache_queries_in_cache";
+show status like "Qcache_hits";
+show status like "Qcache_not_cached";
+
+# The following queries should be fetched from cache
+connect (user2,localhost,mysqltest_2,,mysqltest,0,master.sock);
+connection user2;
+select "user2";
+select * from t1;
+select a from t1;
+select c from t1;
+select * from mysqltest.t1,test.t1;
+--error 1142
+select * from t2;
+show status like "Qcache_queries_in_cache";
+show status like "Qcache_hits";
+show status like "Qcache_not_cached";
+
+# The following queries should not be fetched from cache
+connect (user3,localhost,mysqltest_3,,mysqltest,0,master.sock);
+connection user3;
+select "user3";
+--error 1143
+select * from t1;
+select a from t1;
+--error 1143
+select c from t1;
+--error 1142
+select * from t2;
+--error 1143
+select mysqltest.t1.c from test.t1,mysqltest.t1;
+show status like "Qcache_queries_in_cache";
+show status like "Qcache_hits";
+show status like "Qcache_not_cached";
+
+# Connect without a database
+connect (user4,localhost,mysqltest_1,,*NO-ONE*,0,master.sock);
+connection user4;
+select "user4";
+--error 1046
+select a from t1;
+# The following query is not cached before (different database)
+select * from mysqltest.t1,test.t1;
+# Cache a query with 'no database'
+select a from mysqltest.t1;
+select a from mysqltest.t1;
+show status like "Qcache_queries_in_cache";
+show status like "Qcache_hits";
+show status like "Qcache_not_cached";
+
+# Cleanup
+
+connection root;
+delete from mysql.user where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.db where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.tables_priv where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+delete from mysql.columns_priv where user in ("mysqltest_1","mysqltest_2","mysqltest_3");
+flush privileges;
+drop table test.t1,mysqltest.t1,mysqltest.t2;
+drop database mysqltest;

mysql-test/t/innodb.test

@@ -518,18 +518,18 @@ drop table t1;
 # Test DROP DATABASE
 #
 
-create database test_$1;
-create table test_$1.t1 (a int not null) type= innodb;
-insert into test_$1.t1 values(1);
-create table test_$1.t2 (a int not null) type= myisam;
-insert into test_$1.t2 values(1);
-create table test_$1.t3 (a int not null) type= heap;
-insert into test_$1.t3 values(1);
+create database mysqltest;
+create table mysqltest.t1 (a int not null) type= innodb;
+insert into mysqltest.t1 values(1);
+create table mysqltest.t2 (a int not null) type= myisam;
+insert into mysqltest.t2 values(1);
+create table mysqltest.t3 (a int not null) type= heap;
+insert into mysqltest.t3 values(1);
 commit;
-drop database test_$1;
+drop database mysqltest;
 # Don't check error message
 --error 12,12
-show tables from test_$1;
+show tables from mysqltest;
 
 #
 # Test truncate table

mysql-test/t/query_cache.test

@@ -235,17 +235,17 @@ drop table t1,t2;
 #
 # noncachable ODBC work around (and prepare cache for drop database)
 #
-create database foo;
-create table foo.t1 (i int not null auto_increment, a int, primary key (i));
-insert into foo.t1 (a) values (1);
-select * from foo.t1 where i is null;
+create database mysqltest;
+create table mysqltest.t1 (i int not null auto_increment, a int, primary key (i));
+insert into mysqltest.t1 (a) values (1);
+select * from mysqltest.t1 where i is null;
 
 #
 # drop db
 #
-select * from foo.t1;
+select * from mysqltest.t1;
 show status like "Qcache_queries_in_cache";
-drop database foo;
+drop database mysqltest;
 show status like "Qcache_queries_in_cache";
 
 #

sql/mysql_priv.h

@@ -283,8 +283,9 @@ uint cached_tables(void);
 void kill_mysql(void);
 void close_connection(NET *net,uint errcode=0,bool lock=1);
 bool check_access(THD *thd,uint access,const char *db=0,uint *save_priv=0,
-		  bool no_grant=0);
-bool check_table_access(THD *thd,uint want_access, TABLE_LIST *tables);
+		  bool no_grant=0, bool no_errors=0);
+bool check_table_access(THD *thd,uint want_access, TABLE_LIST *tables,
+			bool no_errors=0);
 bool check_process_priv(THD *thd=0);
 
 int mysql_backup_table(THD* thd, TABLE_LIST* table_list);

sql/net_pkg.cc

@@ -30,6 +30,7 @@ void send_error(NET *net, uint sql_errno, const char *err)
 		      err ? err : net->last_error[0] ?
 		      net->last_error : "NULL")); 
 
+  query_cache_abort(net);
   if (thd)
     thd->query_error = 1; // needed to catch query errors during replication
   if (!err)
@@ -102,9 +103,9 @@ net_printf(NET *net, uint errcode, ...)
   DBUG_ENTER("net_printf");
   DBUG_PRINT("enter",("message: %u",errcode));
 
-  if(thd) thd->query_error = 1;
-  // if we are here, something is wrong :-)
-
+  if (thd)
+    thd->query_error = 1;	// if we are here, something is wrong :-)
+  query_cache_abort(net);	// Safety
   va_start(args,errcode);
   format=ER(errcode);
   offset= net->return_errno ? 2 : 0;

sql/net_serv.cc

@@ -330,7 +330,8 @@ net_real_write(NET *net,const char *packet,ulong len)
   DBUG_ENTER("net_real_write");
 
 #ifdef MYSQL_SERVER
-  query_cache_insert(net, packet, len);
+  if (net->query_cache_query != 0)
+    query_cache_insert(net, packet, len);
 #endif
 
   if (net->error == 2)

sql/sql_cache.cc

-/* Copyright (C) 2000 MySQL AB & MySQL Finland AB & TCX DataKonsult AB
+/* Copyright (C) 2000 MySQL AB
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -530,9 +530,14 @@ byte *query_cache_query_get_key(const byte *record, uint *length,
 }
 
 /*****************************************************************************
- Functions to store things into the query cache
+  Functions to store things into the query cache
 *****************************************************************************/
 
+/*
+  Insert the packet into the query cache.
+  This should only be called if net->query_cache_query != 0
+*/
+
 void query_cache_insert(NET *net, const char *packet, ulong length)
 {
   DBUG_ENTER("query_cache_insert");
@@ -543,45 +548,41 @@ void query_cache_insert(NET *net, const char *packet, ulong length)
     DBUG_VOID_RETURN;
 #endif
 
-  // Quick check on unlocked structure
-  if (net->query_cache_query != 0)
+  STRUCT_LOCK(&query_cache.structure_guard_mutex);
+  Query_cache_block *query_block = ((Query_cache_block*)
+				    net->query_cache_query);
+  if (query_block)
   {
-    STRUCT_LOCK(&query_cache.structure_guard_mutex);
-    Query_cache_block *query_block = ((Query_cache_block*)
-				      net->query_cache_query);
-    if (query_block)
-    {
-      Query_cache_query *header = query_block->query();
-      Query_cache_block *result = header->result();
+    Query_cache_query *header = query_block->query();
+    Query_cache_block *result = header->result();
 
-      DUMP(&query_cache);
-      BLOCK_LOCK_WR(query_block);
-      DBUG_PRINT("qcache", ("insert packet %lu bytes long",length));
+    DUMP(&query_cache);
+    BLOCK_LOCK_WR(query_block);
+    DBUG_PRINT("qcache", ("insert packet %lu bytes long",length));
 
-      /*
-	On success STRUCT_UNLOCK(&query_cache.structure_guard_mutex) will be
-	done by query_cache.append_result_data if success (if not we need
-	query_cache.structure_guard_mutex locked to free query)
-      */
-      if (!query_cache.append_result_data(&result, length, (gptr) packet,
-					  query_block))
-      {
-	query_cache.refused++;
-	DBUG_PRINT("warning", ("Can't append data"));
-	header->result(result);
-	DBUG_PRINT("qcache", ("free query 0x%lx", (ulong) query_block));
-	// The following call will remove the lock on query_block
-	query_cache.free_query(query_block);
-	// append_result_data no success => we need unlock
-	STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
-	DBUG_VOID_RETURN;
-      }
+    /*
+      On success STRUCT_UNLOCK(&query_cache.structure_guard_mutex) will be
+      done by query_cache.append_result_data if success (if not we need
+      query_cache.structure_guard_mutex locked to free query)
+    */
+    if (!query_cache.append_result_data(&result, length, (gptr) packet,
+					query_block))
+    {
+      query_cache.refused++;
+      DBUG_PRINT("warning", ("Can't append data"));
       header->result(result);
-      BLOCK_UNLOCK_WR(query_block);
-    }
-    else
+      DBUG_PRINT("qcache", ("free query 0x%lx", (ulong) query_block));
+      // The following call will remove the lock on query_block
+      query_cache.free_query(query_block);
+      // append_result_data no success => we need unlock
       STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
+      DBUG_VOID_RETURN;
+    }
+    header->result(result);
+    BLOCK_UNLOCK_WR(query_block);
   }
+  else
+    STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
   DBUG_EXECUTE("check_querycache",query_cache.check_integrity(););
   DBUG_VOID_RETURN;
 }
@@ -607,11 +608,11 @@ void query_cache_abort(NET *net)
       BLOCK_LOCK_WR(query_block);
       // The following call will remove the lock on query_block
       query_cache.free_query(query_block);
-      net->query_cache_query=0;
     }
+    net->query_cache_query=0;
     STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
+    DBUG_EXECUTE("check_querycache",query_cache.check_integrity(););
   }
-  DBUG_EXECUTE("check_querycache",query_cache.check_integrity(););
   DBUG_VOID_RETURN;
 }
 
@@ -648,7 +649,6 @@ void query_cache_end_of_result(NET *net)
 #endif
       header->found_rows(current_thd->limit_found_rows);
       header->result()->type = Query_cache_block::RESULT;
-      net->query_cache_query=0;
       header->writer(0);
       BLOCK_UNLOCK_WR(query_block);
     }
@@ -658,8 +658,8 @@ void query_cache_end_of_result(NET *net)
       STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
     }
     net->query_cache_query=0;
+    DBUG_EXECUTE("check_querycache",query_cache.check_integrity(););
   }
-  DBUG_EXECUTE("check_querycache",query_cache.check_integrity(););
   DBUG_VOID_RETURN;
 }
 
@@ -724,6 +724,7 @@ void Query_cache::store_query(THD *thd, TABLE_LIST *tables_used)
     (Not important at this stage)
   */
   TABLE_COUNTER_TYPE tables;
+  ulong tot_length;
   DBUG_ENTER("Query_cache::store_query");
   if (query_cache_size == 0)
     DBUG_VOID_RETURN;
@@ -739,6 +740,17 @@ void Query_cache::store_query(THD *thd, TABLE_LIST *tables_used)
       DBUG_VOID_RETURN;
     DUMP(this);
 
+    /* Key is query + database + flag */
+    if (thd->db_length)
+    {
+      memcpy(thd->query+thd->query_length, thd->db, thd->db_length);
+      DBUG_PRINT("qcache", ("database : %s length %u",
+			    thd->db, thd->db_length)); 
+    }
+    else
+    {
+      DBUG_PRINT("qcache", ("No active database"));
+    }
     /*
        Prepare flags:
        most significant bit - CLIENT_LONG_FLAG,
@@ -749,32 +761,19 @@ void Query_cache::store_query(THD *thd, TABLE_LIST *tables_used)
       flags|= (byte) thd->convert_set->number();
       DBUG_ASSERT(thd->convert_set->number() < 128);
     }
+    tot_length=thd->query_length+1+thd->db_length;
+    thd->query[tot_length-1] = (char) flags;
 
     /* Check if another thread is processing the same query? */
-    thd->query[thd->query_length] = (char) flags;
-    if (thd->db_length)
-    {
-      memcpy(thd->query+thd->query_length+1, thd->db, thd->db_length);
-      DBUG_PRINT("qcache", ("database : %s length %u",
-			    thd->db, thd->db_length)); 
-    }
-    else
-    {
-      DBUG_PRINT("qcache", ("No active database"));
-    }
-      
     Query_cache_block *competitor = (Query_cache_block *)
-      hash_search(&queries, (byte*) thd->query,
-		  thd->query_length+1+thd->db_length);
+      hash_search(&queries, (byte*) thd->query, tot_length);
     DBUG_PRINT("qcache", ("competitor 0x%lx, flags %x", (ulong) competitor,
 			flags));
     if (competitor == 0)
     {
       /* Query is not in cache and no one is working with it; Store it */
-      thd->query[thd->query_length] = (char) flags;
       Query_cache_block *query_block;
-      query_block= write_block_data(thd->query_length+1+thd->db_length,
-				    (gptr) thd->query,
+      query_block= write_block_data(tot_length, (gptr) thd->query,
 				    ALIGN_SIZE(sizeof(Query_cache_query)),
 				    Query_cache_block::QUERY, tables, 1);
       if (query_block != 0)
@@ -801,7 +800,7 @@ void Query_cache::store_query(THD *thd, TABLE_LIST *tables_used)
 	  header->unlock_n_destroy();
 	  free_memory_block(query_block);
 	  STRUCT_UNLOCK(&structure_guard_mutex);
-	  DBUG_VOID_RETURN;
+	  goto end;
 	}
 	double_linked_list_simple_include(query_block, &queries_blocks);
 	inserts++;
@@ -837,13 +836,26 @@ end:
   DBUG_VOID_RETURN;
 }
 
+/*
+  Check if the query is in the cache. If it was cached, send it
+  to the user.
+
+  RESULTS
+        1	Query was not cached.
+	0	The query was cached and user was sent the result.
+	-1	The query was cached but we didn't have rights to use it.
+		No error is sent to the client yet.
+*/
+  
+
 
-my_bool
+int
 Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
 {
   Query_cache_query *query;
   Query_cache_block *first_result_block, *result_block;
   Query_cache_block_table *block_table, *block_table_end;
+  ulong tot_length;
   byte flags;
   DBUG_ENTER("Query_cache::send_result_to_client");
 
@@ -856,9 +868,13 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
       thd->query_cache_type == 0)
 
   {
-    DBUG_PRINT("qcache", ("query cache disabled on not in autocommit mode"));
+    DBUG_PRINT("qcache", ("query cache disabled or not in autocommit mode"));
     goto err;
   }
+
+  /* Check that we haven't forgot to reset the query cache variables */
+  DBUG_ASSERT(thd->net.query_cache_query == 0);
+
   /*
     We can't cache the query if we are using a temporary table because
     we don't know if the query is using a temporary table.
@@ -868,7 +884,9 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
   */
   if (thd->temporary_tables != 0 || !thd->safe_to_cache_query)
   {
-    DBUG_PRINT("qcache", ("SELECT is non-cacheable"));
+    DBUG_PRINT("qcache", ("SELECT is non-cacheable: tmp_tables: %d  safe: %d",
+			  thd->temporary_tables,
+			  thd->safe_to_cache_query));
     goto err;
   }
 
@@ -888,13 +906,22 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
   STRUCT_LOCK(&structure_guard_mutex);
   if (query_cache_size == 0)
   {
-    DBUG_PRINT("qcache", ("query cache disabled and not in autocommit mode"));
-    STRUCT_UNLOCK(&structure_guard_mutex);
-    goto err;
+    DBUG_PRINT("qcache", ("query cache disabled"));
+    goto err_unlock;
   }
-  DBUG_PRINT("qcache", (" sql %u '%s'", query_length, sql));
   Query_cache_block *query_block;
 
+  tot_length=query_length+thd->db_length+1;
+  if (thd->db_length)
+  {
+    memcpy(sql+query_length, thd->db, thd->db_length);
+    DBUG_PRINT("qcache", ("database: '%s' length %u",
+			  thd->db, thd->db_length));
+  }
+  else
+  {
+    DBUG_PRINT("qcache", ("No active database"));
+  }
   /*
      prepare flags:
      Most significant bit - CLIENT_LONG_FLAG,
@@ -906,31 +933,19 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
     flags |= (byte) thd->convert_set->number();
     DBUG_ASSERT(thd->convert_set->number() < 128);
   }
-  sql[query_length] = (char) flags;
-  if (thd->db_length)
-  {
-    memcpy(sql+query_length+1, thd->db, thd->db_length);
-    DBUG_PRINT("qcache", ("database : %s length %u",
-			  thd->db, thd->db_length));
-  }
-  else
-  {
-    DBUG_PRINT("qcache", ("No active database"));
-  }
+  sql[tot_length-1] = (char) flags;
   query_block = (Query_cache_block *)  hash_search(&queries, (byte*) sql,
-						   query_length+1+
-						   thd->db_length);
+						   tot_length);
   
-  sql[query_length] = '\0';
+  sql[query_length] = '\0';			// Restore end null
 
   /* Quick abort on unlocked data */
   if (query_block == 0 ||
       query_block->query()->result() == 0 ||
       query_block->query()->result()->type != Query_cache_block::RESULT)
   {
-    STRUCT_UNLOCK(&structure_guard_mutex);
     DBUG_PRINT("qcache", ("No query in query hash or no results"));
-    goto err;
+    goto err_unlock;
   }
   DBUG_PRINT("qcache", ("Query in query hash 0x%lx", (ulong)query_block));
 
@@ -945,7 +960,7 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
     /* The query is probably yet processed */
     DBUG_PRINT("qcache", ("query found, but no data or data incomplete"));
     BLOCK_UNLOCK_RD(query_block);
-    goto err;
+    goto err_unlock;
   }
   DBUG_PRINT("qcache", ("Query have result 0x%lx", (ulong) query));
 
@@ -960,14 +975,24 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
     Query_cache_table *table = block_table->parent;
     table_list.db = table->db();
     table_list.name = table_list.real_name = table->table();
-    if (check_table_access(thd,SELECT_ACL,&table_list))
+    if (check_table_access(thd,SELECT_ACL,&table_list,1))
     {
       DBUG_PRINT("qcache",
 		 ("probably no SELECT access to %s.%s =>  return to normal processing",
 		  table_list.db, table_list.name));
-      BLOCK_UNLOCK_RD(query_block);
+      refused++;				// This is actually a hit
       STRUCT_UNLOCK(&structure_guard_mutex);
-      goto err;
+      thd->safe_to_cache_query=0;		// Don't try to cache this
+      BLOCK_UNLOCK_RD(query_block);
+      DBUG_RETURN(-1);				// Privilege error
+    }
+    if (table_list.grant.want_privilege)
+    {
+      DBUG_PRINT("qcache", ("Need to check column privileges for %s.%s",
+			    table_list.db, table_list.name));
+      BLOCK_UNLOCK_RD(query_block);
+      thd->safe_to_cache_query=0;		// Don't try to cache this
+      goto err_unlock;				// Parse query
     }
   }
   move_to_query_list_end(query_block);
@@ -996,10 +1021,12 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
   thd->limit_found_rows = query->found_rows();
 
   BLOCK_UNLOCK_RD(query_block);
-  DBUG_RETURN(0);
+  DBUG_RETURN(1);				// Result sent to client
 
+err_unlock:
+  STRUCT_UNLOCK(&structure_guard_mutex);
 err:
-  DBUG_RETURN(1);
+  DBUG_RETURN(0);				// Query was not cached
 }
 
 /*
@@ -2307,7 +2334,7 @@ void Query_cache::double_linked_list_join(Query_cache_block *head_tail,
 *****************************************************************************/
 
 /*
-  if query is cacheable return number tables in query
+  If query is cacheable return number tables in query
   (query without tables are not cached)
 */
 
@@ -2554,10 +2581,12 @@ my_bool Query_cache::move_by_type(byte **border,
     Query_cache_query *new_query= ((Query_cache_query *) new_block->data());
     pthread_cond_init(&new_query->lock, NULL);
     pthread_mutex_init(&new_query->clients_guard,MY_MUTEX_INIT_FAST);
+
     NET *net = new_block->query()->writer();
+    /* QQ: When could this happen ? */
     if (net != 0)
     {
-      net->query_cache_query = (gptr) new_block;
+      net->query_cache_query= (gptr) new_block;
     }
     /* Fix hash to point at moved block */
     hash_replace(&queries, queries.current_record, (byte*) new_block);
@@ -2747,7 +2776,19 @@ uint Query_cache::filename_2_table_key (char *key, const char *path)
   Functions to be used when debugging
 ****************************************************************************/
 
-#ifndef DBUG_OFF
+#if defined(DBUG_OFF) && !defined(USE_QUERY_CACHE_INTEGRITY_CHECK)
+
+void wreck(uint line, const char *message) {}
+void bins_dump() {}
+void cache_dump() {}
+void queries_dump() {}
+void tables_dump() {}
+my_bool check_integrity() {}
+my_bool in_list(Query_cache_block * root, Query_cache_block * point,
+		const char *name) { return 0;}
+my_bool in_blocks(Query_cache_block * point) { return 0; }
+
+#else
 
 void Query_cache::wreck(uint line, const char *message)
 {
@@ -2836,10 +2877,10 @@ void Query_cache::queries_dump()
     {
       uint len;
       char *str = (char*) query_cache_query_get_key((byte*) block, &len, 0);
-      byte flags = (byte) str[len-1];
+      uint flags = (uint) (uchar) str[len-1];
       DBUG_PRINT("qcache", ("%u (%u,%u) %.*s",len,
 			  ((flags & QUERY_CACHE_CLIENT_LONG_FLAG_MASK)? 1:0),
-			  (flags & QUERY_CACHE_CHARSET_CONVERT_MASK), len,
+			  (flags & QUERY_CACHE_CHARSET_CONVERT_MASK), len-1,
 			    str));
       DBUG_PRINT("qcache", ("-b- 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx", (ulong) block,
 			    (ulong) block->next, (ulong) block->prev,

sql/sql_cache.h

@@ -354,7 +354,7 @@ protected:
     Check if the query is in the cache and if this is true send the
     data to client.
   */
-  my_bool send_result_to_client(THD *thd, char *query, uint query_length);
+  int send_result_to_client(THD *thd, char *query, uint query_length);
 
   /* Remove all queries that uses any of the listed following tables */
   void invalidate(TABLE_LIST *tables_used);
@@ -375,7 +375,15 @@ protected:
 
   void destroy();
 
-#ifndef DBUG_OFF
+  friend void query_cache_insert(NET *net, const char *packet, ulong length);
+  friend void query_cache_end_of_result(NET *net);
+  friend void query_cache_abort(NET *net);
+
+  /*
+    The following functions are only used when debugging
+    We don't protect these with ifndef DEBUG_OFF to not have to recompile
+    everything if we want to add checks of the cache at some places.
+  */
   void wreck(uint line, const char *message);
   void bins_dump();
   void cache_dump();
@@ -385,10 +393,6 @@ protected:
   my_bool in_list(Query_cache_block * root, Query_cache_block * point,
 		  const char *name);
   my_bool in_blocks(Query_cache_block * point);
-#endif
-  friend void query_cache_insert(NET *net, const char *packet, ulong length);
-  friend void query_cache_end_of_result(NET *net);
-  friend void query_cache_abort(NET *net);
 };
 
 extern Query_cache query_cache;

sql/sql_do.cc

@@ -22,7 +22,6 @@
 
 int mysql_do(THD *thd, List<Item> &values)
 {
-  int error;
   List_iterator<Item> li(values);
   Item *value;
   DBUG_ENTER("mysql_do");

sql/sql_parse.cc

@@ -842,7 +842,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   {
     char *pos=packet-1+packet_length;		// Point at end null
     /* Remove garage at end of query */
-    while (packet_length > 0 && pos[-1] == ';')
+    while (packet_length > 0 && (pos[-1] == ';' || isspace(pos[-1])))
     {
       pos--;
       packet_length--;
@@ -2261,7 +2261,7 @@ error:
 
 bool
 check_access(THD *thd,uint want_access,const char *db, uint *save_priv,
-	     bool dont_check_global_grants)
+	     bool dont_check_global_grants, bool no_errors)
 {
   uint db_access,dummy;
   if (save_priv)
@@ -2271,7 +2271,8 @@ check_access(THD *thd,uint want_access,const char *db, uint *save_priv,
 
   if ((!db || !db[0]) && !thd->db && !dont_check_global_grants)
   {
-    send_error(&thd->net,ER_NO_DB_ERROR);	/* purecov: tested */
+    if (!no_errors)
+      send_error(&thd->net,ER_NO_DB_ERROR);	/* purecov: tested */
     return TRUE;				/* purecov: tested */
   }
 
@@ -2283,10 +2284,11 @@ check_access(THD *thd,uint want_access,const char *db, uint *save_priv,
   if ((want_access & ~thd->master_access) & ~(DB_ACLS | EXTRA_ACL) ||
       ! db && dont_check_global_grants)
   {						// We can never grant this
-    net_printf(&thd->net,ER_ACCESS_DENIED_ERROR,
-	       thd->priv_user,
-	       thd->host_or_ip,
-	       thd->password ? ER(ER_YES) : ER(ER_NO));/* purecov: tested */
+    if (!no_errors)
+      net_printf(&thd->net,ER_ACCESS_DENIED_ERROR,
+		 thd->priv_user,
+		 thd->host_or_ip,
+		 thd->password ? ER(ER_YES) : ER(ER_NO));/* purecov: tested */
     return TRUE;				/* purecov: tested */
   }
 
@@ -2306,10 +2308,11 @@ check_access(THD *thd,uint want_access,const char *db, uint *save_priv,
       ((grant_option && !dont_check_global_grants) &&
        !(want_access & ~TABLE_ACLS)))
     return FALSE;				/* Ok */
-  net_printf(&thd->net,ER_DBACCESS_DENIED_ERROR,
-	     thd->priv_user,
-	     thd->host_or_ip,
-	     db ? db : thd->db ? thd->db : "unknown"); /* purecov: tested */
+  if (!no_errors)
+    net_printf(&thd->net,ER_DBACCESS_DENIED_ERROR,
+	       thd->priv_user,
+	       thd->host_or_ip,
+	       db ? db : thd->db ? thd->db : "unknown"); /* purecov: tested */
   return TRUE;					/* purecov: tested */
 }
 
@@ -2326,7 +2329,8 @@ bool check_process_priv(THD *thd)
 */
 
 bool
-check_table_access(THD *thd,uint want_access,TABLE_LIST *tables)
+check_table_access(THD *thd,uint want_access,TABLE_LIST *tables,
+		   bool no_errors)
 {
   uint found=0,found_access=0;
   TABLE_LIST *org_tables=tables;
@@ -2341,18 +2345,20 @@ check_table_access(THD *thd,uint want_access,TABLE_LIST *tables)
 	tables->grant.privilege=found_access;
       else
       {
-	if (check_access(thd,want_access,tables->db,&tables->grant.privilege))
+	if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
+			 0, no_errors))
 	  return TRUE;				// Access denied
 	found_access=tables->grant.privilege;
 	found=1;
       }
     }
-    else if (check_access(thd,want_access,tables->db,&tables->grant.privilege))
+    else if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
+			  0, no_errors))
       return TRUE;				// Access denied
   }
   if (grant_option)
     return check_grant(thd,want_access & ~EXTRA_ACL,org_tables,
-		       test(want_access & EXTRA_ACL));
+		       test(want_access & EXTRA_ACL), no_errors);
   return FALSE;
 }
 
@@ -2474,6 +2480,7 @@ mysql_init_query(THD *thd)
   thd->fatal_error=0;				// Safety
   thd->last_insert_id_used=thd->query_start_used=thd->insert_id_used=0;
   thd->sent_row_count=thd->examined_row_count=0;
+  thd->safe_to_cache_query=1;
   DBUG_VOID_RETURN;
 }
 
@@ -2522,9 +2529,8 @@ mysql_parse(THD *thd,char *inBuf,uint length)
 
   mysql_init_query(thd);
   thd->query_length = length;
-  if (query_cache.send_result_to_client(thd, inBuf, length))
+  if (query_cache.send_result_to_client(thd, inBuf, length) <= 0)
   {
-    thd->safe_to_cache_query=1;
     LEX *lex=lex_start(thd, (uchar*) inBuf, length);
     if (!yyparse() && ! thd->fatal_error)
     {