desperate step: add debug output to see what happens in CI

maybe I'm onto something

another attempt

polish

Author: zvonand

src/Disks/ObjectStorages/IObjectStorage.cpp

@@ -148,7 +148,7 @@ void PathWithMetadata::loadMetadata(ObjectStoragePtr object_storage, bool ignore
     {
         const auto & path = isArchive() ? getPathToArchive() : getPath();
 
-        auto storage_to_use = object_storage_to_use.value_or(object_storage);
+        auto storage_to_use = object_storage_to_use ? object_storage_to_use : object_storage;
 
         if (ignore_non_existent_file)
             metadata = storage_to_use->tryGetObjectMetadata(path);

src/Disks/ObjectStorages/IObjectStorage.h

@@ -143,19 +143,19 @@ struct PathWithMetadata
     /// Retry request after short pause
     CommandInTaskResponse command;
     std::optional<String> absolute_path;
-    std::optional<ObjectStoragePtr> object_storage_to_use = std::nullopt;
+    ObjectStoragePtr object_storage_to_use = nullptr;
 
     PathWithMetadata() = default;
 
     explicit PathWithMetadata(
         const String & command_or_path,
         std::optional<ObjectMetadata> metadata_ = std::nullopt,
         std::optional<String> absolute_path_ = std::nullopt,
-        std::optional<ObjectStoragePtr> object_storage_to_use_ = std::nullopt)
+        ObjectStoragePtr object_storage_to_use_ = nullptr)
         : relative_path(std::move(command_or_path))
         , metadata(std::move(metadata_))
         , command(relative_path)
-        , absolute_path(absolute_path_)
+        , absolute_path((absolute_path_.has_value() && !absolute_path_.value().empty()) ? absolute_path_ : std::nullopt)
         , object_storage_to_use(object_storage_to_use_)
     {
         if (command.is_parsed())
@@ -183,7 +183,7 @@ struct PathWithMetadata
 
     void loadMetadata(ObjectStoragePtr object_storage, bool ignore_non_existent_file = true);
 
-    std::optional<ObjectStoragePtr> getObjectStorage() const { return object_storage_to_use; }
+    ObjectStoragePtr getObjectStorage() const { return object_storage_to_use; }
 };
 
 struct ObjectKeyWithMetadata

src/Interpreters/ClusterFunctionReadTask.cpp

@@ -51,7 +51,9 @@ ObjectInfoPtr ClusterFunctionReadTaskResponse::getObjectInfo() const
     auto object = std::make_shared<ObjectInfo>(path);
     object->data_lake_metadata = data_lake_metadata;
     object->file_meta_info = file_meta_info;
-    object->absolute_path = absolute_path;
+    if (absolute_path.has_value() && !absolute_path.value().empty())
+        object->absolute_path = absolute_path;
+    
     return object;
 }
 

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

@@ -112,7 +112,7 @@ Plan getPlan(
     IcebergHistory snapshots_info,
     const PersistentTableComponents & persistent_table_components,
     ObjectStoragePtr object_storage,
-    std::map<String, DB::ObjectStoragePtr> & secondary_storages,
+    SecondaryStorages & secondary_storages,
     StorageObjectStorageConfigurationPtr configuration,
     ContextPtr context,
     CompressionMethod compression_method)
