Antalya 26.8, frontport: Fix GROUP BY server constants over s3Cluster and object_storage_cluster

src/Interpreters/ExpressionAnalyzer.cpp

@@ -1,3 +1,5 @@
+#include <stack>
+
 #include <Interpreters/ExpressionAnalyzer.h>
 
 #include <AggregateFunctions/AggregateFunctionFactory.h>
@@ -140,6 +142,37 @@ bool allowEarlyConstantFolding(const ActionsDAG & actions, const Settings & sett
     return true;
 }
 
+/// True when `node` folds a server constant such as `hostName` or `tcpPort`.
+/// `__getScalar` is excluded: the initiator evaluates it and sends one value to every shard.
+bool containsServerConstant(const ActionsDAG::Node * node)
+{
+    std::stack<const ActionsDAG::Node *> stack;
+    if (node)
+        stack.push(node);
+
+    while (!stack.empty())
+    {
+        const auto * current = stack.top();
+        stack.pop();
+
+        while (current && current->type == ActionsDAG::ActionType::ALIAS)
+            current = current->children.empty() ? nullptr : current->children.front();
+
+        if (!current)
+            continue;
+
+        /// `__getScalar` is marked as a server constant, but the initiator evaluates it
+        /// and sends the same value to every shard, so it does not have to stay a key.
+        if (current->function_base && current->function_base->isServerConstant() && current->function_base->getName() != "__getScalar")
+            return true;
+
+        for (const auto * child : current->children)
+            stack.push(child);
+    }
+
+    return false;
+}
+
 LoggerPtr getLogger() { return ::getLogger("ExpressionAnalyzer"); }
 
 }
@@ -314,6 +347,9 @@ void ExpressionAnalyzer::analyzeAggregation(ActionsDAG & temp_actions)
             else
                 group_by_kind = GroupByKind::ORDINARY;
             bool use_nulls = group_by_kind != GroupByKind::ORDINARY && getContext()->getSettingsRef()[Setting::group_by_use_nulls];
+            /// Cluster table function workers leave `distributed_depth` at 0, same as a local query.
+            /// `SECONDARY_QUERY` is what distinguishes them: a server constant differs per shard only there.
+            const bool is_secondary_query = getContext()->getClientInfo().query_kind == ClientInfo::QueryKind::SECONDARY_QUERY;
 
             /// For GROUPING SETS with multiple groups we always add virtual __grouping_set column
             /// With set number, which is used as an additional key at the stage of merging aggregating data.
