From e63bb9a4a7f30c5daf6b2bd27b4a913a390b3bbe Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Wed, 11 Mar 2020 16:36:33 +0300
Subject: [PATCH] Fix simpliest case of cyclic aliases in QueryNormalizer

---
 dbms/src/Interpreters/QueryNormalizer.cpp     | 17 +++++++++----
 .../tests/gtest_cycle_aliases.cpp             | 24 +++++++++++++++++++
 .../01097_cyclic_defaults.reference           |  1 +
 .../0_stateless/01097_cyclic_defaults.sql     | 15 ++++++++++++
 4 files changed, 52 insertions(+), 5 deletions(-)
 create mode 100644 dbms/src/Interpreters/tests/gtest_cycle_aliases.cpp
 create mode 100644 dbms/tests/queries/0_stateless/01097_cyclic_defaults.reference
 create mode 100644 dbms/tests/queries/0_stateless/01097_cyclic_defaults.sql

diff --git a/dbms/src/Interpreters/QueryNormalizer.cpp b/dbms/src/Interpreters/QueryNormalizer.cpp
index 363d5ba64b1..bb72a9649c6 100644
--- a/dbms/src/Interpreters/QueryNormalizer.cpp
+++ b/dbms/src/Interpreters/QueryNormalizer.cpp
@@ -91,14 +91,21 @@ void QueryNormalizer::visit(ASTIdentifier & node, ASTPtr & ast, Data & data)
             return;
         }
 
+        /// We are alias for other column (node.name), but we are alias by
+        /// ourselves to some other column
         auto & alias_node = it_alias->second;
 
-        /// Let's replace it with the corresponding tree node.
-        if (current_asts.count(alias_node.get()))
+        String our_alias_or_name = alias_node->getAliasOrColumnName();
+        std::optional<String> our_name = IdentifierSemantic::getColumnName(alias_node);
+
+        String node_alias = ast->tryGetAlias();
+
+        if (current_asts.count(alias_node.get()) /// We have loop of multiple aliases
+            || (node.name == our_alias_or_name && our_name && node_alias == *our_name)) /// Our alias points to node.name, direct loop
             throw Exception("Cyclic aliases", ErrorCodes::CYCLIC_ALIASES);
 
-        String my_alias = ast->tryGetAlias();
-        if (!my_alias.empty() && my_alias != alias_node->getAliasOrColumnName())
+        /// Let's replace it with the corresponding tree node.
+        if (!node_alias.empty() && node_alias != our_alias_or_name)
         {
             /// Avoid infinite recursion here
             auto opt_name = IdentifierSemantic::getColumnName(alias_node);
@@ -108,7 +115,7 @@ void QueryNormalizer::visit(ASTIdentifier & node, ASTPtr & ast, Data & data)
             {
                 /// 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 = alias_node->clone();
-                ast->setAlias(my_alias);
+                ast->setAlias(node_alias);
             }
         }
         else
diff --git a/dbms/src/Interpreters/tests/gtest_cycle_aliases.cpp b/dbms/src/Interpreters/tests/gtest_cycle_aliases.cpp
new file mode 100644
index 00000000000..c8037b23d84
--- /dev/null
+++ b/dbms/src/Interpreters/tests/gtest_cycle_aliases.cpp
@@ -0,0 +1,24 @@
+#include <gtest/gtest.h>
+
+#include <Interpreters/QueryNormalizer.h>
+#include <Parsers/IAST.h>
+#include <Parsers/queryToString.h>
+#include <Parsers/ExpressionListParsers.h>
+#include <Parsers/parseQuery.h>
+
+using namespace DB;
+
+TEST(QueryNormalizer, SimpleCycleAlias)
+{
+    String query = "a as b, b as a";
+    ParserExpressionList parser(false);
+    ASTPtr ast = parseQuery(parser, query, 0);
+
+    Aliases aliases;
+    aliases["a"] = parseQuery(parser, "b as a", 0)->children[0];
+    aliases["b"] = parseQuery(parser, "a as b", 0)->children[0];
+
+    Settings settings;
+    QueryNormalizer::Data normalizer_data(aliases, settings);
+    EXPECT_THROW(QueryNormalizer(normalizer_data).visit(ast), Exception);
+}
diff --git a/dbms/tests/queries/0_stateless/01097_cyclic_defaults.reference b/dbms/tests/queries/0_stateless/01097_cyclic_defaults.reference
new file mode 100644
index 00000000000..d00491fd7e5
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01097_cyclic_defaults.reference
@@ -0,0 +1 @@
+1
diff --git a/dbms/tests/queries/0_stateless/01097_cyclic_defaults.sql b/dbms/tests/queries/0_stateless/01097_cyclic_defaults.sql
new file mode 100644
index 00000000000..b0485f6d044
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01097_cyclic_defaults.sql
@@ -0,0 +1,15 @@
+DROP TABLE IF EXISTS table_with_cyclic_defaults;
+
+CREATE TABLE table_with_cyclic_defaults (a DEFAULT b, b DEFAULT a) ENGINE = Memory; --{serverError 174}
+
+CREATE TABLE table_with_cyclic_defaults (a DEFAULT b + 1, b DEFAULT a * a) ENGINE = Memory; --{serverError 174}
+
+CREATE TABLE table_with_cyclic_defaults (a DEFAULT b, b DEFAULT toString(c), c DEFAULT concat(a, '1')) ENGINE = Memory; --{serverError 174}
+
+CREATE TABLE table_with_cyclic_defaults (a DEFAULT b, b DEFAULT c, c DEFAULT a * b) ENGINE = Memory; --{serverError 174}
+
+CREATE TABLE table_with_cyclic_defaults (a String DEFAULT b, b String DEFAULT a) ENGINE = Memory; --{serverError 174}
+
+SELECT 1;
+
+DROP TABLE IF EXISTS table_with_cyclic_defaults;