Fix for alter table delete

ianton-ru · ianton-ru · commit 2baaf668390d · 2025-10-08T20:58:54.000+02:00
diff --git a/src/Core/Range.cpp b/src/Core/Range.cpp
@@ -2,15 +2,13 @@
 #include <Core/Range.h>
 #include <IO/Operators.h>
 #include <IO/WriteBufferFromString.h>
-#include <IO/ReadBufferFromString.h>
 #include <Common/FieldVisitorToString.h>
 #include <Common/FieldAccurateComparison.h>
 
 
 namespace DB
 {
 
-
 FieldRef::FieldRef(ColumnsWithTypeAndName * columns_, size_t row_idx_, size_t column_idx_)
     : Field((*(*columns_)[column_idx_].column)[row_idx_]), columns(columns_), row_idx(row_idx_), column_idx(column_idx_)
 {
diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp
@@ -39,6 +39,10 @@ const VersionToSettingsChangesMap & getSettingsChangesHistory()
         /// controls new feature and it's 'true' by default, use 'false' as previous_value).
         /// It's used to implement `compatibility` setting (see https://github.com/ClickHouse/ClickHouse/issues/35972)
         /// Note: please check if the key already exists to prevent duplicate entries.
+        addSettingsChanges(settings_changes_history, "25.8.1.20384",
+        {
+            {"allow_experimental_iceberg_read_optimization", true, true, "New setting."},
+        });
         addSettingsChanges(settings_changes_history, "25.8",
         {
             {"output_format_json_quote_64bit_integers", true, false, "Disable quoting of the 64 bit integers in JSON by default"},
@@ -132,18 +136,6 @@ const VersionToSettingsChangesMap & getSettingsChangesHistory()
             {"distributed_plan_force_shuffle_aggregation", 0, 0, "New experimental setting"},
             {"allow_experimental_insert_into_iceberg", false, false, "New setting."},
             /// RELEASE CLOSED
-            {"allow_experimental_database_iceberg", false, true, "Turned ON by default for Antalya"},
-            {"allow_experimental_database_unity_catalog", false, true, "Turned ON by default for Antalya"},
-            {"allow_experimental_database_glue_catalog", false, true, "Turned ON by default for Antalya"},
-            {"output_format_parquet_enum_as_byte_array", true, true, "Enable writing Enum as byte array in Parquet by default"},
-            {"lock_object_storage_task_distribution_ms", 0, 0, "New setting."},
-            {"object_storage_cluster", "", "", "New setting"},
-            {"object_storage_max_nodes", 0, 0, "New setting"},
-            {"object_storage_cluster_join_mode", "allow", "allow", "New setting"},
-            {"object_storage_remote_initiator", false, false, "New setting."},
-            {"allow_experimental_export_merge_tree_part", false, false, "New setting."},
-            {"allow_experimental_iceberg_read_optimization", true, true, "New setting."},
-            {"export_merge_tree_part_overwrite_file_if_exists", false, false, "New setting."},
         });
         addSettingsChanges(settings_changes_history, "25.6",
         {
diff --git a/src/Disks/ObjectStorages/IObjectStorage.h b/src/Disks/ObjectStorages/IObjectStorage.h
@@ -9,7 +9,6 @@
 
 #include <Poco/Timestamp.h>
 #include <Poco/Util/AbstractConfiguration.h>
-#include <Poco/JSON/Object.h>
 #include <Core/Defines.h>
 #include <IO/ReadSettings.h>
 #include <IO/WriteSettings.h>
diff --git a/src/Processors/Sources/ConstChunkGenerator.h b/src/Processors/Sources/ConstChunkGenerator.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <Processors/ISource.h>
+#include <Processors/Formats/IInputFormat.h>
 
 
 namespace DB
@@ -13,7 +14,7 @@ class ConstChunkGenerator : public ISource
 public:
     ConstChunkGenerator(SharedHeader header, size_t total_num_rows, size_t max_block_size_)
         : ISource(std::move(header))
-        , remaining_rows(total_num_rows), max_block_size(max_block_size_)
+        , generated_rows(0), remaining_rows(total_num_rows), max_block_size(max_block_size_)
     {
     }
 
@@ -27,10 +28,14 @@ class ConstChunkGenerator : public ISource
 
         size_t num_rows = std::min(max_block_size, remaining_rows);
         remaining_rows -= num_rows;
-        return cloneConstWithDefault(Chunk{getPort().getHeader().getColumns(), 0}, num_rows);
+        auto chunk = cloneConstWithDefault(Chunk{getPort().getHeader().getColumns(), 0}, num_rows);
+        chunk.getChunkInfos().add(std::make_shared<ChunkInfoRowNumOffset>(generated_rows));
+        generated_rows += num_rows;
+        return chunk;
     }
 
 private:
+    size_t generated_rows;
     size_t remaining_rows;
     size_t max_block_size;
 };
diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp
@@ -108,7 +108,6 @@ extern const SettingsBool use_roaring_bitmap_iceberg_positional_deletes;
 extern const SettingsString iceberg_metadata_compression_method;
 extern const SettingsBool allow_experimental_insert_into_iceberg;
 extern const SettingsBool allow_experimental_iceberg_compaction;
-extern const SettingsBool allow_experimental_iceberg_read_optimization;
 }
 
 namespace
@@ -820,12 +819,14 @@ void IcebergMetadata::addDeleteTransformers(
 
     if (!iceberg_object_info->position_deletes_objects.empty())
     {
+        LOG_DEBUG(log, "Constructing filter transform for position delete, there are {} delete objects", iceberg_object_info->position_deletes_objects.size());
         builder.addSimpleTransform(
             [&](const SharedHeader & header)
             { return iceberg_object_info->getPositionDeleteTransformer(object_storage, header, format_settings, local_context); });
     }
     const auto & delete_files = iceberg_object_info->equality_deletes_objects;
-    LOG_DEBUG(log, "Constructing filter transform for equality delete, there are {} delete files", delete_files.size());
+    if (!delete_files.empty())
+        LOG_DEBUG(log, "Constructing filter transform for equality delete, there are {} delete files", delete_files.size());
     for (const ManifestFileEntry & delete_file : delete_files)
     {
         auto simple_transform_adder = [&](const SharedHeader & header)
diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp
@@ -273,8 +273,6 @@ Chunk StorageObjectStorageSource::generate()
 {
     lazyInitialize();
 
-    bool use_iceberg_read_optimization = read_context->getSettingsRef()[Setting::allow_experimental_iceberg_read_optimization];
-
     while (true)
     {
         if (isCancelled() || !reader)
@@ -330,14 +328,14 @@ Chunk StorageObjectStorageSource::generate()
                  .data_lake_snapshot_version = file_iterator->getSnapshotVersion()},
                 read_context);
 
-            if (use_iceberg_read_optimization)
+            /// Not empty when allow_experimental_iceberg_read_optimization=true
+            /// and some columns were removed from read list as columns with constant values.
+            /// Restore data for these columns.
+            for (const auto & constant_column : reader.constant_columns_with_values)
             {
-                for (const auto & constant_column : reader.constant_columns_with_values)
-                {
-                    chunk.addColumn(constant_column.first,
-                        constant_column.second.name_and_type.type->createColumnConst(
-                            chunk.getNumRows(), constant_column.second.value)->convertToFullColumnIfConst());
-                }
+                chunk.addColumn(constant_column.first,
+                    constant_column.second.name_and_type.type->createColumnConst(
+                        chunk.getNumRows(), constant_column.second.value));
             }
 
 #if USE_PARQUET && USE_AWS_S3
@@ -555,18 +553,19 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade
                     else
                         rows_count_from_metadata = column.second.rows_count;
                 }
+
                 if (column.second.hyperrectangle.has_value())
                 {
+                    auto column_name = column.first;
+
+                    auto i_column = requested_columns_list.find(column_name);
+                    if (i_column == requested_columns_list.end())
+                        continue;
+
                     if (column.second.hyperrectangle.value().isPoint() &&
                         (!column.second.nulls_count.has_value() || column.second.nulls_count.value() <= 0))
                     {
-                        auto column_name = column.first;
-
-                        auto i_column = requested_columns_list.find(column_name);
-                        if (i_column == requested_columns_list.end())
-                            continue;
-
-                        /// isPoint() method checks that left==right
+                        /// isPoint() method checks before that left==right
                         constant_columns_with_values[i_column->second.first] =
                             ConstColumnWithValue{
                                 i_column->second.second,
@@ -581,17 +580,9 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade
                             column.second.hyperrectangle.value().left.dump());
                     }
                     else if (column.second.rows_count.has_value() && column.second.nulls_count.has_value()
-                            && column.second.rows_count.value() == column.second.nulls_count.value())
+                            && column.second.rows_count.value() == column.second.nulls_count.value()
+                            && i_column->second.second.type->isNullable())
                     {
-                        auto column_name = column.first;
-
-                        auto i_column = requested_columns_list.find(column_name);
-                        if (i_column == requested_columns_list.end())
-                            continue;
-
-                        if (!i_column->second.second.type->isNullable())
-                            continue;
-
                         constant_columns_with_values[i_column->second.first] =
                             ConstColumnWithValue{
                                 i_column->second.second,
@@ -660,6 +651,8 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade
             columns.emplace_back(type->createColumn(), type, name);
         builder.init(Pipe(std::make_shared<ConstChunkGenerator>(
                               std::make_shared<const Block>(columns), *num_rows_from_cache, max_block_size)));
+        if (!constant_columns.empty())
+            configuration->addDeleteTransformers(object_info, builder, format_settings, context_);
     }
     else
     {
diff --git a/tests/integration/test_storage_iceberg/test.py b/tests/integration/test_storage_iceberg/test.py
@@ -3292,3 +3292,12 @@ def check_events(query_id, event, expected):
     check_events(const_partial_query_id, event, 4) # 1-2025, 6-2025 and 2-2025 must not be read
     check_events(const_partial2_query_id, event, 3) # 6-2025 must not be read, 1-2024, 1-2025, 2-2025 don't have new column 'name'
     check_events(count_query_id, event, 0) # All must not be read
+
+    def compare_selects(query):
+        result_expected = instance.query(f"{query} SETTINGS allow_experimental_iceberg_read_optimization=0")
+        result_optimized = instance.query(f"{query} SETTINGS allow_experimental_iceberg_read_optimization=1")
+        assert result_expected == result_optimized
+
+    compare_selects(f"SELECT _path,* FROM {creation_expression} ORDER BY ALL")
+    compare_selects(f"SELECT _path,* FROM {creation_expression} WHERE name_old='vasily' ORDER BY ALL")
+    compare_selects(f"SELECT _path,* FROM {creation_expression} WHERE ((tag + length(name_old)) % 2 = 1) ORDER BY ALL")

Original file line number	Diff line number	Diff line change
`@@ -2,15 +2,13 @@`
`2`	`2`	`#include <Core/Range.h>`
`3`	`3`	`#include <IO/Operators.h>`
`4`	`4`	`#include <IO/WriteBufferFromString.h>`
`5`		`-#include <IO/ReadBufferFromString.h>`
`6`	`5`	`#include <Common/FieldVisitorToString.h>`
`7`	`6`	`#include <Common/FieldAccurateComparison.h>`
`8`	`7`
`9`	`8`
`10`	`9`	`namespace DB`
`11`	`10`	`{`
`12`	`11`
`13`		`-`
`14`	`12`	`FieldRef::FieldRef(ColumnsWithTypeAndName * columns_, size_t row_idx_, size_t column_idx_)`
`15`	`13`	`: Field(((columns_)[column_idx_].column)[row_idx_]), columns(columns_), row_idx(row_idx_), column_idx(column_idx_)`
`16`	`14`	`{`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,6 @@ extern const SettingsBool use_roaring_bitmap_iceberg_positional_deletes;`
`108`	`108`	`extern const SettingsString iceberg_metadata_compression_method;`
`109`	`109`	`extern const SettingsBool allow_experimental_insert_into_iceberg;`
`110`	`110`	`extern const SettingsBool allow_experimental_iceberg_compaction;`
`111`		`-extern const SettingsBool allow_experimental_iceberg_read_optimization;`
`112`	`111`	`}`
`113`	`112`
`114`	`113`	`namespace`
`@@ -820,12 +819,14 @@ void IcebergMetadata::addDeleteTransformers(`
`820`	`819`
`821`	`820`	`if (!iceberg_object_info->position_deletes_objects.empty())`
`822`	`821`	`{`
	`822`	`+ LOG_DEBUG(log, "Constructing filter transform for position delete, there are {} delete objects", iceberg_object_info->position_deletes_objects.size());`
`823`	`823`	`builder.addSimpleTransform(`
`824`	`824`	`[&](const SharedHeader & header)`
`825`	`825`	`{ return iceberg_object_info->getPositionDeleteTransformer(object_storage, header, format_settings, local_context); });`
`826`	`826`	`}`
`827`	`827`	`const auto & delete_files = iceberg_object_info->equality_deletes_objects;`
`828`		`- LOG_DEBUG(log, "Constructing filter transform for equality delete, there are {} delete files", delete_files.size());`
	`828`	`+ if (!delete_files.empty())`
	`829`	`+ LOG_DEBUG(log, "Constructing filter transform for equality delete, there are {} delete files", delete_files.size());`
`829`	`830`	`for (const ManifestFileEntry & delete_file : delete_files)`
`830`	`831`	`{`
`831`	`832`	`auto simple_transform_adder = [&](const SharedHeader & header)`