Merge pull request ClickHouse#79505 from Algunenano/default_quick

Fix 2 cases of "Logical Error: Can't set alias of * of Asterisk on alias"

Author: nikitamikhaylov

src/Interpreters/OptimizeIfWithConstantConditionVisitor.cpp

@@ -98,13 +98,19 @@ void OptimizeIfWithConstantConditionVisitorData::visit(ASTFunction & function_no
     if (tryExtractConstValueFromCondition(condition_expr, condition))
     {
         ASTPtr replace_ast = condition ? then_expr : else_expr;
+        bool replacement_supports_alias = dynamic_cast<ASTWithAlias *>(replace_ast.get());
+        String if_alias = ast->tryGetAlias();
+        /// We cannot set the resulting alias if the replace ast does not support it (e.g. ASTAsterisk), so it's better to do nothing
+        if (!if_alias.empty() && !replacement_supports_alias)
+            return;
+
         ASTPtr child_copy = ast;
         String replace_alias = replace_ast->tryGetAlias();
-        String if_alias = ast->tryGetAlias();
 
         if (replace_alias.empty())
         {
-            replace_ast->setAlias(if_alias);
+            if (!if_alias.empty())
+                replace_ast->setAlias(if_alias);
             ast = replace_ast;
         }
         else
@@ -113,12 +119,14 @@ void OptimizeIfWithConstantConditionVisitorData::visit(ASTFunction & function_no
             /// But IAST has only method for deep copy of subtree.
             /// This can be a reason of performance degradation in case of deep queries.
             ASTPtr replace_ast_deep_copy = replace_ast->clone();
-            replace_ast_deep_copy->setAlias(if_alias);
+            if (!if_alias.empty())
+                replace_ast_deep_copy->setAlias(if_alias);
             ast = replace_ast_deep_copy;
         }
 
         if (!if_alias.empty())
         {
+            ast->setAlias(if_alias);
             auto alias_it = aliases.find(if_alias);
             if (alias_it != aliases.end() && alias_it->second.get() == child_copy.get())
                 alias_it->second = ast;

src/Interpreters/TreeRewriter.cpp

@@ -366,9 +366,12 @@ void renameDuplicatedColumns(const ASTSelectQuery * select_query)
     for (auto & expr : elements)
     {
         auto name = expr->getAliasOrColumnName();
-
         if (!assigned_column_names.insert(name).second)
         {
+            /// We can't rename with aliases if it doesn't support alias (e.g. asterisk)
+            if (!dynamic_cast<ASTWithAlias *>(expr.get()))
+                continue;
+
             size_t i = 1;
             while (all_column_names.end() != all_column_names.find(name + "_" + toString(i)))
                 ++i;

tests/queries/0_stateless/03443_alias_with_asterisk.reference

@@ -0,0 +1 @@
+CREATE TABLE t0 (c0 Int ALIAS if(NULL, 1, *)) ENGINE = Memory; -- { serverError UNKNOWN_IDENTIFIER }

tests/queries/0_stateless/03443_alias_with_asterisk.sql

@@ -0,0 +1,63 @@
+SELECT
+    `_--t1.a` AS `t1.a`,
+    `_--t1.b` AS `t1.b`,
+    `_--t2.a` AS `t2.a`,
+    `_--t2.b` AS `t2.b`,
+    `toNullable(10)` AS `t3.toNullable(10)`,
+    `_--t3.a` AS `t3.a`,
+    `_--t3.b` AS `t3.b`,
+    `isZeroOrNull(10)` AS `t3.isZeroOrNull(10)`,
+    `_--t3.10` AS `t3.10`,
+    `assumeNotNull(materialize(10))` AS `t3.assumeNotNull(materialize(10))`,
+    `isNotNull(10)` AS `t3.isNotNull(10)`,
+    x AS `t3.x`
+FROM
+(
+    SELECT
+        b AS `_--t1.b`,
+        a AS `_--t1.a`,
+        t2.b AS `_--t2.b`,
+        t2.a AS `_--t2.a`
+    FROM t1
+    ALL INNER JOIN
+    (
+        SELECT
+            a,
+            b
+        FROM t2
+    ) AS t2 ON if(`_--t2.b` > 0, `_--t2.a`, 0) = `_--t1.a`
+    WHERE if(`_--t2.b` > 0, `_--t2.a`, 0) = `_--t1.a`
+) AS `--.s`
+CROSS JOIN
+(
+    SELECT
+        x,
+        `assumeNotNull(materialize(10))`,
+        `isNotNull(10)`,
+        `isZeroOrNull(10)`,
+        `toNullable(10)`,
+        `10` AS `_--t3.10`,
+        b AS `_--t3.b`,
+        a AS `_--t3.a`
+    FROM
+    (
+        SELECT
+            toNullable(10),
+            a,
+            b,
+            isZeroOrNull(10),
+            10,
+            10 AS `10_1`,
+            10 AS `10_2`,
+            assumeNotNull(materialize(10)),
+            10 IS NOT NULL,
+            a AS x
+        FROM t3__fuzz_0
+        WHERE (toNullable(toUInt256(1)) + a) = b
+    ) AS t3
+) AS `--.t`
+WHERE if(`_--t2.b` > 0, `_--t2.a`, 0) = `_--t1.a`
+ORDER BY
+    x ASC NULLS FIRST,
+    `_--t2.a` DESC NULLS LAST,
+    `_--t1.a` DESC NULLS FIRST

tests/queries/0_stateless/03444_explain_asterisk.reference

@@ -0,0 +1,6 @@
+-- Fuzzing `Can't set alias of * of Asterisk on alias`
+DROP TABLE IF EXISTS t1, t2, t3__fuzz_0;
+CREATE TABLE t1 (`a` UInt64, `b` UInt64) ENGINE = Log;
+CREATE TABLE t2 (`a` UInt64, `b` UInt64) ENGINE = Log;
+CREATE TABLE t3__fuzz_0 (`a` LowCardinality(UInt64), `b` UInt64) ENGINE = Log SETTINGS allow_suspicious_low_cardinality_types=1;
+EXPLAIN SYNTAX  SELECT * FROM t1, t2, (SELECT toNullable(10), *, isZeroOrNull(10), 10, *, 10, *, *, 10, *, *, *, assumeNotNull(materialize(10)), 10 IS NOT NULL, a AS x FROM t3__fuzz_0 WHERE (toNullable(toUInt256(1)) + a) = b) AS t3 WHERE if(t2.b > 0, t2.a, 0) = t1.a ORDER BY t3.x ASC NULLS FIRST, t2.a DESC NULLS LAST, t1.a DESC NULLS FIRST;