From 7bc58340b082c1d4822f48ac7b36bddf37b584f5 Mon Sep 17 00:00:00 2001 From: Vitaliy Lyudvichenko Date: Wed, 11 Oct 2017 16:33:27 +0300 Subject: [PATCH] Fixed infinite recursion in expression analyzer. [#CLICKHOUSE-3125] --- dbms/src/Interpreters/ExpressionAnalyzer.cpp | 32 ++++++++++++------- .../00494_alias_substitution_bug.reference | 3 ++ .../00494_alias_substitution_bug.sql | 4 +++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/dbms/src/Interpreters/ExpressionAnalyzer.cpp b/dbms/src/Interpreters/ExpressionAnalyzer.cpp index bdfaf4f6713..5d31e33c188 100644 --- a/dbms/src/Interpreters/ExpressionAnalyzer.cpp +++ b/dbms/src/Interpreters/ExpressionAnalyzer.cpp @@ -1014,7 +1014,7 @@ void ExpressionAnalyzer::normalizeTreeImpl( * For example, in the table there is a column "domain(URL)", and we requested domain(URL). */ String function_string = func_node->getColumnName(); - NamesAndTypesList::const_iterator it = findColumn(function_string); + auto it = findColumn(function_string); if (columns.end() != it) { ast = std::make_shared(func_node->range, function_string); @@ -1051,24 +1051,34 @@ void ExpressionAnalyzer::normalizeTreeImpl( if (identifier_node->kind == ASTIdentifier::Column) { /// If it is an alias, but not a parent alias (for constructs like "SELECT column + 1 AS column"). - Aliases::const_iterator jt = aliases.find(identifier_node->name); - if (jt != aliases.end() && current_alias != identifier_node->name) + auto it_alias = aliases.find(identifier_node->name); + if (it_alias != aliases.end() && current_alias != identifier_node->name) { /// Let's replace it with the corresponding tree node. - if (current_asts.count(jt->second.get())) + if (current_asts.count(it_alias->second.get())) throw Exception("Cyclic aliases", ErrorCodes::CYCLIC_ALIASES); - if (!my_alias.empty() && my_alias != jt->second->getAliasOrColumnName()) + + if (!my_alias.empty() && my_alias != it_alias->second->getAliasOrColumnName()) { - /// In a construct like "a AS b", where a is an alias, you must set alias b to the result of substituting alias a. - ast = jt->second->clone(); - ast->setAlias(my_alias); + /// Avoid infinite recursion here + auto replace_to_identifier = typeid_cast(it_alias->second.get()); + bool is_cycle = replace_to_identifier && + replace_to_identifier->kind == ASTIdentifier::Column && + replace_to_identifier->name == identifier_node->name; + + if (!is_cycle) + { + /// In a construct like "a AS b", where a is an alias, you must set alias b to the result of substituting alias a. + ast = it_alias->second->clone(); + ast->setAlias(my_alias); + replaced = true; + } } else { - ast = jt->second; + ast = it_alias->second; + replaced = true; } - - replaced = true; } } } diff --git a/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.reference b/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.reference index 6ed281c757a..f69e99fe66b 100644 --- a/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.reference +++ b/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.reference @@ -1,2 +1,5 @@ 1 1 +0 0 +0 0 0 +0 0 0 diff --git a/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.sql b/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.sql index 81c4b1a39a4..2f1e1a7c744 100644 --- a/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.sql +++ b/dbms/tests/queries/0_stateless/00494_alias_substitution_bug.sql @@ -3,3 +3,7 @@ CREATE TABLE test.nested (n Nested(x UInt8)) ENGINE = Memory; INSERT INTO test.nested VALUES ([1, 2]); SELECT 1 AS x FROM remote('127.0.0.1', test.nested) ARRAY JOIN n.x; DROP TABLE test.nested; + +SELECT dummy AS dummy, dummy AS b FROM system.one; +SELECT dummy AS dummy, dummy AS b, b AS c FROM system.one; +SELECT b AS c, dummy AS b, dummy AS dummy FROM system.one;