[DO NOT MERGE] Antalya 26.6: __aliasMarker refactoring to cover complex cases with ALIASes

src/Analyzer/Utils.cpp

@@ -1017,6 +1017,63 @@ void resolveAggregateFunctionNodeByName(FunctionNode & function_node, const Stri
     function_node.resolveAsAggregateFunction(std::move(aggregate_function));
 }
 
+namespace
+{
+
+class FinalizeAliasMarkersVisitor : public InDepthQueryTreeVisitor<FinalizeAliasMarkersVisitor>
+{
+public:
+    explicit FinalizeAliasMarkersVisitor(ContextPtr context_) : context(std::move(context_)) {}
+
+    /// Visit children first, so a nested marker chain is materialized from the inside out.
+    bool shouldTraverseTopToBottom() const { return false; }
+
+    void visitImpl(QueryTreeNodePtr & node)
+    {
+        auto * function_node = node->as<FunctionNode>();
+        if (!function_node || function_node->getFunctionName() != "__aliasMarker")
+            return;
+
+        auto & arguments = function_node->getArguments().getNodes();
+        if (arguments.size() != 2 || !arguments[0] || !arguments[1])
+            return;
+
+        /// Already materialized on an earlier hop.
+        if (const auto * id_node = arguments[1]->as<ConstantNode>(); id_node && isString(id_node->getResultType()))
+            return;
+
+        const auto * column_node = arguments[1]->as<ColumnNode>();
+        if (!column_node)
+            return;
+
+        const auto & column_source = column_node->getColumnSourceOrNull();
+        if (!column_source)
+            return;
+
+        /// A lambda parameter -- `arrayMap(x -> __aliasMarker(x, x), ...)` written by hand -- has the `LambdaNode` as
+        /// its source. There is no table alias to build an id from, and the marker is a per-row identity rather than a
+        /// transport marker, so leave it as it is. A marker this pass injected inside a lambda body does not land here:
+        /// its id names an `ALIAS` column of a table, and the table expression is its source.
+        if (column_source->getNodeType() == QueryTreeNodeType::LAMBDA || !column_source->hasAlias())
+            return;
+
+        auto alias_id = column_source->getAlias() + "." + column_node->getColumnName();
+        arguments[1] = std::make_shared<ConstantNode>(std::move(alias_id), std::make_shared<DataTypeString>());
+        resolveOrdinaryFunctionNodeByName(*function_node, "__aliasMarker", context);
+    }
+
+private:
+    ContextPtr context;
+};
+
+}
+
+void finalizeAliasMarkersForDistributedSerialization(QueryTreeNodePtr & node, const ContextPtr & context)
+{
+    FinalizeAliasMarkersVisitor visitor(context);
+    visitor.visit(node);
+}
+
 std::pair<QueryTreeNodePtr, bool> getExpressionSource(const QueryTreeNodePtr & node)
 {
     if (const auto * column = node->as<ColumnNode>())

src/Analyzer/Utils.h

@@ -157,6 +157,19 @@ void resolveOrdinaryFunctionNodeByName(FunctionNode & function_node, const Strin
 /// Arguments and parameters are taken from the node.
 void resolveAggregateFunctionNodeByName(FunctionNode & function_node, const String & function_name);
 
+/** Materialize the id argument of every `__aliasMarker` in the tree, turning the `ColumnNode` it carries into the
+  * `String` constant the shard will read.
+  *
+  * Call this immediately before the tree is rendered as SQL for a shard, and no earlier. The id is the column's
+  * analyzer identifier, and `createUniqueAliasesIfNecessary` -- which runs inside `buildQueryTreeForShard` -- is what
+  * settles the `__tableN` part of it. An id frozen before that point names a table alias that no longer exists by the
+  * time the shard header is built.
+  *
+  * A marker whose id is already a `String` constant was materialized on an earlier hop and is left alone. A marker
+  * carrying anything else is a user-written one, not ours, and is also left alone.
+  */
+void finalizeAliasMarkersForDistributedSerialization(QueryTreeNodePtr & node, const ContextPtr & context);
+
 /// Returns single source of expression node.
 /// First element of pair is source node, can be nullptr if there are no sources or multiple sources.
 /// Second element of pair is true if there is at most one source, false if there are multiple sources.

src/Functions/identity.cpp

@@ -38,12 +38,14 @@ REGISTER_FUNCTION(AliasMarker)
 {
     factory.registerFunction<FunctionAliasMarker>(FunctionDocumentation{
         .description = R"(
-Internal function that marks ALIAS column expressions for the analyzer. Not intended for direct use.
+Internal function. Returns its first argument unchanged. The second argument records which ALIAS column the
+expression was inlined from, so that a shard names the result the way the initiator expects. Not intended for
+direct use, but harmless when used directly.
 )",
-        .syntax = {"__aliasMarker(expr, alias_name)"},
+        .syntax = {"__aliasMarker(expr, alias_id)"},
         .arguments = {
             {"expr", "Expression to mark.", {"Any"}},
-            {"alias_name", "Alias name attached to the expression.", {"String"}},
+            {"alias_id", "Identity of the ALIAS column the expression came from.", {"Any"}},
         },
         .returned_value = {"Returns expr unchanged.", {"Any"}},
         .introduced_in = {25, 8},

src/Functions/identity.h

@@ -108,6 +108,35 @@ struct AliasMarkerName
     static constexpr auto name = "__aliasMarker";
 };
 
+/** `__aliasMarker(expr, id)` is an internal pass-through identity. It returns `expr` untouched; `id` exists only to
+  * give the expression a stable name in the planner's `ActionsDAG`.
+  *
+  * It is injected when an `ALIAS` column is inlined into its defining expression for transport to a shard. The
+  * initiator still sees the un-inlined column, so without the marker the shard would name the output after the
+  * expression (`multiply(__table1.value, 2)`) while the initiator expects the column (`__table1.computed`), and the
+  * two headers could not be matched by name.
+  *
+  * The marker carries the low-level column identity, not the user's SQL alias. A SQL alias cannot do this job: it
+  * participates in user-visible query semantics, it can collide with names the user chose, and in the
+  * mergeable-state path the projection step that would normally apply it is skipped.
+  *
+  * This is also why the marker is not `__actionName`. `__actionName` survives into the `ActionsDAG` as a function node
+  * with a forced result name; `__aliasMarker` is consumed into an alias on top of its child, which is what keeps the
+  * expression behaving like a distinct logical column.
+  *
+  * The second argument travels in two forms. Between injection and serialization it is a `ColumnNode`, so the ordinary
+  * analyzer passes keep transforming it along with everything else -- in particular `createUniqueAliasesIfNecessary`,
+  * which assigns the final `__tableN` aliases. `finalizeAliasMarkersForDistributedSerialization` then materializes it
+  * into a `String` constant just before the query is rendered as SQL. Freezing the id any earlier captures a table
+  * alias that later changes, and the shard then emits a name the initiator never asked for.
+  *
+  * Both forms are accepted here, and so is anything else: a user can write `__aliasMarker(x, x)` and it behaves as an
+  * identity. Rejecting unexpected arguments in the function would turn user SQL into a server-side error for no gain,
+  * since the marker is harmless by construction.
+  *
+  * This is a bridge for as long as distributed transport still goes through SQL text. Once query plan serialization
+  * replaces that boundary, the marker should become unnecessary.
+  */
 class FunctionAliasMarker : public IFunction
 {
 public:
@@ -116,7 +145,9 @@ class FunctionAliasMarker : public IFunction
 
     String getName() const override { return name; }
     size_t getNumberOfArguments() const override { return 2; }
-    ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {1}; }
+    /// The id argument is a constant only after finalization. Before that it is a `ColumnNode`, and between the two
+    /// the function must still resolve.
+    ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {}; }
     bool isSuitableForConstantFolding() const override { return false; }
     bool isSuitableForShortCircuitArgumentsExecution(const DataTypesWithConstInfo & /*arguments*/) const override { return false; }
 
@@ -125,9 +156,6 @@ class FunctionAliasMarker : public IFunction
         if (arguments.size() != 2)
             throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker expects 2 arguments");
 
-        if (!WhichDataType(arguments[1]).isString())
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker is internal and should not be used directly");
-
         return arguments.front();
     }
 

src/Planner/PlannerActionsVisitor.cpp

@@ -95,6 +95,16 @@ String calculateActionNodeNameWithCastIfNeeded(const ConstantNode & constant_nod
     return buffer.str();
 }
 
+/// Return a `__aliasMarker`'s finalized id, or an empty string when it has none yet. Between injection and
+/// serialization the id is a `ColumnNode`, and a user-written marker can hold anything at all.
+String tryExtractAliasMarkerId(const QueryTreeNodePtr & id_argument)
+{
+    if (const auto * id_node = id_argument->as<ConstantNode>(); id_node && isString(id_node->getResultType()))
+        return id_node->getValue().safeGet<String>();
+
+    return {};
+}
+
 class ActionNodeNameHelper
 {
 public:
@@ -198,18 +208,21 @@ class ActionNodeNameHelper
                 const auto & function_node = node->as<FunctionNode &>();
                 if (function_node.getFunctionName() == "__aliasMarker")
                 {
-                    /// Perform sanity check, because user may call this function with unexpected arguments
                     const auto & function_argument_nodes = function_node.getArguments().getNodes();
-                    if (function_argument_nodes.size() == 2)
-                    {
-                        if (const auto * second_argument = function_argument_nodes.at(1)->as<ConstantNode>())
-                        {
-                            if (isString(second_argument->getResultType()))
-                                result = second_argument->getValue().safeGet<String>();
-                        }
-                    }
+                    if (function_argument_nodes.size() != 2)
+                        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker expects 2 arguments");
 
-                    /// Empty node name is not allowed and leads to logical errors
+                    result = tryExtractAliasMarkerId(function_argument_nodes.at(1));
+
+                    /// No finalized id: either the marker has not reached its serialization boundary yet, or a user
+                    /// wrote one by hand. Name the node after the expression it wraps, which is what the name would
+                    /// have been without a marker at all. The two cases are not distinguishable here -- a hand-written
+                    /// `__aliasMarker(x, x)` looks exactly like an injected one whose id is still a `ColumnNode` -- so
+                    /// this cannot be turned into an assertion without rejecting valid SQL (03933).
+                    if (result.empty())
+                        result = calculateActionNodeName(function_argument_nodes.at(0));
+
+                    /// An empty node name is not allowed and leads to logical errors.
                     if (result.empty())
                         throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker is internal and should not be used directly");
                     break;
@@ -1259,15 +1272,14 @@ PlannerActionsVisitorImpl::NodeNameAndNodeMinLevel PlannerActionsVisitorImpl::vi
         if (function_arguments.size() != 2)
             throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker expects 2 arguments");
 
-        const auto * alias_id_node = function_arguments.at(1)->as<ConstantNode>();
-        if (!alias_id_node || !isString(alias_id_node->getResultType()))
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker is internal and should not be used directly");
+        auto [child_name, levels] = visitImpl(function_arguments.at(0));
 
-        const auto & alias_id = alias_id_node->getValue().safeGet<String>();
+        /// Without a finalized id the marker adds no identity of its own, so it resolves to its payload. That happens
+        /// for a marker the user wrote by hand, and for one that has not reached a serialization boundary yet.
+        auto alias_id = tryExtractAliasMarkerId(function_arguments.at(1));
         if (alias_id.empty())
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function __aliasMarker is internal and should not be used directly");
+            alias_id = child_name;
 
-        auto [child_name, levels] = visitImpl(function_arguments.at(0));
         if (alias_id == child_name)
             return {child_name, levels};
 

src/Storages/StorageDistributed.cpp

@@ -10,7 +10,6 @@
 #include <DataTypes/DataTypeLowCardinality.h>
 #include <DataTypes/DataTypeUUID.h>
 #include <DataTypes/DataTypesNumber.h>
-#include <DataTypes/DataTypeString.h>
 #include <DataTypes/NestedUtils.h>
 
 #include <Disks/IVolume.h>
@@ -55,7 +54,6 @@
 #include <Parsers/parseQuery.h>
 
 #include <Analyzer/ColumnNode.h>
-#include <Analyzer/ConstantNode.h>
 #include <Analyzer/FunctionNode.h>
 #include <Analyzer/TableNode.h>
 #include <Analyzer/TableFunctionNode.h>
@@ -213,7 +211,6 @@ namespace Setting
     extern const SettingsBool skip_unavailable_shards;
     extern const SettingsBool enable_global_with_statement;
     extern const SettingsBool allow_experimental_hybrid_table;
-    extern const SettingsBool enable_alias_marker;
 }
 
 namespace DistributedSetting
@@ -849,73 +846,6 @@ StorageSnapshotPtr StorageDistributed::getStorageSnapshot(const StorageMetadataP
 namespace
 {
 
-class ReplaseAliasColumnsVisitor : public InDepthQueryTreeVisitor<ReplaseAliasColumnsVisitor>
-{
-    QueryTreeNodePtr getColumnNodeAliasExpression(const QueryTreeNodePtr & node) const
-    {
-        const auto * column_node = node->as<ColumnNode>();
-        if (!column_node || !column_node->hasExpression())
-            return nullptr;
-
-        const auto & column_source = column_node->getColumnSourceOrNull();
-        if (!column_source || column_source->getNodeType() == QueryTreeNodeType::JOIN
-                           || column_source->getNodeType() == QueryTreeNodeType::CROSS_JOIN
-                           || column_source->getNodeType() == QueryTreeNodeType::ARRAY_JOIN)
-            return nullptr;
-
-        auto column_expression = column_node->getExpression();
-        const auto & column_name = column_node->getColumnName();
-
-        if (!context->getSettingsRef()[Setting::enable_alias_marker])
-        {
-            column_expression->setAlias(column_name);
-            return column_expression;
-        }
-
-        String alias_id;
-        const auto & source_alias = column_source->getAlias();
-        if (!source_alias.empty())
-            alias_id = source_alias + "." + column_name;
-        else
-            alias_id = column_name;
-
-        if (auto * function_node = column_expression->as<FunctionNode>();
-            function_node && function_node->getFunctionName() == "__aliasMarker")
-        {
-            auto & arguments = function_node->getArguments().getNodes();
-            if (arguments.size() == 2)
-                arguments[1] = std::make_shared<ConstantNode>(alias_id, std::make_shared<DataTypeString>());
-
-            column_expression->setAlias(column_name);
-            return column_expression;
-        }
-
-        QueryTreeNodes arguments;
-        arguments.reserve(2);
-        arguments.emplace_back(std::move(column_expression));
-        arguments.emplace_back(std::make_shared<ConstantNode>(alias_id, std::make_shared<DataTypeString>()));
-
-        auto alias_marker_node = std::make_shared<FunctionNode>("__aliasMarker");
-        alias_marker_node->getArguments().getNodes() = std::move(arguments);
-        alias_marker_node->setAlias(column_name);
-        resolveOrdinaryFunctionNodeByName(*alias_marker_node, "__aliasMarker", context);
-
-        return alias_marker_node;
-    }
-
-public:
-    explicit ReplaseAliasColumnsVisitor(ContextPtr context_) : context(std::move(context_)) {}
-
-    void visitImpl(QueryTreeNodePtr & node)
-    {
-        if (auto column_expression = getColumnNodeAliasExpression(node))
-            node = column_expression;
-    }
-
-private:
-    ContextPtr context;
-};
-
 using ColumnNameToColumnNodeMap = std::unordered_map<std::string, ColumnNodePtr>;
 
 ColumnNameToColumnNodeMap buildColumnNodesForTableExpression(const QueryTreeNodePtr & table_expression_node, const ContextPtr & context)
@@ -1151,7 +1081,7 @@ QueryTreeNodePtr buildQueryTreeDistributed(SelectQueryInfo & query_info,
          * (including fully-resolved ALIAS expressions) and rewrite the whole query tree
          * so all references to the replaced table share the same column source and
          * the same alias semantics. This keeps SELECT and WHERE consistent before
-         * ReplaseAliasColumnsVisitor performs final alias expansion.
+         * inlineAliasColumns performs final alias expansion.
          */
         ReplaceColumnNodesForTableExpressionVisitor replace_query_columns_visitor(
             replacement_table_expression,
@@ -1160,8 +1090,7 @@ QueryTreeNodePtr buildQueryTreeDistributed(SelectQueryInfo & query_info,
         replace_query_columns_visitor.visit(query_tree_to_modify);
     }
 
-    ReplaseAliasColumnsVisitor replace_alias_columns_visitor(query_context);
-    replace_alias_columns_visitor.visit(query_tree_to_modify);
+    inlineAliasColumns(query_tree_to_modify, query_context);
 
     const auto & settings = query_context->getSettingsRef();
 
@@ -1174,8 +1103,9 @@ QueryTreeNodePtr buildQueryTreeDistributed(SelectQueryInfo & query_info,
         rewriteJoinToGlobalJoinIfNeeded(query_node.getJoinTree());
     }
 
+    /// `buildQueryTreeForShard` materializes the marker ids on the way out, after the `__tableN` renumbering they are
+    /// built from.
     return buildQueryTreeForShard(query_info.planner_context, query_tree_to_modify, /*allow_global_join_for_right_table*/ false);
-
 }
 
 std::optional<std::pair<String, String>> tryGetParamTypeAndName(const ASTPtr & node)