@@ -350,7 +386,9 @@ void ExpressionAnalyzer::analyzeAggregation(ActionsDAG & temp_actions)
                         if (getContext()->getClientInfo().distributed_depth == 0 || settings.distributed_group_by_no_merge > 0)
                         {
                             /// Constant expressions have non-null column pointer at this stage.
-                            if (node->column)
+                            /// A server constant stays only on a secondary query, where each shard has its own value.
+                            /// A local query has one value, so the key is removed with other constants.
+                            if (node->column && !(is_secondary_query && containsServerConstant(node)))
                             {
                                 select_query->group_by_with_constant_keys = true;
 
@@ -404,7 +442,9 @@ void ExpressionAnalyzer::analyzeAggregation(ActionsDAG & temp_actions)
                     if (getContext()->getClientInfo().distributed_depth == 0 || settings.distributed_group_by_no_merge > 0)
                     {
                         /// Constant expressions have non-null column pointer at this stage.
-                        if (node->column)
+                        /// A server constant stays only on a secondary query, where each shard has its own value.
+                        /// A local query has one value, so the key is removed with other constants.
+                        if (node->column && !(is_secondary_query && containsServerConstant(node)))
                         {
                             select_query->group_by_with_constant_keys = true;
 

src/Storages/ObjectStorage/DataLakes/Iceberg/AlterDropPartitionExecutor.cpp

@@ -215,6 +215,7 @@ AlterDropPartitionExecutor::AlterDropPartitionExecutor(
     const IcebergMetadata & metadata_,
     ContextPtr context_,
     ObjectStoragePtr object_storage_,
+    ExternalStorageCache & external_storages_,
     const PersistentTableComponents & components_,
     const DataLakeStorageSettings & data_lake_settings_,
     String write_format_,
@@ -223,6 +224,7 @@ AlterDropPartitionExecutor::AlterDropPartitionExecutor(
     , metadata(metadata_)
     , context(context_)
     , object_storage(std::move(object_storage_))
+    , external_storages(external_storages_)
     , components(components_)
     , data_lake_settings(data_lake_settings_)
     , write_format(std::move(write_format_))
@@ -365,7 +367,8 @@ AlterDropPartitionExecutor::discoverTargetFilePaths(const SnapshotState & state,
 
     for (const auto & manifest_key : state.snapshot->manifest_list_entries)
     {
-        auto handle = getManifestFileEntriesHandle(object_storage, components, context, log, manifest_key, state.schema_id);
+        auto handle = getManifestFileEntriesHandle(
+            object_storage, components, context, log, manifest_key, state.schema_id, external_storages);
 
         collect(handle.getFilesWithoutDeleted(FileContentType::DATA));
         collect(handle.getFilesWithoutDeleted(FileContentType::POSITION_DELETE));
@@ -460,7 +463,8 @@ AlterDropPartitionExecutor::buildDropPlan(const SnapshotState & state, const Tar
 
     for (const auto & manifest_key : state.snapshot->manifest_list_entries)
     {
-        auto handle = getManifestFileEntriesHandle(object_storage, components, context, log, manifest_key, state.schema_id);
+        auto handle = getManifestFileEntriesHandle(
+            object_storage, components, context, log, manifest_key, state.schema_id, external_storages);
 
         size_t entries_to_keep = 0;
         size_t entries_to_remove = 0;
@@ -575,6 +579,7 @@ AlterDropPartitionExecutor::ManifestListWriteResult AlterDropPartitionExecutor::
         components.path_resolver,
         state.metadata_object,
         object_storage,
+        external_storages,
         context,
         /*manifest_entry_names=*/{},
         new_snapshot,

src/Storages/ObjectStorage/DataLakes/Iceberg/AlterDropPartitionExecutor.h

@@ -30,6 +30,7 @@ namespace DB
 
 class FileNamesGenerator;
 class IcebergMetadata;
+struct ExternalStorageCache;
 
 namespace Iceberg
 {
@@ -42,6 +43,7 @@ class AlterDropPartitionExecutor
         const IcebergMetadata & metadata_,
         ContextPtr context_,
         ObjectStoragePtr object_storage_,
+        ExternalStorageCache & external_storages_,
         const PersistentTableComponents & components_,
         const DataLakeStorageSettings & data_lake_settings_,
         String write_format_,
@@ -106,6 +108,7 @@ class AlterDropPartitionExecutor
     const IcebergMetadata & metadata;
     ContextPtr context;
     ObjectStoragePtr object_storage;
+    ExternalStorageCache & external_storages;
     const PersistentTableComponents & components;
     const DataLakeStorageSettings & data_lake_settings;
     String write_format;

src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp

@@ -973,6 +973,7 @@ void IcebergMetadata::alterPartitionDropImpl(const PartitionCommand & command, C
         *this,
         context,
         object_storage,
+        *external_storages,
         persistent_components,
         data_lake_settings,
         write_format,

tests/queries/0_stateless/05296_s3_cluster_hostname_group_by.reference

@@ -0,0 +1,3 @@
+1	10
+1	10
+1	10

tests/queries/0_stateless/05296_s3_cluster_hostname_group_by.sql

@@ -0,0 +1,32 @@
+-- Tags: no-fasttest
+-- Tag no-fasttest: Depends on Minio
+-- https://github.com/Altinity/ClickHouse/issues/2139
+
+INSERT INTO FUNCTION s3('http://localhost:11111/test/05296_s3_cluster_hostname_group_by.tsv', 'TSV', 'x UInt32')
+SELECT number FROM numbers(10)
+SETTINGS s3_truncate_on_insert = 1;
+
+SELECT host != '', c FROM
+(
+    SELECT hostName() AS host, count() AS c
+    FROM s3Cluster('test_cluster_two_shards_localhost', 'http://localhost:11111/test/05296_s3_cluster_hostname_group_by.tsv', 'TSV', 'x UInt32')
+    GROUP BY host
+)
+SETTINGS enable_analyzer = 0;
+
+SELECT host != '', c FROM
+(
+    SELECT hostName() AS host, count() AS c
+    FROM s3('http://localhost:11111/test/05296_s3_cluster_hostname_group_by.tsv', 'TSV', 'x UInt32')
+    GROUP BY host
+    SETTINGS object_storage_cluster = 'test_cluster_two_shards_localhost'
+)
+SETTINGS enable_analyzer = 0;
+
+SELECT host != '', c FROM
+(
+    SELECT hostName() AS host, count() AS c
+    FROM s3Cluster('test_cluster_two_shards_localhost', 'http://localhost:11111/test/05296_s3_cluster_hostname_group_by.tsv', 'TSV', 'x UInt32')
+    GROUP BY host
+)
+SETTINGS enable_analyzer = 1;

tests/queries/0_stateless/05297_scalar_subquery_group_by_old_analyzer.reference

@@ -0,0 +1,2 @@
+3
+3

tests/queries/0_stateless/05297_scalar_subquery_group_by_old_analyzer.sql

@@ -0,0 +1,7 @@
+-- A scalar subquery is evaluated once and sent to every shard as `__getScalar`.
+-- It must not stay a GROUP BY key just because that function is marked as a server constant.
+-- https://github.com/Altinity/ClickHouse/pull/2490
+
+SELECT count() FROM numbers(3) GROUP BY (SELECT [1, 2]) SETTINGS enable_analyzer = 0;
+
+SELECT count() FROM numbers(3) GROUP BY (SELECT [CAST(1, 'Dynamic')]) SETTINGS enable_analyzer = 0;

tests/queries/0_stateless/05298_local_server_constant_group_by_old_analyzer.reference

@@ -0,0 +1,3 @@
+3
+3
+3

tests/queries/0_stateless/05298_local_server_constant_group_by_old_analyzer.sql

@@ -0,0 +1,9 @@
+-- A local query has one server, so a folded server constant is an ordinary GROUP BY constant.
+-- Keeping it makes `Dynamic` fail group-key validation.
+-- https://github.com/Altinity/ClickHouse/pull/2490
+
+SELECT count() FROM numbers(3) GROUP BY hostName() SETTINGS enable_analyzer = 0;
+
+SELECT count() FROM numbers(3) GROUP BY CAST(hostName(), 'Dynamic') SETTINGS enable_analyzer = 0;
+
+SELECT count() FROM numbers(3) GROUP BY [CAST(hostName(), 'Dynamic')] SETTINGS enable_analyzer = 0;