From fa9399d82607e3f799bae53a5770a70b7dad22cb Mon Sep 17 00:00:00 2001
From: Alexander Kuzmenkov <akuzm@yandex-team.ru>
Date: Mon, 30 Mar 2020 21:00:38 +0300
Subject: [PATCH] [demo] Create unique columns for literals

---
 dbms/src/Interpreters/ActionsVisitor.cpp      | 43 +++++++++++++------
 dbms/src/Parsers/ASTLiteral.h                 |  2 +
 .../01101_literal_column_clash.reference      |  3 ++
 .../01101_literal_column_clash.sql            | 10 +++++
 4 files changed, 46 insertions(+), 12 deletions(-)
 create mode 100644 dbms/tests/queries/0_stateless/01101_literal_column_clash.reference
 create mode 100644 dbms/tests/queries/0_stateless/01101_literal_column_clash.sql

diff --git a/dbms/src/Interpreters/ActionsVisitor.cpp b/dbms/src/Interpreters/ActionsVisitor.cpp
index 4e008a81973..b2091465be9 100644
--- a/dbms/src/Interpreters/ActionsVisitor.cpp
+++ b/dbms/src/Interpreters/ActionsVisitor.cpp
@@ -197,10 +197,17 @@ SetPtr makeExplicitSet(
 
 static String getUniqueName(const Block & block, const String & prefix)
 {
-    int i = 1;
-    while (block.has(prefix + toString(i)))
-        ++i;
-    return prefix + toString(i);
+    auto result = prefix;
+
+    if (block.has(prefix))
+    {
+        int i = 1;
+        while (block.has(prefix + toString(i)))
+            ++i;
+        result = prefix + "_" + toString(i);
+    }
+
+    return result;
 }
 
 ScopeStack::ScopeStack(const ExpressionActionsPtr & actions, const Context & context_)
@@ -431,7 +438,6 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data &
     for (size_t arg = 0; arg < node.arguments->children.size(); ++arg)
     {
         auto & child = node.arguments->children[arg];
-        auto child_column_name = child->getColumnName();
 
         const auto * lambda = child->as<ASTFunction>();
         const auto * identifier = child->as<ASTIdentifier>();
@@ -461,7 +467,7 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data &
             if (!prepared_set->empty())
                 column.name = getUniqueName(data.getSampleBlock(), "__set");
             else
-                column.name = child_column_name;
+                column.name = child->getColumnName();
 
             if (!data.hasColumn(column.name))
             {
@@ -496,6 +502,18 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data &
         {
             /// If the argument is not a lambda expression, call it recursively and find out its type.
             visit(child, data);
+
+            // In the above visit() call, if the argument is a literal, we
+            // generated a unique column name for it. Use it instead of a generic
+            // display name.
+            auto child_column_name = child->getColumnName();
+            auto asLiteral = dynamic_cast<const ASTLiteral *>(child.get());
+            if (asLiteral)
+            {
+                assert(!asLiteral->unique_column_name.empty());
+                child_column_name = asLiteral->unique_column_name;
+            }
+
             if (data.hasColumn(child_column_name))
             {
                 argument_types.push_back(data.getSampleBlock().getByName(child_column_name).type);
@@ -587,18 +605,19 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data &
     }
 }
 
-void ActionsMatcher::visit(const ASTLiteral & literal, const ASTPtr & ast, Data & data)
+void ActionsMatcher::visit(const ASTLiteral & literal, const ASTPtr & /* ast */,
+    Data & data)
 {
-    CachedColumnName column_name;
-    if (data.hasColumn(column_name.get(ast)))
-        return;
-
     DataTypePtr type = applyVisitor(FieldToDataType(), literal.value);
 
     ColumnWithTypeAndName column;
     column.column = type->createColumnConst(1, convertFieldToType(literal.value, *type));
     column.type = type;
-    column.name = column_name.get(ast);
+
+    // Always create columns for literals with a unique name. Otherwise, there
+    // may be some weird clashes, see 01101_literal_column_clash.
+    column.name = getUniqueName(data.getSampleBlock(), literal.getColumnName());
+    const_cast<ASTLiteral &>(literal).unique_column_name = column.name;
 
     data.addAction(ExpressionAction::addColumn(column));
 }
diff --git a/dbms/src/Parsers/ASTLiteral.h b/dbms/src/Parsers/ASTLiteral.h
index 552f5da04a2..e9fb3d3b8ec 100644
--- a/dbms/src/Parsers/ASTLiteral.h
+++ b/dbms/src/Parsers/ASTLiteral.h
@@ -17,6 +17,8 @@ class ASTLiteral : public ASTWithAlias
 public:
     Field value;
 
+    String unique_column_name;
+
     /// For ConstantExpressionTemplate
     std::optional<TokenIterator> begin;
     std::optional<TokenIterator> end;
diff --git a/dbms/tests/queries/0_stateless/01101_literal_column_clash.reference b/dbms/tests/queries/0_stateless/01101_literal_column_clash.reference
new file mode 100644
index 00000000000..0dc94464bfc
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01101_literal_column_clash.reference
@@ -0,0 +1,3 @@
+1
+7	3
+xyzabc	2
diff --git a/dbms/tests/queries/0_stateless/01101_literal_column_clash.sql b/dbms/tests/queries/0_stateless/01101_literal_column_clash.sql
new file mode 100644
index 00000000000..11e3b622277
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01101_literal_column_clash.sql
@@ -0,0 +1,10 @@
+-- https://github.com/ClickHouse/ClickHouse/issues/9810
+select cast(1 as String)
+from (select 1 as iid) as t1
+join (select '1' as sid) as t2 on t2.sid = cast(t1.iid as String);
+
+-- even simpler cases
+select cast(7 as String), * from (select 3 "'String'");
+SELECT concat('xyz', 'abc'), * FROM (SELECT 2 AS "'xyz'");
+
+