@@ -239,14 +239,16 @@ void writeDataFiles(
     ContextPtr context,
     StorageObjectStorageConfigurationPtr configuration,
     const String & table_location,
-    std::map<String, ObjectStoragePtr> & secondary_storages)
+    SecondaryStorages & secondary_storages)
 {
     for (auto & [_, data_file] : initial_plan.path_to_data_file)
     {
         auto delete_file_transform = std::make_shared<IcebergBitmapPositionDeleteTransform>(
             sample_block, data_file->data_object_info, object_storage, format_settings, context, table_location, secondary_storages);
 
-        ObjectStoragePtr storage_to_use = data_file->data_object_info->getObjectStorage().value_or(object_storage);
+        ObjectStoragePtr storage_to_use = data_file->data_object_info->getObjectStorage();
+        if (!storage_to_use)
+            storage_to_use = object_storage;
         StorageObjectStorage::ObjectInfo object_info(data_file->data_object_info->getPath());
         auto read_buffer = createReadBuffer(object_info, storage_to_use, context, getLogger("IcebergCompaction"));
 
@@ -536,7 +538,7 @@ void compactIcebergTable(
     IcebergHistory snapshots_info,
     const PersistentTableComponents & persistent_table_components,
     ObjectStoragePtr object_storage_,
-    std::map<String, DB::ObjectStoragePtr> & secondary_storages_,
+    SecondaryStorages & secondary_storages_,
     StorageObjectStorageConfigurationPtr configuration_,
     const std::optional<FormatSettings> & format_settings_,
     SharedHeader sample_block_,

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

@@ -5,6 +5,7 @@
 #include <Storages/ObjectStorage/DataLakes/Iceberg/PersistentTableComponents.h>
 #include <Storages/ObjectStorage/DataLakes/Iceberg/Snapshot.h>
 #include <Storages/ObjectStorage/StorageObjectStorage.h>
+#include <Storages/ObjectStorage/Utils.h>
 
 
 namespace DB::Iceberg
@@ -15,7 +16,7 @@ void compactIcebergTable(
     IcebergHistory snapshots_info,
     const PersistentTableComponents & persistent_table_components,
     DB::ObjectStoragePtr object_storage_,
-    std::map<String, DB::ObjectStoragePtr> & secondary_storages_,
+    SecondaryStorages & secondary_storages_,
     DB::StorageObjectStorageConfigurationPtr configuration_,
     const std::optional<DB::FormatSettings> & format_settings_,
     DB::SharedHeader sample_block_,

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

@@ -37,22 +37,15 @@ namespace Setting
 extern const SettingsBool use_roaring_bitmap_iceberg_positional_deletes;
 };
 
-namespace
-{
-String toupper(String & str)
-{
-    std::transform(str.begin(), str.end(), str.begin(), ::toupper);
-    return str;
-}
-}
 
 IcebergDataObjectInfo::IcebergDataObjectInfo(Iceberg::ManifestFileEntry data_manifest_file_entry_)
-    : PathWithMetadata(data_manifest_file_entry_.file_path, std::nullopt, data_manifest_file_entry_.file_path_key)
+    : PathWithMetadata(data_manifest_file_entry_.file_path, std::nullopt,
+                       data_manifest_file_entry_.file_path_key.empty() ? std::nullopt : std::make_optional(data_manifest_file_entry_.file_path_key))
     , data_object_file_path_key(data_manifest_file_entry_.file_path_key)
     , underlying_format_read_schema_id(data_manifest_file_entry_.schema_id)
     , sequence_number(data_manifest_file_entry_.added_sequence_number)
 {
-    if (!position_deletes_objects.empty() && toupper(data_manifest_file_entry_.file_format) != "PARQUET")
+    if (!position_deletes_objects.empty() && Poco::toUpperInPlace(data_manifest_file_entry_.file_format) != "PARQUET")
     {
         throw Exception(
             ErrorCodes::NOT_IMPLEMENTED,
@@ -65,17 +58,14 @@ IcebergDataObjectInfo::IcebergDataObjectInfo(
     Iceberg::ManifestFileEntry data_manifest_file_entry_,
     ObjectStoragePtr resolved_storage,
     const String & resolved_key)
-    : PathWithMetadata(resolved_key, std::nullopt, data_manifest_file_entry_.file_path, resolved_storage)
+    : PathWithMetadata(resolved_key, std::nullopt,
+                       data_manifest_file_entry_.file_path.empty() ? std::nullopt : std::make_optional(data_manifest_file_entry_.file_path), 
+                       resolved_storage)
     , data_object_file_path_key(data_manifest_file_entry_.file_path_key)
     , underlying_format_read_schema_id(data_manifest_file_entry_.schema_id)
     , sequence_number(data_manifest_file_entry_.added_sequence_number)
 {
-    auto toupper = [](String & str)
-    {
-        std::transform(str.begin(), str.end(), str.begin(), ::toupper);
-        return str;
-    };
-    if (!position_deletes_objects.empty() && toupper(data_manifest_file_entry_.file_format) != "PARQUET")
+    if (!position_deletes_objects.empty() && Poco::toUpperInPlace(data_manifest_file_entry_.file_format) != "PARQUET")
     {
         throw Exception(
             ErrorCodes::NOT_IMPLEMENTED,
@@ -90,7 +80,7 @@ std::shared_ptr<ISimpleTransform> IcebergDataObjectInfo::getPositionDeleteTransf
     const std::optional<FormatSettings> & format_settings,
     ContextPtr context_,
     const String & table_location,
-    std::map<String, ObjectStoragePtr> & secondary_storages)
+    SecondaryStorages & secondary_storages)
 {
     IcebergDataObjectInfoPtr self = shared_from_this();
     if (!context_->getSettingsRef()[Setting::use_roaring_bitmap_iceberg_positional_deletes].value)

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

@@ -8,6 +8,7 @@
 
 #include <Storages/ObjectStorage/DataLakes/Iceberg/ManifestFile.h>
 #include <Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteObject.h>
+#include <Storages/ObjectStorage/Utils.h>
 #include <base/defines.h>
 
 
@@ -34,7 +35,7 @@ struct IcebergDataObjectInfo : public PathWithMetadata, std::enable_shared_from_
         const std::optional<FormatSettings> & format_settings,
         ContextPtr context_,
         const String & table_location,
-        std::map<String, ObjectStoragePtr> & secondary_storages);
+        SecondaryStorages & secondary_storages);
 
     void addPositionDeleteObject(Iceberg::ManifestFileEntry position_delete_object)
     {

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

@@ -24,6 +24,7 @@
 #include <IO/ReadBufferFromString.h>
 #include <IO/ReadHelpers.h>
 #include <Interpreters/Context.h>
+#include <Disks/DiskType.h>
 
 #include <IO/CompressedReadBufferWrapper.h>
 #include <Interpreters/ExpressionActions.h>
@@ -134,7 +135,7 @@ std::optional<ManifestFileEntry> SingleThreadIcebergKeysIterator::next()
                 data_snapshot->manifest_list_entries[manifest_file_index].manifest_file_absolute_path,
                 data_snapshot->manifest_list_entries[manifest_file_index].added_sequence_number,
                 data_snapshot->manifest_list_entries[manifest_file_index].added_snapshot_id,
-                secondary_storages);
+                *secondary_storages);
             internal_data_index = 0;
         }
         auto files = files_generator(current_manifest_file_content);
@@ -202,7 +203,7 @@ SingleThreadIcebergKeysIterator::SingleThreadIcebergKeysIterator(
     Iceberg::IcebergTableStateSnapshotPtr table_snapshot_,
     Iceberg::IcebergDataSnapshotPtr data_snapshot_,
     PersistentTableComponents persistent_components_,
-    std::map<String, ObjectStoragePtr> & secondary_storages_)
+    std::shared_ptr<SecondaryStorages> secondary_storages_)
     : object_storage(object_storage_)
     , filter_dag(filter_dag_ ? std::make_shared<ActionsDAG>(filter_dag_->clone()) : nullptr)
     , local_context(local_context_)
@@ -238,7 +239,7 @@ IcebergIterator::IcebergIterator(
     Iceberg::IcebergTableStateSnapshotPtr table_snapshot_,
     Iceberg::IcebergDataSnapshotPtr data_snapshot_,
     PersistentTableComponents persistent_components_,
-    std::map<String, ObjectStoragePtr> & secondary_storages_)
+    std::shared_ptr<SecondaryStorages> secondary_storages_)
     : filter_dag(filter_dag_ ? std::make_unique<ActionsDAG>(filter_dag_->clone()) : nullptr)
     , object_storage(std::move(object_storage_))
     , local_context(local_context_)
@@ -337,7 +338,7 @@ ObjectInfoPtr IcebergIterator::next(size_t)
     {
         // Resolve the data file path to get the correct storage and key
         auto [storage_to_use, resolved_key] = resolveObjectStorageForPath(
-            persistent_components.table_location, manifest_file_entry.file_path, object_storage, secondary_storages, local_context);
+            persistent_components.table_location, manifest_file_entry.file_path, object_storage, *secondary_storages, local_context);
 
         IcebergDataObjectInfoPtr object_info = std::make_shared<IcebergDataObjectInfo>(manifest_file_entry, storage_to_use, resolved_key);
 

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

@@ -25,6 +25,7 @@
 #include <Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadataFilesCache.h>
 #include <Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.h>
 #include <Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h>
+#include <Storages/ObjectStorage/Utils.h>
 
 namespace DB
 {
@@ -46,7 +47,7 @@ class SingleThreadIcebergKeysIterator
         IcebergTableStateSnapshotPtr table_snapshot_,
         IcebergDataSnapshotPtr data_snapshot_,
         PersistentTableComponents persistent_components,
-        std::map<String, ObjectStoragePtr> & secondary_storages_);
+        std::shared_ptr<SecondaryStorages> secondary_storages_);
 
     std::optional<DB::Iceberg::ManifestFileEntry> next();
 
@@ -63,7 +64,7 @@ class SingleThreadIcebergKeysIterator
     PersistentTableComponents persistent_components;
     FilesGenerator files_generator;
     LoggerPtr log;
-    std::map<String, ObjectStoragePtr> & secondary_storages;
+    std::shared_ptr<SecondaryStorages> secondary_storages;
 
     // By Iceberg design it is difficult to avoid storing position deletes in memory.
     size_t manifest_file_index = 0;
@@ -92,7 +93,7 @@ class IcebergIterator : public IObjectIterator
         Iceberg::IcebergTableStateSnapshotPtr table_snapshot_,
         Iceberg::IcebergDataSnapshotPtr data_snapshot_,
         Iceberg::PersistentTableComponents persistent_components_,
-        std::map<String, ObjectStoragePtr> & secondary_storages_);
+        std::shared_ptr<SecondaryStorages> secondary_storages_);
 
     ObjectInfoPtr next(size_t) override;
 
