Merge pull request #71254 from rschu1ze/fix-69010

Query cache: Improve handling of system table names in literals
This commit is contained in:
Robert Schulze 2024-11-01 13:38:22 +00:00 committed by GitHub
commit 97530704e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 40 additions and 5 deletions

View File

@ -89,11 +89,40 @@ struct HasSystemTablesMatcher
{
database_table = identifier->name();
}
/// Handle SELECT [...] FROM clusterAllReplicas(<cluster>, '<table>')
else if (const auto * literal = node->as<ASTLiteral>())
/// SELECT [...] FROM clusterAllReplicas(<cluster>, '<table>')
/// This SQL syntax is quite common but we need to be careful. A naive attempt to cast 'node' to an ASTLiteral will be too general
/// and introduce false positives in queries like
/// 'SELECT * FROM users WHERE name = 'system.metrics' SETTINGS use_query_cache = true;'
/// Therefore, make sure we are really in `clusterAllReplicas`. EXPLAIN AST for
/// 'SELECT * FROM clusterAllReplicas('default', system.one) SETTINGS use_query_cache = 1'
/// returns:
/// [...]
/// Function clusterAllReplicas (children 1)
/// ExpressionList (children 2)
/// Literal 'test_shard_localhost'
/// Literal 'system.one'
/// [...]
else if (const auto * function = node->as<ASTFunction>())
{
const auto & value = literal->value;
database_table = toString(value);
if (function->name == "clusterAllReplicas")
{
const ASTs & function_children = function->children;
if (!function_children.empty())
{
if (const auto * expression_list = function_children[0]->as<ASTExpressionList>())
{
const ASTs & expression_list_children = expression_list->children;
if (expression_list_children.size() >= 2)
{
if (const auto * literal = expression_list_children[1]->as<ASTLiteral>())
{
const auto & value = literal->value;
database_table = toString(value);
}
}
}
}
}
}
Tokens tokens(database_table.c_str(), database_table.c_str() + database_table.size(), /*max_query_size*/ 2048, /*skip_insignificant*/ true);

View File

@ -44,9 +44,16 @@ SELECT * SETTINGS use_query_cache = 1;
SELECT * FROM information_schema.tables SETTINGS use_query_cache = 1; -- { serverError QUERY_CACHE_USED_WITH_SYSTEM_TABLE }
SELECT * FROM INFORMATION_SCHEMA.TABLES SETTINGS use_query_cache = 1; -- { serverError QUERY_CACHE_USED_WITH_SYSTEM_TABLE }
-- Issue #69010: A system table name appears as a literal. That's okay and must not throw.
DROP TABLE IF EXISTS tab;
CREATE TABLE tab (uid Int16, name String) ENGINE = Memory;
SELECT * FROM tab WHERE name = 'system.one' SETTINGS use_query_cache = true;
DROP TABLE tab;
-- System tables can be "hidden" inside e.g. table functions
SELECT * FROM clusterAllReplicas('test_shard_localhost', system.one) SETTINGS use_query_cache = 1; -- {serverError QUERY_CACHE_USED_WITH_SYSTEM_TABLE }
SELECT * FROM clusterAllReplicas('test_shard_localhost', 'system.one') SETTINGS use_query_cache = 1; -- {serverError QUERY_CACHE_USED_WITH_SYSTEM_TABLE }
-- Note how in the previous query ^^ 'system.one' is also a literal. ClusterAllReplicas gets special handling.
-- Criminal edge case that a user creates a table named "system". The query cache must not reject queries against it.
DROP TABLE IF EXISTS system;
@ -60,5 +67,4 @@ CREATE TABLE system.system (c UInt64) ENGINE = Memory;
SElECT * FROM system.system SETTINGS use_query_cache = 1; -- { serverError QUERY_CACHE_USED_WITH_SYSTEM_TABLE }
DROP TABLE system.system;
-- Cleanup
SYSTEM DROP QUERY CACHE;