mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2024-11-23 16:12:01 +00:00
Merge pull request #64199 from rschu1ze/fix-64136
Query Cache: Consider identical queries against different databases as different
This commit is contained in:
commit
0bfa56b468
@ -177,6 +177,21 @@ ASTPtr removeQueryCacheSettings(ASTPtr ast)
|
||||
return transformed_ast;
|
||||
}
|
||||
|
||||
IAST::Hash calculateAstHash(ASTPtr ast, const String & current_database)
|
||||
{
|
||||
ast = removeQueryCacheSettings(ast);
|
||||
|
||||
/// Hash the AST, it must consider aliases (issue #56258)
|
||||
SipHash hash;
|
||||
ast->updateTreeHash(hash, /*ignore_aliases=*/ false);
|
||||
|
||||
/// Also hash the database specified via SQL `USE db`, otherwise identifiers in same query (AST) may mean different columns in different
|
||||
/// tables (issue #64136)
|
||||
hash.update(current_database);
|
||||
|
||||
return getSipHash128AsPair(hash);
|
||||
}
|
||||
|
||||
String queryStringFromAST(ASTPtr ast)
|
||||
{
|
||||
WriteBufferFromOwnString buf;
|
||||
@ -186,17 +201,15 @@ String queryStringFromAST(ASTPtr ast)
|
||||
|
||||
}
|
||||
|
||||
/// Hashing of ASTs must consider aliases (issue #56258)
|
||||
static constexpr bool ignore_aliases = false;
|
||||
|
||||
QueryCache::Key::Key(
|
||||
ASTPtr ast_,
|
||||
const String & current_database,
|
||||
Block header_,
|
||||
std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_,
|
||||
bool is_shared_,
|
||||
std::chrono::time_point<std::chrono::system_clock> expires_at_,
|
||||
bool is_compressed_)
|
||||
: ast_hash(removeQueryCacheSettings(ast_)->getTreeHash(ignore_aliases))
|
||||
: ast_hash(calculateAstHash(ast_, current_database))
|
||||
, header(header_)
|
||||
, user_id(user_id_)
|
||||
, current_user_roles(current_user_roles_)
|
||||
@ -207,8 +220,8 @@ QueryCache::Key::Key(
|
||||
{
|
||||
}
|
||||
|
||||
QueryCache::Key::Key(ASTPtr ast_, std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_)
|
||||
: QueryCache::Key(ast_, {}, user_id_, current_user_roles_, false, std::chrono::system_clock::from_time_t(1), false) /// dummy values for everything != AST or user name
|
||||
QueryCache::Key::Key(ASTPtr ast_, const String & current_database, std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_)
|
||||
: QueryCache::Key(ast_, current_database, {}, user_id_, current_user_roles_, false, std::chrono::system_clock::from_time_t(1), false) /// dummy values for everything != AST, current database, user name/roles
|
||||
{
|
||||
}
|
||||
|
||||
|
@ -88,6 +88,7 @@ public:
|
||||
|
||||
/// Ctor to construct a Key for writing into query cache.
|
||||
Key(ASTPtr ast_,
|
||||
const String & current_database,
|
||||
Block header_,
|
||||
std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_,
|
||||
bool is_shared_,
|
||||
@ -95,7 +96,7 @@ public:
|
||||
bool is_compressed);
|
||||
|
||||
/// Ctor to construct a Key for reading from query cache (this operation only needs the AST + user name).
|
||||
Key(ASTPtr ast_, std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_);
|
||||
Key(ASTPtr ast_, const String & current_database, std::optional<UUID> user_id_, const std::vector<UUID> & current_user_roles_);
|
||||
|
||||
bool operator==(const Key & other) const;
|
||||
};
|
||||
|
@ -1101,7 +1101,7 @@ static std::tuple<ASTPtr, BlockIO> executeQueryImpl(
|
||||
{
|
||||
if (can_use_query_cache && settings.enable_reads_from_query_cache)
|
||||
{
|
||||
QueryCache::Key key(ast, context->getUserID(), context->getCurrentRoles());
|
||||
QueryCache::Key key(ast, context->getCurrentDatabase(), context->getUserID(), context->getCurrentRoles());
|
||||
QueryCache::Reader reader = query_cache->createReader(key);
|
||||
if (reader.hasCacheEntryForKey())
|
||||
{
|
||||
@ -1224,7 +1224,7 @@ static std::tuple<ASTPtr, BlockIO> executeQueryImpl(
|
||||
&& (!ast_contains_system_tables || system_table_handling == QueryCacheSystemTableHandling::Save))
|
||||
{
|
||||
QueryCache::Key key(
|
||||
ast, res.pipeline.getHeader(),
|
||||
ast, context->getCurrentDatabase(), res.pipeline.getHeader(),
|
||||
context->getUserID(), context->getCurrentRoles(),
|
||||
settings.query_cache_share_between_users,
|
||||
std::chrono::system_clock::now() + std::chrono::seconds(settings.query_cache_ttl),
|
||||
|
@ -0,0 +1,2 @@
|
||||
1
|
||||
2
|
30
tests/queries/0_stateless/02494_query_cache_use_database.sql
Normal file
30
tests/queries/0_stateless/02494_query_cache_use_database.sql
Normal file
@ -0,0 +1,30 @@
|
||||
-- Tags: no-parallel, no-fasttest
|
||||
-- Tag no-fasttest: Depends on OpenSSL
|
||||
-- Tag no-parallel: Messes with internal cache
|
||||
|
||||
-- Test for issue #64136
|
||||
|
||||
SYSTEM DROP QUERY CACHE;
|
||||
|
||||
DROP DATABASE IF EXISTS db1;
|
||||
DROP DATABASE IF EXISTS db2;
|
||||
|
||||
CREATE DATABASE db1;
|
||||
CREATE DATABASE db2;
|
||||
|
||||
CREATE TABLE db1.tab(a UInt64, PRIMARY KEY a);
|
||||
CREATE TABLE db2.tab(a UInt64, PRIMARY KEY a);
|
||||
|
||||
INSERT INTO db1.tab values(1);
|
||||
INSERT INTO db2.tab values(2);
|
||||
|
||||
USE db1;
|
||||
SELECT * FROM tab SETTINGS use_query_cache=1;
|
||||
|
||||
USE db2;
|
||||
SELECT * FROM tab SETTINGS use_query_cache=1;
|
||||
|
||||
DROP DATABASE db1;
|
||||
DROP DATABASE db2;
|
||||
|
||||
SYSTEM DROP QUERY CACHE;
|
Loading…
Reference in New Issue
Block a user