@@ -116,7 +117,7 @@ class IcebergIterator : public IObjectIterator
     std::mutex exception_mutex;
     Iceberg::PersistentTableComponents persistent_components;
     Int32 table_schema_id;
-    std::map<String, ObjectStoragePtr> & secondary_storages;
+    std::shared_ptr<SecondaryStorages> secondary_storages;  // Sometimes data or manifests can be located on another storage
 };
 }
 

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

@@ -133,6 +133,7 @@ IcebergMetadata::IcebergMetadata(
     IcebergMetadataFilesCachePtr cache_ptr,
     CompressionMethod metadata_compression_method_)
     : object_storage(std::move(object_storage_))
+    , secondary_storages(std::make_shared<SecondaryStorages>())
     , configuration(std::move(configuration_))
     , persistent_components(PersistentTableComponents{
           .schema_processor = std::make_shared<IcebergSchemaProcessor>(context_),
@@ -502,7 +503,7 @@ void IcebergMetadata::updateSnapshot(ContextPtr local_context, Poco::JSON::Objec
             auto [partition_key, sorting_key] = extractIcebergKeys(metadata_object);
 
             String manifest_list_path = snapshot->getValue<String>(f_manifest_list);
-            auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath(persistent_components.table_location, manifest_list_path, object_storage, secondary_storages, local_context);
+            auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath(persistent_components.table_location, manifest_list_path, object_storage, *secondary_storages, local_context);
 
             relevant_snapshot = std::make_shared<IcebergDataSnapshot>(
                 getManifestList(
@@ -549,7 +550,7 @@ bool IcebergMetadata::optimize(const StorageMetadataPtr & metadata_snapshot, Con
             snapshots_info,
             persistent_components,
             object_storage,
-            secondary_storages,
+            *secondary_storages,
             configuration_ptr,
             format_settings,
             sample_block,
@@ -930,7 +931,7 @@ std::optional<size_t> IcebergMetadata::totalRows(ContextPtr local_context) const
             manifest_list_entry.manifest_file_absolute_path,
             manifest_list_entry.added_sequence_number,
             manifest_list_entry.added_snapshot_id,
-            secondary_storages);
+            *secondary_storages);
         auto data_count = manifest_file_ptr->getRowsCountInAllFilesExcludingDeleted(FileContentType::DATA);
         auto position_deletes_count = manifest_file_ptr->getRowsCountInAllFilesExcludingDeleted(FileContentType::POSITION_DELETE);
         if (!data_count.has_value() || !position_deletes_count.has_value())
@@ -971,7 +972,7 @@ std::optional<size_t> IcebergMetadata::totalBytes(ContextPtr local_context) cons
             manifest_list_entry.manifest_file_absolute_path,
             manifest_list_entry.added_sequence_number,
             manifest_list_entry.added_snapshot_id,
-            secondary_storages);
+            *secondary_storages);
         auto count = manifest_file_ptr->getBytesCountInAllDataFilesExcludingDeleted();
         if (!count.has_value())
             return {};
@@ -1052,7 +1053,7 @@ void IcebergMetadata::addDeleteTransformers(
         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, persistent_components.table_location, secondary_storages); });
+            { return iceberg_object_info->getPositionDeleteTransformer(object_storage, header, format_settings, local_context, persistent_components.table_location, *secondary_storages); });
     }
     const auto & delete_files = iceberg_object_info->equality_deletes_objects;
     if (!delete_files.empty())
@@ -1065,7 +1066,7 @@ void IcebergMetadata::addDeleteTransformers(
             Block delete_file_header;
 
             auto [delete_storage_to_use, resolved_delete_key] = resolveObjectStorageForPath(
-                persistent_components.table_location, delete_file.file_path, object_storage, secondary_storages, local_context);
+                persistent_components.table_location, delete_file.file_path, object_storage, *secondary_storages, local_context);
 
             PathWithMetadata delete_file_object(resolved_delete_key, std::nullopt, delete_file.file_path, delete_storage_to_use);
             {