diff --git a/src/Core/ProtocolDefines.h b/src/Core/ProtocolDefines.h index ec5b8bd015cb..84379f04d80e 100644 --- a/src/Core/ProtocolDefines.h +++ b/src/Core/ProtocolDefines.h @@ -39,7 +39,8 @@ static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_META static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO = 4; static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_EXCLUDED_ROWS = 5; static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_FILE_STATS = 6; -static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION = DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_FILE_STATS; +static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH = 9; +static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION = DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH; static constexpr auto DATA_LAKE_TABLE_STATE_SNAPSHOT_PROTOCOL_VERSION = 1; diff --git a/src/Core/Settings.cpp b/src/Core/Settings.cpp index 99ed9098617a..0478825c331b 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -641,6 +641,9 @@ Use multiple threads for azure multipart upload. )", 0) \ DECLARE(Bool, s3_throw_on_zero_files_match, false, R"( Throw an error, when ListObjects request cannot match any files +)", 0) \ + DECLARE(Bool, object_storage_propagate_credentials_to_other_storages, false, R"( +Reuse base-storage credentials for a secondary object storage. For `S3`, credentials are reused when the endpoint matches; when this setting is enabled, they are also reused across different endpoints, including less secure connections (for example, from `https` to plain `http`). For `Azure`, reads stay within the base account. )", 0) \ DECLARE(Bool, hdfs_throw_on_zero_files_match, false, R"( Throw an error if matched zero files according to glob expansion rules. diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp index 8c1dc4774a75..12b2ba8f95c2 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -268,7 +268,7 @@ const VersionToSettingsChangesMap & getSettingsChangesHistory() addSettingsChanges(settings_changes_history, "26.1.3.20001.altinityantalya", { // {"iceberg_partition_timezone", "", "", "New setting."}, - // {"s3_propagate_credentials_to_other_storages", false, false, "New setting"}, + {"object_storage_propagate_credentials_to_other_storages", false, false, "New setting"}, // {"export_merge_tree_part_filename_pattern", "", "{part_name}_{checksum}", "New setting"}, // {"use_parquet_metadata_cache", false, true, "Enables cache of parquet file metadata."}, // {"input_format_parquet_use_metadata_cache", true, false, "Obsolete. No-op"}, // https://github.com/Altinity/ClickHouse/pull/586 diff --git a/src/Databases/DataLake/DatabaseDataLake.cpp b/src/Databases/DataLake/DatabaseDataLake.cpp index 12fbb051ba4a..3b5735c2ed43 100644 --- a/src/Databases/DataLake/DatabaseDataLake.cpp +++ b/src/Databases/DataLake/DatabaseDataLake.cpp @@ -593,7 +593,9 @@ StoragePtr DatabaseDataLake::tryGetTableImpl(const String & name, ContextPtr con LOG_DEBUG(log, "Has no credentials"); } } - else if (!lightweight && table_metadata.requiresCredentials() && std::find(vended_credentials_catalogs.begin(), vended_credentials_catalogs.end(), catalog->getCatalogType()) == vended_credentials_catalogs.end()) + else if (!lightweight && table_metadata.requiresCredentials() + && std::find(vended_credentials_catalogs.begin(), vended_credentials_catalogs.end(), catalog->getCatalogType()) == vended_credentials_catalogs.end() + && table_metadata.getStorageType() != DatabaseDataLakeStorageType::Local) { throw Exception( ErrorCodes::BAD_ARGUMENTS, diff --git a/src/IO/S3/URI.cpp b/src/IO/S3/URI.cpp index dd5429dcf55f..7164b5d3c877 100644 --- a/src/IO/S3/URI.cpp +++ b/src/IO/S3/URI.cpp @@ -17,10 +17,12 @@ namespace DB struct URIConverter { - static void modifyURI(Poco::URI & uri, NameToNameMap mapper) + static void modifyURI(Poco::URI & uri, NameToNameMap mapper, bool enable_url_encoding = true) { Macros macros({{"bucket", uri.getHost()}}); - uri = macros.expand(mapper[uri.getScheme()]).empty() ? uri : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery()); + uri = macros.expand(mapper[uri.getScheme()]).empty() + ? uri + : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery(), enable_url_encoding); } }; @@ -32,7 +34,7 @@ namespace ErrorCodes namespace S3 { -URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool keep_presigned_query_parameters, S3UriStyle uri_style) +URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool keep_presigned_query_parameters, S3UriStyle uri_style, bool enable_url_encoding) { /// Case when AWS Private Link Interface is being used /// E.g. (bucket.vpce-07a1cd78f1bd55c5f-j3a3vg6w.s3.us-east-1.vpce.amazonaws.com/bucket-name/key) @@ -44,9 +46,9 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool keep_pre else uri_str = uri_; - uri = Poco::URI(uri_str); + uri = Poco::URI(uri_str, enable_url_encoding); /// Keep a copy of how Poco parsed the original string before any mapping - Poco::URI original_uri(uri_str); + Poco::URI original_uri(uri_str, enable_url_encoding); bool looks_like_presigned = false; for (const auto & [qk, qv] : original_uri.getQueryParameters()) { @@ -91,7 +93,7 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool keep_pre } if (!mapper.empty()) - URIConverter::modifyURI(uri, mapper); + URIConverter::modifyURI(uri, mapper, enable_url_encoding); } storage_name = "S3"; diff --git a/src/IO/S3/URI.h b/src/IO/S3/URI.h index 64b4def76744..241d75a647a3 100644 --- a/src/IO/S3/URI.h +++ b/src/IO/S3/URI.h @@ -41,7 +41,8 @@ struct URI const std::string & uri_, bool allow_archive_path_syntax = false, bool keep_presigned_query_parameters = true, - S3UriStyle uri_style = S3UriStyle::AUTO); + S3UriStyle uri_style = S3UriStyle::AUTO, + bool enable_url_encoding = true); void addRegionToURI(const std::string & region); static void validateBucket(const std::string & bucket, const Poco::URI & uri); diff --git a/src/Interpreters/ClusterFunctionReadTask.cpp b/src/Interpreters/ClusterFunctionReadTask.cpp index f3cfad1fba31..b6476630ca49 100644 --- a/src/Interpreters/ClusterFunctionReadTask.cpp +++ b/src/Interpreters/ClusterFunctionReadTask.cpp @@ -35,10 +35,8 @@ ClusterFunctionReadTaskResponse::ClusterFunctionReadTaskResponse(ObjectInfoPtr o data_lake_metadata = object->data_lake_metadata.value(); #if USE_AVRO - if (std::dynamic_pointer_cast(object)) - { - iceberg_info = dynamic_cast(*object).info; - } + if (auto iceberg_object = std::dynamic_pointer_cast(object)) + iceberg_info = iceberg_object->info; #endif const bool send_over_whole_archive = !context->getSettingsRef()[Setting::cluster_function_process_archive_on_multiple_nodes]; diff --git a/src/Interpreters/IcebergMetadataLog.cpp b/src/Interpreters/IcebergMetadataLog.cpp index 9536bc4ae96b..22ca3b2d7a88 100644 --- a/src/Interpreters/IcebergMetadataLog.cpp +++ b/src/Interpreters/IcebergMetadataLog.cpp @@ -105,12 +105,16 @@ void insertRowToLogTable( throw Exception(ErrorCodes::BAD_ARGUMENTS, "Iceberg metadata log table is not configured"); } + String normalized_table_path = table_path; + while (normalized_table_path.size() > 1 && normalized_table_path.back() == '/') + normalized_table_path.pop_back(); + iceberg_metadata_log->add( DB::IcebergMetadataLogElement{ .current_time = spec.tv_sec, .query_id = local_context->getCurrentQueryId(), .content_type = row_log_level, - .table_path = table_path, + .table_path = normalized_table_path, .file_path = file_path.serialize(), .metadata_content = row, .row_in_file = row_in_file, diff --git a/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp b/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp index 4517834d0dec..b05ccdb0f490 100644 --- a/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp @@ -158,7 +158,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE } - const auto file_path_key = IcebergPathFromMetadata::deserialize( + const auto file_path_from_metadata = IcebergPathFromMetadata::deserialize( getValueFromRowByName(row_index, c_data_file_file_path, TypeIndex::String).safeGet()); /// NOTE: This is weird, because in manifest file partition looks like this: /// { @@ -247,7 +247,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE case FileContentType::DATA: { return std::make_shared( FileContentType::DATA, - file_path_key, + file_path_from_metadata, row_index, status, sequence_number, @@ -294,7 +294,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE } return std::make_shared( FileContentType::POSITION_DELETE, - file_path_key, + file_path_from_metadata, row_index, status, sequence_number, @@ -325,7 +325,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE c_data_file_equality_ids); return std::make_shared( FileContentType::EQUALITY_DELETE, - file_path_key, + file_path_from_metadata, row_index, status, sequence_number, diff --git a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h index 389232fb1d16..48494736b0ce 100644 --- a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h @@ -56,6 +56,9 @@ class IDataLakeMetadata : boost::noncopyable virtual bool operator==(const IDataLakeMetadata & other) const = 0; + /// Returns the full table location URI (e.g. `s3a://bucket/prefix/table/`) + virtual std::string getTableLocation() const { return {}; } + /// Return iterator to `data files`. using FileProgressCallback = std::function; virtual ObjectIterator iterate( diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp index 66f07c521b27..477c22316d9e 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp @@ -80,6 +80,9 @@ struct Plan std::unordered_map> manifest_list_to_manifest_files; std::unordered_map>> snapshot_id_to_data_files; std::unordered_map> path_to_data_file; + /// Raw paths of every file referenced by the snapshots being compacted, used at cleanup + /// time to also remove files that live outside the base object_storage. + std::unordered_set referenced_file_paths; FileNamesGenerator generator; Poco::JSON::Object::Ptr initial_metadata_object; @@ -121,6 +124,7 @@ static Plan getPlan( const DataLakeStorageSettings & data_lake_settings, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, const String & write_format, ContextPtr context, CompressionMethod compression_method) @@ -163,14 +167,16 @@ static Plan getPlan( std::unordered_map> manifest_files; for (const auto & snapshot : snapshots_info) { - auto manifest_list = getManifestList(object_storage, persistent_table_components, context, snapshot.manifest_list_path, log); + plan.referenced_file_paths.insert(snapshot.manifest_list_path); + auto manifest_list = getManifestList(object_storage, persistent_table_components, context, snapshot.manifest_list_path, log, secondary_storages); for (const auto & manifest_file : manifest_list) { plan.manifest_list_to_manifest_files[snapshot.manifest_list_path].push_back(manifest_file.manifest_file_path); if (!plan.manifest_file_to_first_snapshot.contains(manifest_file.manifest_file_path)) plan.manifest_file_to_first_snapshot[manifest_file.manifest_file_path] = snapshot.snapshot_id; + plan.referenced_file_paths.insert(manifest_file.manifest_file_path); auto files_handle = getManifestFileEntriesHandle( - object_storage, persistent_table_components, context, log, manifest_file, static_cast(current_schema_id)); + object_storage, persistent_table_components, context, log, manifest_file, static_cast(current_schema_id), secondary_storages); if (!manifest_files.contains(manifest_file.manifest_file_path)) { @@ -179,28 +185,39 @@ static Plan getPlan( } manifest_files[manifest_file.manifest_file_path]->manifest_lists_path.push_back(snapshot.manifest_list_path); for (const auto & pos_delete_file : files_handle.getFilesWithoutDeleted(FileContentType::POSITION_DELETE)) + { all_positional_delete_files.push_back(pos_delete_file); + plan.referenced_file_paths.insert(pos_delete_file->parsed_entry->file_path_key); + } for (const auto & data_file : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { + plan.referenced_file_paths.insert(data_file->parsed_entry->file_path_key); auto partition_index = plan.partition_encoder.encodePartition(data_file->parsed_entry->partition_key_value); if (plan.partitions.size() <= partition_index) plan.partitions.push_back({}); + const auto & raw_metadata_path = data_file->parsed_entry->file_path_key.serialize(); + auto [resolved_storage, resolved_key] = resolveObjectStorageForPath( + persistent_table_components.table_location, + raw_metadata_path, object_storage, secondary_storages, context, + persistent_table_components.path_resolver); + IcebergDataObjectInfoPtr data_object_info = std::make_shared( - data_file, persistent_table_components.path_resolver.resolve(data_file->parsed_entry->file_path_key), 0); + data_file, raw_metadata_path, 0, resolved_storage, resolved_key); std::shared_ptr data_file_ptr; - if (!plan.path_to_data_file.contains(manifest_file.manifest_file_path)) + auto path_identifier = Iceberg::IcebergPathFromMetadata::makeStorageIdentity(resolved_storage, resolved_key); + if (!plan.path_to_data_file.contains(path_identifier)) { data_file_ptr = std::make_shared(DataFilePlan{ .data_object_info = data_object_info, .manifest_list = manifest_files[manifest_file.manifest_file_path], .patched_path = plan.generator.generateDataFileName()}); - plan.path_to_data_file[manifest_file.manifest_file_path] = data_file_ptr; + plan.path_to_data_file[path_identifier] = data_file_ptr; } else { - data_file_ptr = plan.path_to_data_file[manifest_file.manifest_file_path]; + data_file_ptr = plan.path_to_data_file[path_identifier]; } plan.partitions[partition_index].push_back(data_file_ptr); plan.snapshot_id_to_data_files[snapshot.snapshot_id].push_back(plan.partitions[partition_index].back()); @@ -219,7 +236,7 @@ static Plan getPlan( { if (data_file->data_object_info->info.sequence_number <= delete_file->sequence_number) data_file->data_object_info->addPositionDeleteObject( - delete_file, persistent_table_components.path_resolver.resolve(delete_file->parsed_entry->file_path_key)); + delete_file, delete_file->parsed_entry->file_path_key.serialize()); } } plan.history = std::move(snapshots_info); @@ -235,7 +252,8 @@ static void writeDataFiles( const std::optional & format_settings, ContextPtr context, const String & write_format, - CompressionMethod write_compression_method) + CompressionMethod write_compression_method, + std::shared_ptr secondary_storages) { ColumnMapperPtr column_mapper; { @@ -261,10 +279,15 @@ static void writeDataFiles( format_settings, // todo make compaction using same FormatParserSharedResources std::make_shared(context->getSettingsRef(), 1), - context); + context, + path_resolver, + secondary_storages); - RelativePathWithMetadata relative_path(data_file->data_object_info->getPath()); - auto read_buffer = createReadBuffer(relative_path, object_storage, context, getLogger("IcebergCompaction")); + ObjectStoragePtr storage_to_use = data_file->data_object_info->getResolvedStorage(); + if (!storage_to_use) + storage_to_use = object_storage; + RelativePathWithMetadata object_info(data_file->data_object_info->getPath()); + auto read_buffer = createReadBuffer(object_info, storage_to_use, context, getLogger("IcebergCompaction")); const Settings & settings = context->getSettingsRef(); auto parser_shared_resources = std::make_shared( @@ -373,7 +396,7 @@ void checkIfIcebergHistorySupported(const IcebergHistory & history) } static void writeMetadataFiles( - Plan & plan, const IcebergPathResolver & path_resolver, ObjectStoragePtr object_storage, ContextPtr context, SharedHeader sample_block_, String write_format, String table_path) + Plan & plan, const IcebergPathResolver & path_resolver, ObjectStoragePtr object_storage, SecondaryStorages & secondary_storages, ContextPtr context, SharedHeader sample_block_, String write_format, String table_path) { auto log = getLogger("IcebergCompaction"); @@ -478,6 +501,7 @@ static void writeMetadataFiles( { manifest_entry->patched_path = plan.generator.generateManifestEntryName(); manifest_file_renamings[manifest_entry->path] = manifest_entry->patched_path; + auto buffer_manifest_entry = object_storage->writeObject( StoredObject(path_resolver.resolve(manifest_entry->patched_path)), WriteMode::Rewrite, @@ -574,6 +598,7 @@ static void writeMetadataFiles( path_resolver, metadata_object, object_storage, + secondary_storages, context, renamed_manifest_entries, new_snapshots[i].snapshot, @@ -601,22 +626,56 @@ static void writeMetadataFiles( } } -static std::vector getOldFiles(ObjectStoragePtr object_storage, const String & table_path) +static std::vector> getOldFiles( + ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, + ContextPtr context, + const PersistentTableComponents & persistent_table_components, + const Plan & plan) { - auto metadata_files = listFiles(*object_storage, table_path, "metadata", ""); - auto data_files = listFiles(*object_storage, table_path, "data", ""); + std::vector> result; - for (auto && data_file : data_files) - metadata_files.push_back(data_file); + /// Base-storage keys already scheduled for removal, to dedupe referenced files against the listings. + std::unordered_set base_storage_keys; - return metadata_files; + for (auto && file : listFiles(*object_storage, persistent_table_components.table_path, "metadata", "")) + { + base_storage_keys.insert(file); + result.emplace_back(object_storage, std::move(file)); + } + for (auto && file : listFiles(*object_storage, persistent_table_components.table_path, "data", "")) + { + base_storage_keys.insert(file); + result.emplace_back(object_storage, std::move(file)); + } + + for (const auto & raw_path : plan.referenced_file_paths) + { + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, + raw_path.serialize(), + object_storage, + secondary_storages, + context, + persistent_table_components.path_resolver); + + /// Secondary-storage files are never in the listings above; base-storage files can also be + /// referenced outside the table `metadata`/`data` prefixes (e.g. a same-bucket external path) + /// and must be removed too. + if (storage_to_use.get() != object_storage.get() || base_storage_keys.insert(key_in_storage).second) + result.emplace_back(std::move(storage_to_use), std::move(key_in_storage)); + } + + return result; } -static void clearOldFiles(ObjectStoragePtr object_storage, const std::vector & old_files) +static void clearOldFiles(const std::vector> & old_files) { - for (const auto & metadata_file : old_files) + auto log = getLogger("IcebergCompaction"); + for (const auto & [storage, key] : old_files) { - object_storage->removeObjectIfExists(StoredObject(metadata_file)); + LOG_DEBUG(log, "Removing old file during compaction: storage={}, key={}", storage->getDescription(), key); + storage->removeObjectIfExists(StoredObject(key)); } } @@ -624,6 +683,7 @@ void compactIcebergTable( IcebergHistory snapshots_info, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage_, + std::shared_ptr secondary_storages_, const DataLakeStorageSettings & data_lake_settings, const std::optional & format_settings_, SharedHeader sample_block_, @@ -637,12 +697,14 @@ void compactIcebergTable( data_lake_settings, persistent_table_components, object_storage_, + *secondary_storages_, write_format, context_, persistent_table_components.metadata_compression_method); if (plan.need_optimize) { - auto old_files = getOldFiles(object_storage_, persistent_table_components.table_path); + auto old_files = getOldFiles( + object_storage_, *secondary_storages_, context_, persistent_table_components, plan); writeDataFiles( plan, sample_block_, @@ -651,9 +713,10 @@ void compactIcebergTable( format_settings_, context_, write_format, - persistent_table_components.metadata_compression_method); - writeMetadataFiles(plan, persistent_table_components.path_resolver, object_storage_, context_, sample_block_, write_format, persistent_table_components.table_path); - clearOldFiles(object_storage_, old_files); + persistent_table_components.metadata_compression_method, + secondary_storages_); + writeMetadataFiles(plan, persistent_table_components.path_resolver, object_storage_, *secondary_storages_, context_, sample_block_, write_format, persistent_table_components.table_path); + clearOldFiles(old_files); } } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h index 0916002f99f3..302bc3d30e69 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h @@ -5,6 +5,7 @@ #include #include #include +#include namespace DB::Iceberg @@ -15,6 +16,7 @@ void compactIcebergTable( IcebergHistory snapshots_info, const PersistentTableComponents & persistent_table_components, DB::ObjectStoragePtr object_storage_, + std::shared_ptr secondary_storages_, const DataLakeStorageSettings & data_lake_settings, const std::optional & format_settings_, DB::SharedHeader sample_block_, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp index c026aae02891..b7228ba06b0a 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -343,16 +344,34 @@ std::pair, Strings> applyRetentionPolicy( // File collection helpers // --------------------------------------------------------------------------- +/// Resolve a metadata path to the identity of the object it points at, so files spelled +/// differently (s3:// vs s3a:// vs https) but pointing at the same object compare equal. +Iceberg::IcebergPathFromMetadata resolveFileIdentity( + const Iceberg::IcebergPathFromMetadata & path, + const ObjectStoragePtr & object_storage, + const PersistentTableComponents & persistent_table_components, + const ContextPtr & context, + SecondaryStorages & secondary_storages) +{ + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.path_resolver.getTableLocation(), + path.serialize(), object_storage, secondary_storages, context, + persistent_table_components.path_resolver); + return Iceberg::IcebergPathFromMetadata::makeStorageIdentity(storage, key); +} + void collectAllFilePaths( const Iceberg::ManifestFileIterator::ManifestFileEntriesHandle & entries_handle, + const ObjectStoragePtr & object_storage, + const PersistentTableComponents & persistent_table_components, + const ContextPtr & context, + SecondaryStorages & secondary_storages, std::set & out) { - for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::DATA)) - out.insert(entry->parsed_entry->file_path_key); - for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::POSITION_DELETE)) - out.insert(entry->parsed_entry->file_path_key); - for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::EQUALITY_DELETE)) - out.insert(entry->parsed_entry->file_path_key); + for (auto content_type : {FileContentType::DATA, FileContentType::POSITION_DELETE, FileContentType::EQUALITY_DELETE}) + for (const auto & entry : entries_handle.getFilesWithoutDeleted(content_type)) + out.insert(resolveFileIdentity( + entry->parsed_entry->file_path_key, object_storage, persistent_table_components, context, secondary_storages)); } void collectRetainedFiles( @@ -364,7 +383,8 @@ void collectRetainedFiles( Int32 current_schema_id, std::set & retained_manifest_paths, std::set & retained_data_file_paths, - std::set & retained_manifest_list_paths) + std::set & retained_manifest_list_paths, + SecondaryStorages & secondary_storages) { for (UInt32 i = 0; i < retained_snapshots->size(); ++i) { @@ -373,17 +393,21 @@ void collectRetainedFiles( continue; auto manifest_list_path = IcebergPathFromMetadata::deserialize(snapshot->getValue(Iceberg::f_manifest_list)); - retained_manifest_list_paths.insert(manifest_list_path); + retained_manifest_list_paths.insert( + resolveFileIdentity(manifest_list_path, object_storage, persistent_table_components, context, secondary_storages)); - auto manifest_keys = getManifestList(object_storage, persistent_table_components, context, manifest_list_path, log); + auto manifest_keys = getManifestList( + object_storage, persistent_table_components, context, manifest_list_path, log, secondary_storages); for (const auto & manifest_entry : manifest_keys) { - retained_manifest_paths.insert(manifest_entry.manifest_file_path); + retained_manifest_paths.insert( + resolveFileIdentity(manifest_entry.manifest_file_path, object_storage, persistent_table_components, context, secondary_storages)); auto entries_handle = getManifestFileEntriesHandle( object_storage, persistent_table_components, context, log, - manifest_entry, current_schema_id); - collectAllFilePaths(entries_handle, retained_data_file_paths); + manifest_entry, current_schema_id, secondary_storages); + collectAllFilePaths( + entries_handle, object_storage, persistent_table_components, context, secondary_storages, retained_data_file_paths); } } } @@ -407,23 +431,34 @@ ExpiredFiles collectExpiredFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + SecondaryStorages & secondary_storages) { ExpiredFiles result; std::set seen_expired_manifest_list_paths; std::set seen_expired_manifest_paths; for (const auto & manifest_list_path : expired_manifest_list_paths) { - if (retained_manifest_list_paths.contains(manifest_list_path)) + Iceberg::IcebergPathFromMetadata manifest_list_id; + try + { + manifest_list_id = resolveFileIdentity(manifest_list_path, object_storage, persistent_table_components, context, secondary_storages); + } + catch (...) + { + LOG_WARNING(log, "Failed to resolve manifest list {}, skipping", manifest_list_path); + continue; + } + if (retained_manifest_list_paths.contains(manifest_list_id)) continue; - if (seen_expired_manifest_list_paths.contains(manifest_list_path)) + if (seen_expired_manifest_list_paths.contains(manifest_list_id)) continue; ManifestFileCacheKeys manifest_keys; try { - manifest_keys = getManifestList(object_storage, persistent_table_components, context, manifest_list_path, log); + manifest_keys = getManifestList(object_storage, persistent_table_components, context, manifest_list_path, log, secondary_storages); } catch (...) { @@ -433,32 +468,42 @@ ExpiredFiles collectExpiredFiles( for (const auto & manifest_entry : manifest_keys) { - if (retained_manifest_paths.contains(manifest_entry.manifest_file_path)) + Iceberg::IcebergPathFromMetadata manifest_id; + try + { + manifest_id = resolveFileIdentity(manifest_entry.manifest_file_path, object_storage, persistent_table_components, context, secondary_storages); + } + catch (...) + { + LOG_WARNING(log, "Failed to resolve manifest file {}, skipping", manifest_entry.manifest_file_path); + continue; + } + if (retained_manifest_paths.contains(manifest_id)) continue; - if (seen_expired_manifest_paths.contains(manifest_entry.manifest_file_path)) + if (seen_expired_manifest_paths.contains(manifest_id)) continue; try { auto entries_handle = getManifestFileEntriesHandle( object_storage, persistent_table_components, context, log, - manifest_entry, current_schema_id); + manifest_entry, current_schema_id, secondary_storages); for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::DATA)) - if (!retained_data_file_paths.contains(entry->parsed_entry->file_path_key)) + if (!retained_data_file_paths.contains(resolveFileIdentity(entry->parsed_entry->file_path_key, object_storage, persistent_table_components, context, secondary_storages))) { result.all_paths.push_back(entry->parsed_entry->file_path_key); ++result.data_files; } for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::POSITION_DELETE)) - if (!retained_data_file_paths.contains(entry->parsed_entry->file_path_key)) + if (!retained_data_file_paths.contains(resolveFileIdentity(entry->parsed_entry->file_path_key, object_storage, persistent_table_components, context, secondary_storages))) { result.all_paths.push_back(entry->parsed_entry->file_path_key); ++result.position_delete_files; } for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::EQUALITY_DELETE)) - if (!retained_data_file_paths.contains(entry->parsed_entry->file_path_key)) + if (!retained_data_file_paths.contains(resolveFileIdentity(entry->parsed_entry->file_path_key, object_storage, persistent_table_components, context, secondary_storages))) { result.all_paths.push_back(entry->parsed_entry->file_path_key); ++result.equality_delete_files; @@ -470,12 +515,12 @@ ExpiredFiles collectExpiredFiles( continue; } - seen_expired_manifest_paths.insert(manifest_entry.manifest_file_path); + seen_expired_manifest_paths.insert(manifest_id); result.all_paths.push_back(manifest_entry.manifest_file_path); ++result.manifest_files; } - seen_expired_manifest_list_paths.insert(manifest_list_path); + seen_expired_manifest_list_paths.insert(manifest_list_id); result.all_paths.push_back(manifest_list_path); ++result.manifest_lists; } @@ -633,13 +678,18 @@ void deleteExpiredFiles( const std::vector & files_to_delete, const Iceberg::IcebergPathResolver & path_resolver, ObjectStoragePtr object_storage, - LoggerPtr log) + ContextPtr context, + LoggerPtr log, + SecondaryStorages & secondary_storages) { for (const auto & file_path : files_to_delete) { try { - object_storage->removeObjectIfExists(StoredObject(path_resolver.resolve(file_path))); + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + path_resolver.getTableLocation(), file_path.serialize(), object_storage, secondary_storages, context, + path_resolver); + storage_to_use->removeObjectIfExists(StoredObject(key_in_storage)); LOG_DEBUG(log, "Deleted expired file {}", file_path); } catch (...) @@ -664,7 +714,8 @@ ExpireSnapshotsResult expireSnapshots( const PersistentTableComponents & persistent_table_components, const String & write_format, std::shared_ptr catalog, - const String & table_name) + const String & table_name, + SecondaryStorages & secondary_storages) { auto common_path = persistent_table_components.table_path; if (!common_path.starts_with('/')) @@ -747,10 +798,12 @@ ExpireSnapshotsResult expireSnapshots( std::set retained_manifest_list_paths; collectRetainedFiles( partition.retained_snapshots, object_storage, persistent_table_components, context, log, - current_schema_id, retained_manifest_paths, retained_data_file_paths, retained_manifest_list_paths); + current_schema_id, retained_manifest_paths, retained_data_file_paths, retained_manifest_list_paths, + secondary_storages); auto expired_files = collectExpiredFiles( partition.expired_manifest_list_paths, retained_manifest_list_paths, retained_manifest_paths, retained_data_file_paths, - object_storage, persistent_table_components, context, log, current_schema_id); + object_storage, persistent_table_components, context, log, current_schema_id, + secondary_storages); if (options.dry_run) { @@ -799,7 +852,7 @@ ExpireSnapshotsResult expireSnapshots( } LOG_INFO(log, "Deleting {} expired files for {} expired snapshots", expired_files.all_paths.size(), partition.expired_snapshot_ids.size()); - deleteExpiredFiles(expired_files.all_paths, persistent_table_components.path_resolver, object_storage, log); + deleteExpiredFiles(expired_files.all_paths, persistent_table_components.path_resolver, object_storage, context, log, secondary_storages); LOG_INFO(log, "Expired {} snapshots, deleted {} files", partition.expired_snapshot_ids.size(), expired_files.all_paths.size()); return ExpireSnapshotsResult{ @@ -831,7 +884,8 @@ Pipe executeExpireSnapshots( const PersistentTableComponents & persistent_components, const String & write_format, std::shared_ptr catalog, - const String & table_name) + const String & table_name, + SecondaryStorages & secondary_storages) { auto parsed = makeSchema().parse(args); auto options = buildOptions(parsed); @@ -844,7 +898,8 @@ Pipe executeExpireSnapshots( persistent_components, write_format, catalog, - table_name); + table_name, + secondary_storages); return resultToPipe(result); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h index e3a682c6c56b..4a6e97eea4ad 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h @@ -10,6 +10,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -22,7 +23,8 @@ ExpireSnapshotsResult expireSnapshots( const PersistentTableComponents & persistent_table_components, const String & write_format, std::shared_ptr catalog, - const String & table_name); + const String & table_name, + SecondaryStorages & secondary_storages); Pipe executeExpireSnapshots( const ASTPtr & args, @@ -32,7 +34,8 @@ Pipe executeExpireSnapshots( const PersistentTableComponents & persistent_components, const String & write_format, std::shared_ptr catalog, - const String & table_name); + const String & table_name, + SecondaryStorages & secondary_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp index 4e161ca8c863..5eb3f8501350 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp @@ -1,3 +1,4 @@ +#include #include "config.h" #include @@ -11,15 +12,18 @@ #include #include +#include #include #include #include +#include namespace DB::ErrorCodes { extern const int NOT_IMPLEMENTED; extern const int UNKNOWN_PROTOCOL; +extern const int PROTOCOL_VERSION_MISMATCH; } @@ -53,10 +57,11 @@ String computePartitionId(const Row & partition_key_value) #if USE_AVRO IcebergDataObjectInfo::IcebergDataObjectInfo( - Iceberg::ProcessedManifestFileEntryPtr data_manifest_file_entry_, const String & resolved_storage_path_, Int32 schema_id_relevant_to_iterator_) - : ObjectInfo(RelativePathWithMetadata(resolved_storage_path_)) + Iceberg::ProcessedManifestFileEntryPtr data_manifest_file_entry_, const String & metadata_path_, Int32 schema_id_relevant_to_iterator_, ObjectStoragePtr resolved_storage_, const String & resolved_key_) + : ObjectInfo(RelativePathWithMetadata(resolved_key_.empty() ? metadata_path_ : resolved_key_)) , info{ data_manifest_file_entry_->parsed_entry->file_path_key, + metadata_path_, data_manifest_file_entry_->resolved_schema_id, schema_id_relevant_to_iterator_, data_manifest_file_entry_->sequence_number, @@ -67,7 +72,11 @@ IcebergDataObjectInfo::IcebergDataObjectInfo( /* equality_deletes_objects */ {}, data_manifest_file_entry_->parsed_entry->record_count, data_manifest_file_entry_->parsed_entry->file_size_in_bytes} + , resolved_storage(std::move(resolved_storage_)) { + /// resolved_storage and resolved_key must be provided together or neither must be provided + /// (default-constructed, meaning the path has not been resolved yet). + chassert(resolved_key_.empty() == (resolved_storage == nullptr)); } IcebergDataObjectInfo::IcebergDataObjectInfo(const RelativePathWithMetadata & path_) @@ -86,13 +95,15 @@ std::shared_ptr IcebergDataObjectInfo::getPositionDeleteTransf const SharedHeader & header, const std::optional & format_settings, FormatParserSharedResourcesPtr parser_shared_resources, - ContextPtr context_) + ContextPtr context_, + const Iceberg::IcebergPathResolver & path_resolver, + std::shared_ptr secondary_storages) { IcebergDataObjectInfoPtr self = shared_from_this(); if (!context_->getSettingsRef()[Setting::use_roaring_bitmap_iceberg_positional_deletes].value) - return std::make_shared(header, self, object_storage, format_settings, parser_shared_resources, context_); + return std::make_shared(header, self, object_storage, format_settings, parser_shared_resources, context_, path_resolver, secondary_storages); else - return std::make_shared(header, self, object_storage, format_settings, parser_shared_resources, context_); + return std::make_shared(header, self, object_storage, format_settings, parser_shared_resources, context_, path_resolver, secondary_storages); } void IcebergDataObjectInfo::addPositionDeleteObject(Iceberg::ProcessedManifestFileEntryPtr position_delete_object, const String & resolved_storage_path) @@ -123,7 +134,30 @@ void IcebergDataObjectInfo::addEqualityDeleteObject(const Iceberg::ProcessedMani void IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version) const { checkVersion(protocol_version); + + if (requires_external_storage && protocol_version < DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH) + { + throw Exception( + ErrorCodes::PROTOCOL_VERSION_MISMATCH, + "Iceberg data file '{}' is outside of the table location, " + "worker needs to have protocol version >= {}, but has {}. ", + data_object_file_metadata_path, + DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH, + protocol_version); + } + + auto path_for_protocol = [&](const String & path) -> String + { + if (protocol_version < DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH) + return SchemeAuthorityKey(path).key; + return path; + }; + writeStringBinary(data_object_file_path_key.serialize(), out); + if (protocol_version >= DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH) + { + writeStringBinary(data_object_file_metadata_path, out); + } writeVarInt(underlying_format_read_schema_id, out); writeVarInt(schema_id_relevant_to_iterator, out); writeVarInt(sequence_number, out); @@ -132,12 +166,12 @@ void IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol(WriteBuf writeVarUInt(position_deletes_objects.size(), out); for (const auto & pos_delete_obj : position_deletes_objects) { - writeStringBinary(pos_delete_obj.file_path, out); + writeStringBinary(path_for_protocol(pos_delete_obj.file_path), out); writeStringBinary(pos_delete_obj.file_format, out); if (pos_delete_obj.reference_data_file_path.has_value()) { writeVarUInt(1, out); - writeStringBinary(pos_delete_obj.reference_data_file_path.value(), out); + writeStringBinary(path_for_protocol(pos_delete_obj.reference_data_file_path.value()), out); } else { @@ -149,7 +183,7 @@ void IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol(WriteBuf writeVarUInt(equality_deletes_objects.size(), out); for (const auto & eq_delete_obj : equality_deletes_objects) { - writeStringBinary(eq_delete_obj.file_path, out); + writeStringBinary(path_for_protocol(eq_delete_obj.file_path), out); writeStringBinary(eq_delete_obj.file_format, out); writeVarInt(eq_delete_obj.schema_id, out); if (eq_delete_obj.equality_ids.has_value()) @@ -198,6 +232,10 @@ void IcebergObjectSerializableInfo::deserializeForClusterFunctionProtocol(ReadBu readStringBinary(raw_path, in); data_object_file_path_key = IcebergPathFromMetadata::deserialize(std::move(raw_path)); } + if (protocol_version >= DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH) + { + readStringBinary(data_object_file_metadata_path, in); + } readVarInt(underlying_format_read_schema_id, in); readVarInt(schema_id_relevant_to_iterator, in); readVarInt(sequence_number, in); diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h index 6763d1013201..4dcb6da86a75 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h @@ -21,6 +21,10 @@ String computePartitionId(const Row & partition_key_value); struct IcebergObjectSerializableInfo { IcebergPathFromMetadata data_object_file_path_key; + /// Raw path string as written in the Iceberg manifest, preserved as-is (may be a full URI like + /// `s3://bucket/...` or a relative path). Used for the `_path` virtual column and as a stable + /// task identifier. Not a canonicalised storage key — see `IcebergPathResolver::resolve` for that. + String data_object_file_metadata_path; Int32 underlying_format_read_schema_id{}; Int32 schema_id_relevant_to_iterator{}; Int64 sequence_number{}; @@ -32,6 +36,9 @@ struct IcebergObjectSerializableInfo std::optional record_count; std::optional file_size_in_bytes; + /// Set to true by the coordinator when the file is outside of the table location + bool requires_external_storage = false; + void serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version) const; void deserializeForClusterFunctionProtocol(ReadBuffer & in, size_t protocol_version); @@ -44,6 +51,7 @@ struct IcebergObjectSerializableInfo #if USE_AVRO #include +#include #include @@ -60,7 +68,14 @@ struct IcebergDataObjectInfo : public ObjectInfo, std::enable_shared_from_this & format_settings, FormatParserSharedResourcesPtr parser_shared_resources, - ContextPtr context_); + ContextPtr context_, + const Iceberg::IcebergPathResolver & path_resolver, + std::shared_ptr secondary_storages); std::optional getFileFormat() const override { return info.file_format; } @@ -83,8 +100,25 @@ struct IcebergDataObjectInfo : public ObjectInfo, std::enable_shared_from_this getMetadataPath() const + { + if (info.data_object_file_metadata_path.empty()) + return std::nullopt; + return info.data_object_file_metadata_path; + } + + std::shared_ptr clone() const override { return std::make_shared(*this); } + + ObjectStoragePtr getResolvedStorage() const { return resolved_storage; } + + void setResolvedStorage(ObjectStoragePtr storage) { resolved_storage = std::move(storage); } + void addEqualityDeleteObject(const Iceberg::ProcessedManifestFileEntryPtr & equality_delete_object, const String & resolved_storage_path); Iceberg::IcebergObjectSerializableInfo info; + +private: + /// For files located in a different storage than the table's main storage + ObjectStoragePtr resolved_storage; }; using IcebergDataObjectInfoPtr = std::shared_ptr; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp index 39cc26a5fd6a..c8194f05c782 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include @@ -185,7 +186,8 @@ std::optional SingleThreadIcebergKeysIterator::ne local_context, log, manifest_list_entry.manifest_file_path, - manifest_list_entry.manifest_file_byte_size); + manifest_list_entry.manifest_file_byte_size, + *secondary_storages); current_manifest_file_iterator = Iceberg::ManifestFileIterator::create( manifest_file_cacheable_part.deserializer, @@ -212,7 +214,8 @@ SingleThreadIcebergKeysIterator::SingleThreadIcebergKeysIterator( const ActionsDAG * filter_dag_, Iceberg::TableStateSnapshotPtr table_snapshot_, Iceberg::IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components_) + PersistentTableComponents persistent_components_, + std::shared_ptr secondary_storages_) : object_storage(object_storage_) , filter_dag( [&]() -> std::shared_ptr @@ -235,6 +238,7 @@ SingleThreadIcebergKeysIterator::SingleThreadIcebergKeysIterator( , data_snapshot(data_snapshot_) , persistent_components(persistent_components_) , log(getLogger("IcebergIterator")) + , secondary_storages(secondary_storages_) , manifest_file_content_type(manifest_file_content_type_) { } @@ -246,10 +250,12 @@ IcebergIterator::IcebergIterator( IDataLakeMetadata::FileProgressCallback callback_, Iceberg::TableStateSnapshotPtr table_snapshot_, Iceberg::IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components_) + PersistentTableComponents persistent_components_, + std::shared_ptr secondary_storages_) : logger(getLogger("IcebergIterator")) , filter_dag(filter_dag_ ? std::make_shared(filter_dag_->clone()) : nullptr) , object_storage(std::move(object_storage_)) + , local_context(local_context_) , table_state_snapshot(table_snapshot_) , persistent_components(persistent_components_) , data_files_iterator( @@ -259,7 +265,8 @@ IcebergIterator::IcebergIterator( filter_dag.get(), table_snapshot_, data_snapshot_, - persistent_components_) + persistent_components_, + secondary_storages_) , deletes_iterator( object_storage, local_context_, @@ -267,9 +274,11 @@ IcebergIterator::IcebergIterator( filter_dag.get(), table_snapshot_, data_snapshot_, - persistent_components_) + persistent_components_, + secondary_storages_) , blocking_queue(100) , callback(std::move(callback_)) + , secondary_storages(secondary_storages_) { auto delete_file = deletes_iterator.next(); while (delete_file.has_value()) @@ -328,11 +337,17 @@ ObjectInfoPtr IcebergIterator::next(size_t) Iceberg::ProcessedManifestFileEntryPtr manifest_file_entry; if (blocking_queue.pop(manifest_file_entry)) { - IcebergDataObjectInfoPtr object_info - = std::make_shared( - manifest_file_entry, - persistent_components.path_resolver.resolve(manifest_file_entry->parsed_entry->file_path_key), - table_state_snapshot->schema_id); + const auto & raw_metadata_path = manifest_file_entry->parsed_entry->file_path_key.serialize(); + auto [storage_to_use, resolved_key] = resolveObjectStorageForPath( + persistent_components.table_location, raw_metadata_path, + object_storage, *secondary_storages, local_context, + persistent_components.path_resolver); + + IcebergDataObjectInfoPtr object_info = std::make_shared( + manifest_file_entry, raw_metadata_path, table_state_snapshot->schema_id, storage_to_use, resolved_key); + + object_info->info.requires_external_storage = (storage_to_use != object_storage); + for (const auto & position_delete : defineDeletesSpan(manifest_file_entry, position_deletes_files, /* is_equality_delete */ false, logger)) { @@ -368,7 +383,7 @@ ObjectInfoPtr IcebergIterator::next(size_t) lower.has_value() ? lower->serialize() : "[no lower bound]", upper.has_value() ? upper->serialize() : "[no upper bound]"); object_info->addPositionDeleteObject( - position_delete, persistent_components.path_resolver.resolve(position_delete->parsed_entry->file_path_key)); + position_delete, position_delete->parsed_entry->file_path_key.serialize()); } } @@ -385,7 +400,7 @@ ObjectInfoPtr IcebergIterator::next(size_t) defineDeletesSpan(manifest_file_entry, equality_deletes_files, /* is_equality_delete */ true, logger)) { object_info->addEqualityDeleteObject( - equality_delete, persistent_components.path_resolver.resolve(equality_delete->parsed_entry->file_path_key)); + equality_delete, equality_delete->parsed_entry->file_path_key.serialize()); } if (!object_info->info.equality_deletes_objects.empty()) @@ -397,6 +412,42 @@ ObjectInfoPtr IcebergIterator::next(size_t) object_info->info.data_object_file_path_key); } + if (!object_info->info.requires_external_storage) + { + /// Flag the file if it resolves to a different storage/key (e.g. a same-bucket file outside the table prefix) + auto needs_absolute_path_protocol = [&](const String & file_path) + { + auto [del_storage, del_key] = resolveObjectStorageForPath( + persistent_components.table_location, file_path, object_storage, *secondary_storages, local_context, + persistent_components.path_resolver); + if (del_storage != object_storage) + return true; + try + { + auto [stripped_storage, stripped_key] = resolveObjectStorageForPath( + persistent_components.table_location, SchemeAuthorityKey(file_path).key, object_storage, + *secondary_storages, local_context, persistent_components.path_resolver); + return stripped_storage != object_storage || stripped_key != del_key; + } + catch (const Exception &) + { + /// The stripped key is unresolvable, so old workers cannot read it either. + return true; + } + }; + auto any_needs_protocol = [&](const auto & delete_objects) + { + for (const auto & del : delete_objects) + if (needs_absolute_path_protocol(del.file_path)) + return true; + return false; + }; + + object_info->info.requires_external_storage = + any_needs_protocol(object_info->info.position_deletes_objects) + || any_needs_protocol(object_info->info.equality_deletes_objects); + } + ProfileEvents::increment(ProfileEvents::IcebergMetadataReturnedObjectInfos); if (callback) diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h index 09c21b8ec292..36542ac96dee 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h @@ -27,6 +27,7 @@ #include #include #include +#include namespace DB { @@ -44,7 +45,8 @@ class SingleThreadIcebergKeysIterator const ActionsDAG * filter_dag_, TableStateSnapshotPtr table_snapshot_, IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components); + PersistentTableComponents persistent_components, + std::shared_ptr secondary_storages_); std::optional next(); @@ -57,6 +59,8 @@ class SingleThreadIcebergKeysIterator PersistentTableComponents persistent_components; LoggerPtr log; + std::shared_ptr secondary_storages; + size_t manifest_file_index = 0; Iceberg::ManifestIteratorPtr current_manifest_file_iterator; @@ -75,7 +79,8 @@ class IcebergIterator : public IObjectIterator IDataLakeMetadata::FileProgressCallback callback_, Iceberg::TableStateSnapshotPtr table_snapshot_, Iceberg::IcebergDataSnapshotPtr data_snapshot_, - Iceberg::PersistentTableComponents persistent_components); + Iceberg::PersistentTableComponents persistent_components, + std::shared_ptr secondary_storages_); ObjectInfoPtr next(size_t) override; @@ -86,6 +91,7 @@ class IcebergIterator : public IObjectIterator LoggerPtr logger; std::shared_ptr filter_dag; ObjectStoragePtr object_storage; + ContextPtr local_context; const Iceberg::TableStateSnapshotPtr table_state_snapshot; Iceberg::PersistentTableComponents persistent_components; Iceberg::SingleThreadIcebergKeysIterator data_files_iterator; @@ -97,6 +103,7 @@ class IcebergIterator : public IObjectIterator std::vector equality_deletes_files; std::exception_ptr exception; std::mutex exception_mutex; + std::shared_ptr secondary_storages; // Sometimes data or manifests can be located on another storage }; } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp index 82adf64ab122..526bcb475cf0 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp @@ -5,6 +5,7 @@ #include "config.h" #if USE_AVRO +#include #include #include #include @@ -77,6 +78,7 @@ #include #include #include +#include #include #include @@ -226,6 +228,7 @@ IcebergMetadata::IcebergMetadata( ContextPtr context_) : log(getLogger("IcebergMetadata")) , object_storage(std::move(object_storage_)) + , secondary_storages(std::make_shared()) , persistent_components(std::move(persistent_components_)) , data_lake_settings(configuration_->getDataLakeSettings()) , write_format(configuration_->format) @@ -284,7 +287,7 @@ void IcebergMetadata::backgroundMetadataPrefetcherThread() { /// second, we fetch, parse and cache each manifest file auto manifest_file_ptr = getManifestFileEntriesHandle( - object_storage, persistent_components, ctx, log, entry, actual_table_state_snapshot.schema_id); + object_storage, persistent_components, ctx, log, entry, actual_table_state_snapshot.schema_id, *secondary_storages); } } @@ -414,7 +417,7 @@ IcebergDataSnapshotPtr IcebergMetadata::createIcebergDataSnapshotFromSnapshotJSO return std::make_shared( - getManifestList(object_storage, persistent_components, local_context, manifest_list_file_path, log), + getManifestList(object_storage, persistent_components, local_context, manifest_list_file_path, log, *secondary_storages), snapshot_id, schema_id, total_rows, @@ -443,6 +446,7 @@ bool IcebergMetadata::optimize( snapshots_info, persistent_components, object_storage, + secondary_storages, data_lake_settings, format_settings, sample_block, @@ -603,6 +607,7 @@ void IcebergMetadata::mutate( metadata_snapshot, storage_id, object_storage, + *secondary_storages, data_lake_settings, persistent_components, write_format, @@ -683,7 +688,7 @@ Pipe IcebergMetadata::executeCommand( return Iceberg::executeExpireSnapshots( args, context, object_storage_, data_lake_settings, persistent_components, - write_format, catalog_, storage_id.getTableName()); + write_format, catalog_, storage_id.getTableName(), *secondary_storages); } else if (command_name == "remove_orphan_files") { @@ -696,7 +701,7 @@ Pipe IcebergMetadata::executeCommand( } return Iceberg::executeRemoveOrphanFiles( - args, context, object_storage_, data_lake_settings, persistent_components); + args, context, object_storage_, data_lake_settings, persistent_components, *secondary_storages); } else { @@ -992,7 +997,7 @@ IcebergMetadata::IcebergFiles IcebergMetadata::getFilesForManifest( const auto & manifest_list_entry = data_snapshot->manifest_list_entries[manifest_index]; auto handle = getManifestFileEntriesHandle( - object_storage, persistent_components, local_context, log, manifest_list_entry, table_state.schema_id); + object_storage, persistent_components, local_context, log, manifest_list_entry, table_state.schema_id, *secondary_storages); IcebergFiles result; for (auto content_type : {FileContentType::DATA, FileContentType::POSITION_DELETE, FileContentType::EQUALITY_DELETE}) @@ -1029,7 +1034,7 @@ bool IcebergMetadata::isDataSortedBySortingKey(StorageMetadataPtr storage_metada for (const auto & manifest_list_entry : data_snapshot->manifest_list_entries) { auto files_handle = getManifestFileEntriesHandle( - object_storage, persistent_components, context, log, manifest_list_entry, table_state_snapshot->schema_id); + object_storage, persistent_components, context, log, manifest_list_entry, table_state_snapshot->schema_id, *secondary_storages); if (!files_handle.areAllDataFilesSortedBySortOrderID(sorting_key.sort_order_id.value())) return false; @@ -1060,7 +1065,7 @@ std::optional IcebergMetadata::totalRows(ContextPtr local_context) const for (const auto & manifest_list_entry : actual_data_snapshot->manifest_list_entries) { auto manifest_file_ptr = getManifestFileEntriesHandle( - object_storage, persistent_components, local_context, log, manifest_list_entry, actual_table_state_snapshot.schema_id); + object_storage, persistent_components, local_context, log, manifest_list_entry, actual_table_state_snapshot.schema_id, *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()) @@ -1089,7 +1094,7 @@ std::optional IcebergMetadata::totalBytes(ContextPtr local_context) cons for (const auto & manifest_list_entry : actual_data_snapshot->manifest_list_entries) { auto manifest_file_ptr = getManifestFileEntriesHandle( - object_storage, persistent_components, local_context, log, manifest_list_entry, actual_table_state_snapshot.schema_id); + object_storage, persistent_components, local_context, log, manifest_list_entry, actual_table_state_snapshot.schema_id, *secondary_storages); auto count = manifest_file_ptr.getBytesCountInAllDataFilesExcludingDeleted(); if (!count.has_value()) return {}; @@ -1125,7 +1130,8 @@ ObjectIterator IcebergMetadata::iterate( callback, iceberg_table_state, getRelevantDataSnapshotFromTableStateSnapshot(*iceberg_table_state, local_context), - persistent_components); + persistent_components, + secondary_storages); } NamesAndTypesList IcebergMetadata::getTableSchema(ContextPtr local_context) const @@ -1182,7 +1188,7 @@ void IcebergMetadata::addDeleteTransformers( { builder.addSimpleTransform( [&](const SharedHeader & header) - { return iceberg_object_info->getPositionDeleteTransformer(object_storage, header, format_settings, parser_shared_resources, local_context); }); + { return iceberg_object_info->getPositionDeleteTransformer(object_storage, header, format_settings, parser_shared_resources, local_context, persistent_components.path_resolver, secondary_storages); }); } const auto & delete_files = iceberg_object_info->info.equality_deletes_objects; LOG_DEBUG(log, "Constructing filter transform for equality delete, there are {} delete files", delete_files.size()); @@ -1192,9 +1198,14 @@ void IcebergMetadata::addDeleteTransformers( { /// get header of delete file Block delete_file_header; - RelativePathWithMetadata delete_file_object(delete_file.file_path); + + 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.path_resolver); + + RelativePathWithMetadata delete_file_object(resolved_delete_key); { - auto schema_read_buffer = createReadBuffer(delete_file_object, object_storage, local_context, log); + auto schema_read_buffer = createReadBuffer(delete_file_object, delete_storage_to_use, local_context, log); auto schema_reader = FormatFactory::instance().getSchemaReader(delete_file.file_format, *schema_read_buffer, local_context); auto columns_with_names = schema_reader->readSchema(); ColumnsWithTypeAndName initial_header_data; @@ -1221,7 +1232,7 @@ void IcebergMetadata::addDeleteTransformers( } /// Then we read the content of the delete file. auto mutable_columns_for_set = block_for_set.cloneEmptyColumns(); - std::unique_ptr data_read_buffer = createReadBuffer(delete_file_object, object_storage, local_context, log); + std::unique_ptr data_read_buffer = createReadBuffer(delete_file_object, delete_storage_to_use, local_context, log); CompressionMethod compression_method = chooseCompressionMethod(delete_file.file_path, "auto"); auto delete_format = FormatFactory::instance().getInput( delete_file.file_format, @@ -1299,7 +1310,7 @@ SinkToStoragePtr IcebergMetadata::write( { if (context->getSettingsRef()[Setting::allow_insert_into_iceberg]) { - return std::make_shared(object_storage, configuration, format_settings, sample_block, context, catalog, persistent_components, table_id); + return std::make_shared(object_storage, configuration, format_settings, sample_block, context, catalog, persistent_components, table_id, secondary_storages); } else { @@ -1312,12 +1323,49 @@ SinkToStoragePtr IcebergMetadata::write( void IcebergMetadata::drop(ContextPtr context) { - if (context->getSettingsRef()[Setting::iceberg_delete_data_on_drop].value) + if (!context->getSettingsRef()[Setting::iceberg_delete_data_on_drop].value) + return; + + /// Files outside `table_path` (secondary storage, or base storage elsewhere in the bucket) are only + /// discoverable through the metadata graph the base wipe below removes, so enumerate them first. Let + /// a failure propagate rather than wiping the metadata re-enumeration on retry depends on (fail closed). + auto external_files = Iceberg::collectReachableFiles( + object_storage, persistent_components, data_lake_settings, context, log, *secondary_storages).external_files; + + /// Delete these files leaf-first (reverse of the traversal's append order) so an interrupted drop + /// can re-enumerate the rest on retry; batch per storage. Shared files are deleted too, as with `PURGE`. + std::reverse(external_files.begin(), external_files.end()); + for (size_t i = 0; i < external_files.size();) { - auto files = listFiles(*object_storage, persistent_components.table_path, persistent_components.table_path, ""); - for (const auto & file : files) - object_storage->removeObjectIfExists(StoredObject(file)); + auto storage = external_files[i].first; + StoredObjects batch; + while (i < external_files.size() && external_files[i].first.get() == storage.get()) + { + batch.emplace_back(external_files[i].second); + ++i; + } + /// Log per object before removal (as in `clearOldFiles`): `removeObjectsIfExist` is best-effort + /// and does not confirm each object was present, so record the attempt; a fail-closed interrupt + /// then still leaves an audit trail of what this drop was purging. + const auto storage_description = storage->getDescription(); + for (const auto & object : batch) + LOG_DEBUG(log, "Removing external file during drop: storage={}, key={}", storage_description, object.remote_path); + storage->removeObjectsIfExist(batch); } + + /// Wipe the base subtree last, restricted to `table_path`: referenced files elsewhere in the bucket + /// were already deleted above via `external_files`, and unreferenced objects elsewhere (possibly + /// another table's data) must be left alone. `listFiles` joins path and prefix, so the prefix must be + /// empty: passing `table_path` for both scans the non-existent `table_path/table_path`. + auto files = listFiles(*object_storage, persistent_components.table_path, "", ""); + StoredObjects base_objects; + base_objects.reserve(files.size()); + for (const auto & file : files) + base_objects.emplace_back(file); + const auto base_description = object_storage->getDescription(); + for (const auto & object : base_objects) + LOG_DEBUG(log, "Removing file during drop: storage={}, key={}", base_description, object.remote_path); + object_storage->removeObjectsIfExist(base_objects); } ColumnMapperPtr IcebergMetadata::getColumnMapperForObject(ObjectInfoPtr object_info) const diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h index ee7b86b47791..b216b5f5c42d 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h @@ -28,6 +28,7 @@ #include #include #include +#include namespace DB { @@ -139,6 +140,8 @@ class IcebergMetadata : public IDataLakeMetadata CompressionMethod getCompressionMethod() const { return persistent_components.metadata_compression_method; } + std::string getTableLocation() const override { return persistent_components.table_location; } + bool optimize(const StorageMetadataPtr & metadata_snapshot, ContextPtr context, const std::optional & format_settings) override; bool supportsDelete() const override { return true; } void mutate( @@ -200,7 +203,8 @@ class IcebergMetadata : public IDataLakeMetadata LoggerPtr log; const ObjectStoragePtr object_storage; - const DB::Iceberg::PersistentTableComponents persistent_components; + mutable std::shared_ptr secondary_storages; + DB::Iceberg::PersistentTableComponents persistent_components; const DataLakeStorageSettings & data_lake_settings; const String write_format; BackgroundSchedulePoolTaskHolder background_metadata_prefetch_task; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.cpp index 833d5f179917..71f55cd6add3 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.cpp @@ -1,6 +1,7 @@ #include #include +#include #include namespace DB::ErrorCodes @@ -11,6 +12,11 @@ extern const int BAD_ARGUMENTS; namespace DB::Iceberg { +IcebergPathFromMetadata IcebergPathFromMetadata::makeStorageIdentity(const ObjectStoragePtr & storage, const String & key) +{ + return IcebergPathFromMetadata(storage->getDescription() + '\0' + storage->getObjectsNamespace() + '\0' + key); +} + // This function is used to get the file path inside the directory which corresponds to Iceberg table from the full blob path which is written in manifest and metadata files. // For example, if the full blob path is s3://bucket/table_name/data/00000-1-1234567890.avro, the function will return table_name/data/00000-1-1234567890.avro // Common path should end with "" or "/". diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h index e3e970ce12bf..26cb3a564d0f 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h @@ -3,6 +3,8 @@ #include #include +#include + namespace DB { class FileNamesGenerator; @@ -33,6 +35,10 @@ class IcebergPathFromMetadata /// Also needed to get the file which corresponds to a line in the Chunk when used for position-delete algorithms. static IcebergPathFromMetadata deserialize(String path_) { return IcebergPathFromMetadata(std::move(path_)); } + /// Identity of the physical object a path resolves to, as the triple (storage description, namespace, key). + /// Lets paths spelled differently (s3:// vs s3a:// vs https) but pointing at the same object compare equal. + static IcebergPathFromMetadata makeStorageIdentity(const ObjectStoragePtr & storage, const String & key); + /// Extract the raw path string for writing into Iceberg metadata files, /// serialization, cache keys, virtual column values, etc. const String & serialize() const { return raw_path; } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp index 9529764f1fdd..9da8ef72c7f9 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -465,6 +466,7 @@ void generateManifestList( const Iceberg::IcebergPathResolver & path_resolver, Poco::JSON::Object::Ptr metadata, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, ContextPtr context, const std::vector & manifest_entry_names, Poco::JSON::Object::Ptr new_snapshot, @@ -571,8 +573,9 @@ void generateManifestList( auto manifest_list = Iceberg::IcebergPathFromMetadata::deserialize( snapshots->getObject(static_cast(i))->getValue(Iceberg::f_manifest_list)); - auto resolved_manifest_list_path = path_resolver.resolve(manifest_list); - forEachAvroEntry(resolved_manifest_list_path, object_storage, context, "IcebergWrites", + auto [manifest_list_storage, resolved_manifest_list_path] = resolveObjectStorageForPath( + path_resolver.getTableLocation(), manifest_list.serialize(), object_storage, secondary_storages, context, path_resolver); + forEachAvroEntry(resolved_manifest_list_path, manifest_list_storage, context, "IcebergWrites", [&](const avro::GenericDatum & datum) { const avro::GenericRecord & old_entry = datum.value(); @@ -646,7 +649,8 @@ IcebergStorageSink::IcebergStorageSink( ContextPtr context_, std::shared_ptr catalog_, const Iceberg::PersistentTableComponents & persistent_table_components_, - const StorageID & table_id_) + const StorageID & table_id_, + std::shared_ptr secondary_storages_) : SinkToStorage(sample_block_) , sample_block(sample_block_) , object_storage(object_storage_) @@ -657,6 +661,7 @@ IcebergStorageSink::IcebergStorageSink( , persistent_table_components(persistent_table_components_) , data_lake_settings(configuration_->getDataLakeSettings()) , write_format(configuration_->format) + , secondary_storages(std::move(secondary_storages_)) { auto [last_version, metadata_path, compression_method] = getLatestOrExplicitMetadataFileAndVersion( object_storage, @@ -1040,7 +1045,7 @@ bool IcebergStorageSink::initializeMetadata() try { generateManifestList( - persistent_table_components.path_resolver, metadata, object_storage, context, manifest_entries, new_snapshot, manifest_entry_sizes, *buffer_manifest_list, Iceberg::FileContentType::DATA, + persistent_table_components.path_resolver, metadata, object_storage, *secondary_storages, context, manifest_entries, new_snapshot, manifest_entry_sizes, *buffer_manifest_list, Iceberg::FileContentType::DATA, /* use_previous_snapshots = */ true); buffer_manifest_list->finalize(); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h index f25c77baef8d..0a68f1e9ed88 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h @@ -42,6 +42,8 @@ namespace DB { +struct SecondaryStorages; + String removeEscapedSlashes(const String & json_str); void generateManifestFile( @@ -66,6 +68,7 @@ void generateManifestList( const Iceberg::IcebergPathResolver & path_resolver, Poco::JSON::Object::Ptr metadata, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, ContextPtr context, const std::vector & manifest_entry_names, Poco::JSON::Object::Ptr new_snapshot, @@ -85,7 +88,8 @@ class IcebergStorageSink final : public SinkToStorage ContextPtr context_, std::shared_ptr catalog_, const Iceberg::PersistentTableComponents & persistent_table_components_, - const StorageID & table_id_); + const StorageID & table_id_, + std::shared_ptr secondary_storages_); ~IcebergStorageSink() override; @@ -129,6 +133,7 @@ class IcebergStorageSink final : public SinkToStorage Iceberg::PersistentTableComponents persistent_table_components; const DataLakeStorageSettings & data_lake_settings; const String write_format; + std::shared_ptr secondary_storages; }; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp index d21cf0de238c..0834a0e5f4e9 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp index 8a04b5c83b3e..d55d65af34e3 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp @@ -351,6 +351,7 @@ static std::optional writeDataFiles( static bool writeMetadataFiles( DataFileWriteResultWithStats & delete_filenames, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, ContextPtr context, FileNamesGenerator & filename_generator, const Iceberg::IcebergPathResolver & path_resolver, @@ -497,6 +498,7 @@ static bool writeMetadataFiles( path_resolver, metadata, object_storage, + secondary_storages, context, manifest_entries, new_snapshot, @@ -564,6 +566,7 @@ void mutate( StorageMetadataPtr storage_metadata, StorageID storage_id, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, const DataLakeStorageSettings & data_lake_settings, const PersistentTableComponents & persistent_table_components, const String & write_format, @@ -660,6 +663,7 @@ void mutate( auto result_delete_files_metadata = writeMetadataFiles( mutation_files->delete_file, object_storage, + secondary_storages, context, filename_generator, persistent_table_components.path_resolver, @@ -682,6 +686,7 @@ void mutate( auto result_data_files_metadata = writeMetadataFiles( *mutation_files->data_file, object_storage, + secondary_storages, context, filename_generator, persistent_table_components.path_resolver, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h index 1dbe55e0fb11..08d4210df67a 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h @@ -16,6 +16,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -27,6 +28,7 @@ void mutate( StorageMetadataPtr storage_metadata, StorageID storage_id, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, const DataLakeStorageSettings & data_lake_settings, const PersistentTableComponents & persistent_table_components, const String & write_format, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp index ddb913f9464b..d7a4afac7f8b 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp @@ -75,11 +75,12 @@ void IcebergPositionDeleteTransform::initializeDeleteSources() continue; } + auto [delete_storage_to_use, resolved_key] = resolveObjectStorageForPath( + path_resolver.getTableLocation(), position_deletes_object.file_path, object_storage, *secondary_storages, context, + path_resolver); - auto object_path = position_deletes_object.file_path; - auto object_metadata = object_storage->getObjectMetadata(object_path, /*with_tags=*/ false); - auto object_info = RelativePathWithMetadata{object_path, object_metadata}; - + auto object_metadata = delete_storage_to_use->getObjectMetadata(resolved_key, /*with_tags=*/ false); + RelativePathWithMetadata object_info(resolved_key, object_metadata); String format = position_deletes_object.file_format; if (boost::to_lower_copy(format) != "parquet") @@ -87,7 +88,7 @@ void IcebergPositionDeleteTransform::initializeDeleteSources() Block initial_header; { - std::unique_ptr read_buf_schema = createReadBuffer(object_info, object_storage, context, log); + std::unique_ptr read_buf_schema = createReadBuffer(object_info, delete_storage_to_use, context, log); auto schema_reader = FormatFactory::instance().getSchemaReader(format, *read_buf_schema, context); auto columns_with_names = schema_reader->readSchema(); ColumnsWithTypeAndName initial_header_data; @@ -98,9 +99,9 @@ void IcebergPositionDeleteTransform::initializeDeleteSources() initial_header = Block(initial_header_data); } - CompressionMethod compression_method = chooseCompressionMethod(object_path, "auto"); + CompressionMethod compression_method = chooseCompressionMethod(resolved_key, "auto"); - delete_read_buffers.push_back(createReadBuffer(object_info, object_storage, context, log)); + delete_read_buffers.push_back(createReadBuffer(object_info, delete_storage_to_use, context, log)); auto syntax_result = TreeRewriter(context).analyze(where_ast, initial_header.getNamesAndTypesList()); ExpressionAnalyzer analyzer(where_ast, syntax_result, context); diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h index f1738b3828b5..dcc0a5461b1f 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -28,7 +29,9 @@ class IcebergPositionDeleteTransform : public ISimpleTransform ObjectStoragePtr object_storage_, const std::optional & format_settings_, FormatParserSharedResourcesPtr parser_shared_resources_, - ContextPtr context_) + ContextPtr context_, + const IcebergPathResolver & path_resolver_, + std::shared_ptr secondary_storages_) : ISimpleTransform(header_, header_, false) , header(header_) , iceberg_object_info(iceberg_object_info_) @@ -36,6 +39,8 @@ class IcebergPositionDeleteTransform : public ISimpleTransform , format_settings(format_settings_) , context(context_) , parser_shared_resources(parser_shared_resources_) + , path_resolver(path_resolver_) + , secondary_storages(std::move(secondary_storages_)) { initializeDeleteSources(); } @@ -62,6 +67,9 @@ class IcebergPositionDeleteTransform : public ISimpleTransform ContextPtr context; FormatParserSharedResourcesPtr parser_shared_resources; + const IcebergPathResolver path_resolver; + std::shared_ptr secondary_storages; + /// We need to keep the read buffers alive since the delete_sources depends on them. std::vector> delete_read_buffers; std::vector> delete_sources; @@ -78,8 +86,10 @@ class IcebergBitmapPositionDeleteTransform final : public IcebergPositionDeleteT ObjectStoragePtr object_storage_, const std::optional & format_settings_, FormatParserSharedResourcesPtr parser_shared_resources_, - ContextPtr context_) - : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_) + ContextPtr context_, + const IcebergPathResolver & path_resolver_, + std::shared_ptr secondary_storages_) + : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_, path_resolver_, std::move(secondary_storages_)) { initialize(); } @@ -104,8 +114,10 @@ class IcebergStreamingPositionDeleteTransform final : public IcebergPositionDele ObjectStoragePtr object_storage_, const std::optional & format_settings_, FormatParserSharedResourcesPtr parser_shared_resources_, - ContextPtr context_) - : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_) + ContextPtr context_, + const IcebergPathResolver & path_resolver_, + std::shared_ptr secondary_storages_) + : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_, path_resolver_, std::move(secondary_storages_)) { initialize(); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp index b71285fbf079..365d9f2bb34c 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp @@ -256,12 +256,26 @@ RemoveOrphanFilesResult removeOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_table_components) + const PersistentTableComponents & persistent_table_components, + SecondaryStorages & secondary_storages) { auto log = getLogger("IcebergRemoveOrphanFiles"); - auto [reachable, metadata_version] = collectReachableFiles( - object_storage, persistent_table_components, data_lake_settings, context, log); + auto [reachable, metadata_version, external_files] = collectReachableFiles( + object_storage, persistent_table_components, data_lake_settings, context, log, secondary_storages); + + /// Fail closed: the scan below covers only `table_path` on the base storage. Files that resolve + /// elsewhere (secondary storage, or base storage outside `table_path`) have no bounded directory to + /// scan, so there is no safe way to reach them without risking unrelated objects that share the bucket. + if (!external_files.empty()) + throw Exception( + ErrorCodes::BAD_ARGUMENTS, + "remove_orphan_files is not supported for Iceberg tables that reference files outside the " + "table's base directory (found {} such file(s) in the metadata graph): orphan detection scans " + "and deletes only within the base directory on the base storage, so it cannot see files on " + "other storages or elsewhere in the bucket. Aborting to avoid reporting an incomplete cleanup " + "as successful.", + external_files.size()); String scan_path = resolveScanPath(persistent_table_components.table_path, params); if (!object_storage->existsOrHasAnyChild(scan_path)) @@ -277,8 +291,8 @@ RemoveOrphanFilesResult removeOrphanFiles( if (params.dry_run || scan.orphan_paths.empty()) return tallyByCategory(scan.orphan_paths, scan.skipped_missing_metadata); - auto [_recheck_files, recheck_version] = collectReachableFiles( - object_storage, persistent_table_components, data_lake_settings, context, log); + auto [_recheck_files, recheck_version, _recheck_external_files] = collectReachableFiles( + object_storage, persistent_table_components, data_lake_settings, context, log, secondary_storages); if (recheck_version != metadata_version) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Metadata version changed during orphan scan (v{} -> v{}); " @@ -306,7 +320,8 @@ Pipe executeRemoveOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_components) + const PersistentTableComponents & persistent_components, + SecondaryStorages & secondary_storages) { /// `persistent_components.format_version` is captured when the table was opened and /// can become stale if an external tool (e.g. Spark) upgrades the table v1 -> v2 @@ -370,7 +385,7 @@ Pipe executeRemoveOrphanFiles( params.location = parsed.getAs("location"); params.dry_run = parsed.getAs("dry_run") != 0; - auto result = removeOrphanFiles(params, context, object_storage, data_lake_settings, persistent_components); + auto result = removeOrphanFiles(params, context, object_storage, data_lake_settings, persistent_components, secondary_storages); return resultToPipe(result); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h index 1809d3880bad..3572bbfc2698 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -17,7 +18,8 @@ Pipe executeRemoveOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_components); + const PersistentTableComponents & persistent_components, + SecondaryStorages & secondary_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp index 572bf495340e..2f0ad966a594 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp @@ -4,6 +4,11 @@ #include +#include +#include +#include +#include + #include #include @@ -12,6 +17,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -22,7 +28,8 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + SecondaryStorages & secondary_storages) { SnapshotReferencedFiles files; @@ -36,14 +43,14 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( files.manifest_list_paths.insert(manifest_list_path); auto manifest_keys = getManifestList( - object_storage, persistent_table_components, context, manifest_list_path, log); + object_storage, persistent_table_components, context, manifest_list_path, log, secondary_storages); for (const auto & manifest_entry : manifest_keys) { files.manifest_paths.insert(manifest_entry.manifest_file_path); auto entries_handle = getManifestFileEntriesHandle( - object_storage, persistent_table_components, context, log, manifest_entry, current_schema_id); + object_storage, persistent_table_components, context, log, manifest_entry, current_schema_id, secondary_storages); for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::DATA)) files.data_file_paths.insert(entry->parsed_entry->file_path_key); @@ -60,11 +67,12 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( namespace { +using VisitPathFn = std::function; + void collectStatisticsPaths( const Poco::JSON::Object::Ptr & metadata, const char * field_name, - const IcebergPathResolver & resolver, - std::unordered_set & out) + const VisitPathFn & visit) { if (!metadata->has(field_name)) return; @@ -77,7 +85,7 @@ void collectStatisticsPaths( if (entry->has(f_statistics_path)) { String stat_path = entry->getValue(f_statistics_path); - out.insert(resolver.resolve(IcebergPathFromMetadata::deserialize(stat_path))); + visit(IcebergPathFromMetadata::deserialize(stat_path)); } } } @@ -89,12 +97,17 @@ void collectMetadataRootFiles( const String & metadata_path, const Poco::JSON::Object::Ptr & metadata, const IcebergPathResolver & resolver, + const VisitPathFn & visit, std::unordered_set & out) { + /// `metadata_path` deliberately bypasses `visit`: it is already a base-storage key produced by + /// re-resolving the latest metadata within `table_path` (see the caller), not a URI-style path + /// taken from metadata contents, so feeding it to the resolver inside `visit` would misparse it. out.insert(metadata_path); - auto version_hint = IcebergPathFromMetadata::deserialize(fmt::format("{}metadata/version-hint.text", resolver.getTableLocation())); - out.insert(resolver.resolve(version_hint)); + /// `getTableLocation` has no trailing '/' (unlike `FileNamesGenerator`, which appends one). + auto version_hint = IcebergPathFromMetadata::deserialize(fmt::format("{}/metadata/version-hint.text", resolver.getTableLocation())); + visit(version_hint); if (metadata->has(f_metadata_log)) { @@ -107,14 +120,14 @@ void collectMetadataRootFiles( if (entry->has(f_metadata_file)) { String mf_path = entry->getValue(f_metadata_file); - out.insert(resolver.resolve(IcebergPathFromMetadata::deserialize(mf_path))); + visit(IcebergPathFromMetadata::deserialize(mf_path)); } } } } - collectStatisticsPaths(metadata, f_statistics, resolver, out); - collectStatisticsPaths(metadata, f_partition_statistics, resolver, out); + collectStatisticsPaths(metadata, f_statistics, visit); + collectStatisticsPaths(metadata, f_partition_statistics, visit); } } @@ -125,7 +138,8 @@ ReachableFilesResult collectReachableFiles( const PersistentTableComponents & persistent_table_components, const DataLakeStorageSettings & data_lake_settings, ContextPtr context, - LoggerPtr log) + LoggerPtr log, + SecondaryStorages & secondary_storages) { auto [version, metadata_path, compression_method] = getLatestOrExplicitMetadataFileAndVersion( object_storage, @@ -149,40 +163,68 @@ ReachableFilesResult collectReachableFiles( persistent_table_components.table_uuid); std::unordered_set reachable; + std::vector> external_files; + std::set> seen_external; const auto & resolver = persistent_table_components.path_resolver; + /// `reachable` is matched against a base-storage listing of `table_path`, so keep only base-storage + /// keys under that prefix; everything else (secondary storage, or base storage outside `table_path`) + /// goes to `external_files`, deduped by (storage, key), for the cleanup callers to handle. The + /// callers list `table_path` with a trailing '/', so normalize the prefix once and match that. + String base_subtree_prefix = persistent_table_components.table_path; + if (!base_subtree_prefix.empty() && base_subtree_prefix.back() != '/') + base_subtree_prefix += '/'; + + /// The latest metadata JSON was re-resolved above with `ignore_explicit_metadata_file_path`, so + /// every branch of `getLatestOrExplicitMetadataFileAndVersion` (listing, table-UUID selection, + /// version-hint) yields a base-storage key under `table_path/metadata/` — never an external path. + /// `collectMetadataRootFiles` relies on this to insert it into `reachable` directly. + chassert(metadata_path.starts_with(base_subtree_prefix)); + + auto visit = [&](const IcebergPathFromMetadata & path) + { + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.table_location, path.serialize(), object_storage, secondary_storages, context, resolver); + if (storage.get() == object_storage.get() && key.starts_with(base_subtree_prefix)) + reachable.insert(std::move(key)); + else if (seen_external.emplace(storage.get(), key).second) + external_files.emplace_back(std::move(storage), std::move(key)); + }; + collectMetadataRootFiles( metadata_path, metadata, resolver, + visit, reachable); if (!metadata->has(f_snapshots)) { LOG_INFO(log, "No snapshots in metadata, reachable set contains only metadata-root files"); - return {std::move(reachable), version}; + return {std::move(reachable), version, std::move(external_files)}; } auto snapshots = metadata->get(f_snapshots).extract(); if (!snapshots || snapshots->size() == 0) { LOG_INFO(log, "Empty snapshots array, reachable set contains only metadata-root files"); - return {std::move(reachable), version}; + return {std::move(reachable), version, std::move(external_files)}; } Int32 current_schema_id = metadata->getValue(f_current_schema_id); auto snapshot_files = collectSnapshotReferencedFiles( - snapshots, object_storage, persistent_table_components, context, log, current_schema_id); + snapshots, object_storage, persistent_table_components, context, log, current_schema_id, secondary_storages); for (const auto & path : snapshot_files.manifest_list_paths) - reachable.insert(resolver.resolve(path)); + visit(path); for (const auto & path : snapshot_files.manifest_paths) - reachable.insert(resolver.resolve(path)); + visit(path); for (const auto & path : snapshot_files.data_file_paths) - reachable.insert(resolver.resolve(path)); + visit(path); - LOG_INFO(log, "Collected {} reachable files from metadata graph", reachable.size()); - return {std::move(reachable), version}; + LOG_INFO(log, "Collected {} reachable files from metadata graph ({} outside the base subtree)", + reachable.size(), external_files.size()); + return {std::move(reachable), version, std::move(external_files)}; } } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.h index a9961d8e469b..f82145c5abd7 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.h @@ -5,6 +5,8 @@ #if USE_AVRO #include +#include +#include #include #include @@ -15,6 +17,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -35,26 +38,33 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id); + Int32 current_schema_id, + SecondaryStorages & secondary_storages); struct ReachableFilesResult { + /// Base-storage keys of reachable files inside `table_path`, for matching against a base-storage listing. std::unordered_set files; Int32 metadata_version; + /// Reachable files a base-storage listing of `table_path` cannot see: on a secondary storage, or on + /// the base storage but outside `table_path`. Deduplicated and paired with their storage. + std::vector> external_files; }; -/// Collect all files reachable through the metadata graph. +/// Collect all files reachable through the current metadata graph. /// /// Traverses: metadata JSON files (from metadata-log), manifest lists (from snapshots), /// manifest files (from manifest lists), data/delete files (from manifest files), -/// and statistics files. All returned paths are resolved storage paths. -/// Also returns the metadata version used, for TOCTOU detection. +/// and statistics files. Base-storage files inside `table_path` go to `files` (as keys); everything +/// else goes to `external_files` (as resolved (storage, key) pairs). Also returns the metadata version +/// used, for TOCTOU detection. ReachableFilesResult collectReachableFiles( ObjectStoragePtr object_storage, const PersistentTableComponents & persistent_table_components, const DataLakeStorageSettings & data_lake_settings, ContextPtr context, - LoggerPtr log); + LoggerPtr log, + SecondaryStorages & secondary_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp index 045470229dc9..d6ed24a6d0f8 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp @@ -70,7 +70,8 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( ContextPtr local_context, LoggerPtr log, const IcebergPathFromMetadata & filename, - size_t bytes_size) + size_t bytes_size, + SecondaryStorages & secondary_storages) { auto log_level = local_context->getSettingsRef()[Setting::iceberg_metadata_log_level].value; @@ -79,15 +80,19 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( auto create_fn = [&, use_iceberg_metadata_cache]() { - RelativePathWithMetadata manifest_object_info(persistent_table_components.path_resolver.resolve(filename)); + auto [storage_to_use, resolved_key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, filename.serialize(), object_storage, secondary_storages, local_context, + persistent_table_components.path_resolver); + + RelativePathWithMetadata manifest_object_info(resolved_key_in_storage); auto read_settings = local_context->getReadSettings(); /// Do not utilize filesystem cache if more precise cache enabled if (use_iceberg_metadata_cache) read_settings.enable_filesystem_cache = false; - auto buffer = createReadBuffer(manifest_object_info, object_storage, local_context, log, read_settings); - auto manifest_file_deserializer = std::make_unique( + auto buffer = createReadBuffer(manifest_object_info, storage_to_use, local_context, log, read_settings); + auto manifest_file_deserializer = std::make_shared( std::move(buffer), filename, getFormatSettings(local_context)); return Iceberg::ManifestFileCacheableInfo{std::move(manifest_file_deserializer), bytes_size}; @@ -108,7 +113,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH ContextPtr local_context, LoggerPtr log, const ManifestFileCacheKey & cache_key, - Int32 table_snapshot_schema_id) + Int32 table_snapshot_schema_id, + SecondaryStorages & secondary_storages) { auto cacheable_info = getManifestFile( object_storage, @@ -116,7 +122,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH local_context, log, cache_key.manifest_file_path, - static_cast(cache_key.manifest_file_byte_size)); + cache_key.manifest_file_byte_size, + secondary_storages); auto iterator = Iceberg::ManifestFileIterator::create( cacheable_info.deserializer, @@ -141,7 +148,8 @@ ManifestFileCacheKeys getManifestList( const PersistentTableComponents & persistent_table_components, ContextPtr local_context, const IcebergPathFromMetadata & filename, - LoggerPtr log) + LoggerPtr log, + SecondaryStorages & secondary_storages) { IcebergMetadataLogLevel log_level = local_context->getSettingsRef()[Setting::iceberg_metadata_log_level].value; @@ -150,14 +158,18 @@ ManifestFileCacheKeys getManifestList( auto create_fn = [&, use_iceberg_metadata_cache]() { - RelativePathWithMetadata object_info(persistent_table_components.path_resolver.resolve(filename)); + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, filename.serialize(), object_storage, secondary_storages, local_context, + persistent_table_components.path_resolver); + + RelativePathWithMetadata object_info(key_in_storage); auto read_settings = local_context->getReadSettings(); /// Do not utilize filesystem cache if more precise cache enabled if (use_iceberg_metadata_cache) read_settings.enable_filesystem_cache = false; - auto manifest_list_buf = createReadBuffer(object_info, object_storage, local_context, log, read_settings); + auto manifest_list_buf = createReadBuffer(object_info, storage_to_use, local_context, log, read_settings); AvroForIcebergDeserializer manifest_list_deserializer(std::move(manifest_list_buf), filename, getFormatSettings(local_context)); /// The manifest list's own Avro metadata governs how it is parsed. A table whose @@ -172,7 +184,7 @@ ManifestFileCacheKeys getManifestList( local_context, manifest_list_deserializer.getMetadataContent(), DB::IcebergMetadataLogLevel::ManifestListMetadata, - persistent_table_components.path_resolver.getTableRoot(), + persistent_table_components.table_path, filename, std::nullopt, std::nullopt); @@ -209,13 +221,13 @@ ManifestFileCacheKeys getManifestList( manifest_list_deserializer.getValueFromRowByName(i, f_content, TypeIndex::Int32).safeGet()); } manifest_file_cache_keys.emplace_back( - manifest_file_name, manifest_length, added_sequence_number, added_snapshot_id.safeGet(), content_type); + manifest_file_name, static_cast(manifest_length), added_sequence_number, added_snapshot_id.safeGet(), content_type); insertRowToLogTable( local_context, manifest_list_deserializer.getContent(i), DB::IcebergMetadataLogLevel::ManifestListEntry, - persistent_table_components.path_resolver.getTableRoot(), + persistent_table_components.table_path, filename, i, std::nullopt); diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h index 2bb2edfdff13..a3e57a7a302b 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h @@ -16,6 +16,7 @@ #include #include +#include namespace DB::Iceberg { @@ -26,7 +27,8 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( ContextPtr local_context, LoggerPtr log, const IcebergPathFromMetadata & filename, - size_t bytes_size); + size_t bytes_size, + SecondaryStorages & secondary_storages); /// Creates a fully initialized ManifestFileIterator from a cache key. /// All entries are drained so that aggregate methods (e.g. getRowsCountInAllFilesExcludingDeleted) @@ -37,7 +39,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH ContextPtr local_context, LoggerPtr log, const ManifestFileCacheKey & cache_key, - Int32 table_snapshot_schema_id); + Int32 table_snapshot_schema_id, + SecondaryStorages & secondary_storages); ManifestFileCacheKeys getManifestList( @@ -45,7 +48,8 @@ ManifestFileCacheKeys getManifestList( const PersistentTableComponents & persistent_table_components, ContextPtr local_context, const IcebergPathFromMetadata & filename, - LoggerPtr log); + LoggerPtr log, + SecondaryStorages & secondary_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp index f7a3f164ac1d..5bba27100f25 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp @@ -40,12 +40,13 @@ #include #include #include +#include #if USE_AVRO #include -#include #include +#include #include #include #include @@ -104,7 +105,6 @@ static constexpr size_t MAX_LIST_RETRIES = 5; namespace DB::Iceberg { - using namespace DB; /// Best-effort heuristic based on ClickHouse naming conventions. @@ -1454,3 +1454,29 @@ void forEachAvroEntry( } #endif + +namespace DB +{ + +ObjectStoragePtr getResolvedStorageFromObjectInfo([[maybe_unused]] const ObjectInfoPtr & object_info, const ObjectStoragePtr & default_storage) +{ +#if USE_AVRO + if (auto iceberg_info = std::dynamic_pointer_cast(object_info)) + { + if (auto resolved = iceberg_info->getResolvedStorage()) + return resolved; + } +#endif + return default_storage; +} + +std::optional getMetadataPathFromObjectInfo([[maybe_unused]] const ObjectInfoPtr & object_info) +{ +#if USE_AVRO + if (auto iceberg_info = std::dynamic_pointer_cast(object_info)) + return iceberg_info->getMetadataPath(); +#endif + return std::nullopt; +} + +} diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h index 43d2c040ad59..cbbd3fa86294 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h @@ -1,9 +1,8 @@ #pragma once +#include #include "config.h" - -#if USE_AVRO - +#include #include #include #include @@ -16,6 +15,19 @@ #include #include + +namespace DB +{ +struct ObjectInfo; +using ObjectInfoPtr = std::shared_ptr; + +/// These functions are always available; they return fallback values when USE_AVRO is not defined +ObjectStoragePtr getResolvedStorageFromObjectInfo([[maybe_unused]] const ObjectInfoPtr & object_info, const ObjectStoragePtr & default_storage); +std::optional getMetadataPathFromObjectInfo([[maybe_unused]] const ObjectInfoPtr & object_info); +} + +#if USE_AVRO + #include #include #include diff --git a/src/Storages/ObjectStorage/IObjectIterator.cpp b/src/Storages/ObjectStorage/IObjectIterator.cpp index 7ae3d1cbb1b2..6b6cb90c0123 100644 --- a/src/Storages/ObjectStorage/IObjectIterator.cpp +++ b/src/Storages/ObjectStorage/IObjectIterator.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -81,6 +82,12 @@ ObjectInfoPtr ObjectIteratorWithPathAndFileFilter::next(size_t id) path = path.substr(1); path = std::filesystem::path(object_namespace) / path; + /// Iceberg exposes the raw metadata path (an absolute URI possibly pointing outside + /// the table location) as `_path`, so the pushdown filter must evaluate the same + /// value, otherwise a `_path` predicate would wrongly discard external files. + if (auto metadata_path = getMetadataPathFromObjectInfo(object)) + path = *metadata_path; + VirtualColumnUtils::filterByPathOrFile( keys, std::vector{path}, filter_actions, virtual_columns, hive_partition_columns, getContext()); @@ -165,24 +172,28 @@ ObjectInfoPtr ObjectIteratorSplitByBuckets::next(size_t id) } } - auto buffer = createReadBuffer(last_object_info->relative_path_with_metadata, object_storage, getContext(), log); + /// An Iceberg external file may live in a different storage than the base one. + auto storage_to_use = getResolvedStorageFromObjectInfo(last_object_info, object_storage); + auto buffer = createReadBuffer(last_object_info->relative_path_with_metadata, storage_to_use, getContext(), log); size_t bucket_size = getContext()->getSettingsRef()[Setting::cluster_table_function_buckets_batch_size]; auto file_bucket_infos = splitter->splitToBuckets(bucket_size, *buffer, format_settings); for (const auto & file_bucket : file_bucket_infos) { - auto copy_object_info = *last_object_info; + /// Clone polymorphically: a plain `ObjectInfo` copy would slice an + /// `IcebergDataObjectInfo` and lose its resolved storage and metadata path. + auto copy_object_info = last_object_info->clone(); if (has_cache_entry) { auto filtered = file_bucket->filterByMatchingRowGroups(matching_row_groups); if (!filtered) continue; - copy_object_info.file_bucket_info = std::move(filtered); + copy_object_info->file_bucket_info = std::move(filtered); } else { - copy_object_info.file_bucket_info = file_bucket; + copy_object_info->file_bucket_info = file_bucket; } - pending_objects_info.push(std::make_shared(copy_object_info)); + pending_objects_info.push(std::move(copy_object_info)); } } } @@ -194,7 +205,12 @@ ObjectInfoPtr ObjectIteratorSplitByBuckets::next(size_t id) String ObjectInfo::getIdentifier() const { - String result = getPath(); + return getIdentifierForPath(getPath()); +} + +String ObjectInfo::getIdentifierForPath(const String & path) const +{ + String result = path; if (file_bucket_info) result += file_bucket_info->getIdentifier(); return result; diff --git a/src/Storages/ObjectStorage/IObjectIterator.h b/src/Storages/ObjectStorage/IObjectIterator.h index 47febf269ee4..ced59e8fdd25 100644 --- a/src/Storages/ObjectStorage/IObjectIterator.h +++ b/src/Storages/ObjectStorage/IObjectIterator.h @@ -53,7 +53,12 @@ struct ObjectInfo FileBucketInfoPtr file_bucket_info; + /// Polymorphic copy: preserves the dynamic type (e.g. `IcebergDataObjectInfo` with its + /// resolved storage and metadata path) where a plain copy construction would slice it. + virtual std::shared_ptr clone() const { return std::make_shared(*this); } + String getIdentifier() const; + String getIdentifierForPath(const String & path) const; }; using ObjectInfoPtr = std::shared_ptr; diff --git a/src/Storages/ObjectStorage/ReadBufferIterator.cpp b/src/Storages/ObjectStorage/ReadBufferIterator.cpp index 3531a28e9a78..e24beecac3fe 100644 --- a/src/Storages/ObjectStorage/ReadBufferIterator.cpp +++ b/src/Storages/ObjectStorage/ReadBufferIterator.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -43,9 +44,9 @@ ReadBufferIterator::ReadBufferIterator( format = configuration->format; } -SchemaCache::Key ReadBufferIterator::getKeyForSchemaCache(const ObjectInfo & object_info, const String & format_name) const +SchemaCache::Key ReadBufferIterator::getKeyForSchemaCache(const ObjectInfoPtr & object_info, const String & format_name) const { - auto source = StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, object_info); + auto source = StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, object_info, object_storage); return DB::getKeyForSchemaCache(source, format_name, format_settings, getContext()); } @@ -57,7 +58,7 @@ SchemaCache::Keys ReadBufferIterator::getKeysForSchemaCache() const read_keys.begin(), read_keys.end(), std::back_inserter(sources), - [&](const auto & elem) { return StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, *elem); }); + [&](const auto & elem) { return StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, elem, object_storage); }); return DB::getKeysForSchemaCache(sources, *format, format_settings, getContext()); } @@ -76,7 +77,8 @@ std::optional ReadBufferIterator::tryGetColumnsFromCache( const auto & path = object_info->isArchive() ? object_info->getPathToArchive() : object_info->getPath(); if (!object_info->getObjectMetadata()) { - auto meta = object_storage->tryGetObjectMetadata(path, /*with_tags=*/ false); + auto storage_to_use = getResolvedStorageFromObjectInfo(object_info, object_storage); + auto meta = storage_to_use->tryGetObjectMetadata(path, /*with_tags=*/ false); if (meta) object_info->setObjectMetadata(*meta); } @@ -87,7 +89,7 @@ std::optional ReadBufferIterator::tryGetColumnsFromCache( if (format) { - const auto cache_key = getKeyForSchemaCache(*object_info, *format); + const auto cache_key = getKeyForSchemaCache(object_info, *format); if (auto columns = schema_cache.tryGetColumns(cache_key, get_last_mod_time)) return columns; } @@ -98,7 +100,7 @@ std::optional ReadBufferIterator::tryGetColumnsFromCache( /// If we have such entry for some format, we can use this format to read the file. for (const auto & format_name : FormatFactory::instance().getAllInputFormats()) { - const auto cache_key = getKeyForSchemaCache(*object_info, format_name); + const auto cache_key = getKeyForSchemaCache(object_info, format_name); if (auto columns = schema_cache.tryGetColumns(cache_key, get_last_mod_time)) { /// Now format is known. It should be the same for all files. @@ -114,13 +116,13 @@ std::optional ReadBufferIterator::tryGetColumnsFromCache( void ReadBufferIterator::setNumRowsToLastFile(size_t num_rows) { if (query_settings.schema_inference_use_cache) - schema_cache.addNumRows(getKeyForSchemaCache(*current_object_info, *format), num_rows); + schema_cache.addNumRows(getKeyForSchemaCache(current_object_info, *format), num_rows); } void ReadBufferIterator::setSchemaToLastFile(const ColumnsDescription & columns) { if (query_settings.schema_inference_use_cache) - schema_cache.addColumns(getKeyForSchemaCache(*current_object_info, *format), columns); + schema_cache.addColumns(getKeyForSchemaCache(current_object_info, *format), columns); } void ReadBufferIterator::setFormatName(const String & format_name) @@ -140,8 +142,11 @@ std::unique_ptr ReadBufferIterator::recreateLastReadBuffer() auto context = getContext(); const auto & path = current_object_info->isArchive() ? current_object_info->getPathToArchive() : current_object_info->getPath(); - auto impl - = createReadBuffer(current_object_info->relative_path_with_metadata, object_storage, context, getLogger("ReadBufferIterator")); + auto impl = createReadBuffer( + current_object_info->relative_path_with_metadata, + getResolvedStorageFromObjectInfo(current_object_info, object_storage), + context, + getLogger("ReadBufferIterator")); const auto compression_method = chooseCompressionMethod(current_object_info->getFileName(), configuration->compression_method); const auto zstd_window = static_cast(context->getSettingsRef()[Setting::zstd_window_log_max]); @@ -268,7 +273,10 @@ ReadBufferIterator::Data ReadBufferIterator::next() { compression_method = chooseCompressionMethod(filename, configuration->compression_method); read_buf = createReadBuffer( - current_object_info->relative_path_with_metadata, object_storage, getContext(), getLogger("ReadBufferIterator")); + current_object_info->relative_path_with_metadata, + getResolvedStorageFromObjectInfo(current_object_info, object_storage), + getContext(), + getLogger("ReadBufferIterator")); } if (!query_settings.skip_empty_files || !read_buf->eof()) diff --git a/src/Storages/ObjectStorage/ReadBufferIterator.h b/src/Storages/ObjectStorage/ReadBufferIterator.h index af1c28f43760..a14d609752b7 100644 --- a/src/Storages/ObjectStorage/ReadBufferIterator.h +++ b/src/Storages/ObjectStorage/ReadBufferIterator.h @@ -38,7 +38,7 @@ class ReadBufferIterator : public IReadBufferIterator, WithContext std::unique_ptr recreateLastReadBuffer() override; private: - SchemaCache::Key getKeyForSchemaCache(const ObjectInfo & object_info, const String & format_name) const; + SchemaCache::Key getKeyForSchemaCache(const ObjectInfoPtr & object_info, const String & format_name) const; SchemaCache::Keys getKeysForSchemaCache() const; std::optional tryGetColumnsFromCache( const ObjectInfos::iterator & begin, const ObjectInfos::iterator & end); diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp index d004243b3131..2ba28196e215 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #if ENABLE_DISTRIBUTED_CACHE #include #include @@ -179,6 +180,29 @@ std::string StorageObjectStorageSource::getUniqueStoragePathIdentifier( return fs::path(configuration.getNamespace()) / path; } +std::string StorageObjectStorageSource::getUniqueStoragePathIdentifier( + const StorageObjectStorageConfiguration & configuration, + const ObjectInfoPtr & object_info, + const ObjectStoragePtr & object_storage, + bool include_connection_info) +{ + /// Files outside the table location are read from a resolved (secondary) storage; the same + /// path may exist in different storages, so identify such files by the resolved storage. + auto resolved_storage = getResolvedStorageFromObjectInfo(object_info, object_storage); + if (resolved_storage != object_storage) + { + auto path = object_info->getPath(); + if (path.starts_with("/")) + path = path.substr(1); + + if (include_connection_info) + return fs::path(resolved_storage->getDescription()) / resolved_storage->getObjectsNamespace() / path; + return fs::path(resolved_storage->getObjectsNamespace()) / path; + } + + return getUniqueStoragePathIdentifier(configuration, *object_info, include_connection_info); +} + std::shared_ptr StorageObjectStorageSource::createFileIterator( StorageObjectStorageConfigurationPtr configuration, const StorageObjectStorageQuerySettings & query_settings, @@ -202,11 +226,17 @@ std::shared_ptr StorageObjectStorageSource::createFileIterator( { const bool expect_whole_archive = !local_context->getSettingsRef()[Setting::cluster_function_process_archive_on_multiple_nodes]; + /// Use the full table location URI (e.g. `s3a://bucket/prefix/table/`) when available + std::string table_location = configuration->getPathForRead().path; + if (auto * metadata = configuration->getExternalMetadata()) + table_location = metadata->getTableLocation(); + auto distributed_iterator = std::make_unique( local_context->getClusterFunctionReadTaskCallback(), local_context->getSettingsRef()[Setting::max_threads], /*is_archive_=*/is_archive && !expect_whole_archive, object_storage, + table_location, local_context); if (is_archive && expect_whole_archive) @@ -427,6 +457,8 @@ Chunk StorageObjectStorageSource::generate() read_context); } + std::string path_for_virtual_column = getMetadataPathFromObjectInfo(object_info).value_or(path); + const String * iceberg_metadata_file_path = nullptr; #if USE_AVRO if (const auto * iceberg_info = dynamic_cast(object_info.get())) @@ -437,7 +469,7 @@ Chunk StorageObjectStorageSource::generate() chunk, read_from_format_info.requested_virtual_columns, { - .path = path, + .path = path_for_virtual_column, .storage_id = storage_snapshot->storage.getStorageID(), .size = object_info->isArchive() ? object_info->fileSizeInArchive() : object_metadata->size_bytes, .filename = &filename, @@ -586,7 +618,7 @@ Chunk StorageObjectStorageSource::generate() if (reader.getInputFormat() && read_context->getSettingsRef()[Setting::use_cache_for_count_from_files] && !format_filter_info->filter_actions_dag) - addNumRowsToCache(*reader.getObjectInfo(), total_rows_in_file); + addNumRowsToCache(reader.getObjectInfo(), total_rows_in_file); total_rows_in_file = 0; @@ -607,11 +639,11 @@ Chunk StorageObjectStorageSource::generate() return {}; } -void StorageObjectStorageSource::addNumRowsToCache(const ObjectInfo & object_info, size_t num_rows) +void StorageObjectStorageSource::addNumRowsToCache(const ObjectInfoPtr & object_info, size_t num_rows) { const auto cache_key = getKeyForSchemaCache( - getUniqueStoragePathIdentifier(*configuration, object_info), - object_info.getFileFormat().value_or(configuration->format), + getUniqueStoragePathIdentifier(*configuration, object_info, object_storage), + object_info->getFileFormat().value_or(configuration->format), format_settings, read_context); schema_cache.addNumRows(cache_key, num_rows); @@ -670,16 +702,18 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade bool with_tags = read_from_format_info.requested_virtual_columns.contains("_tags"); const auto & path = object_info->isArchive() ? object_info->getPathToArchive() : object_info->getPath(); + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(object_info, object_storage); + if (query_settings.ignore_non_existent_file) { - auto metadata = object_storage->tryGetObjectMetadata(path, with_tags); + auto metadata = storage_to_use->tryGetObjectMetadata(path, with_tags); if (!metadata) return {}; object_info->setObjectMetadata(metadata.value()); } else - object_info->setObjectMetadata(object_storage->getObjectMetadata(path, with_tags)); + object_info->setObjectMetadata(storage_to_use->getObjectMetadata(path, with_tags)); } if (query_settings.skip_empty_files && object_info->getObjectMetadata()->size_bytes == 0 @@ -734,7 +768,7 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade return std::nullopt; const auto cache_key = getKeyForSchemaCache( - getUniqueStoragePathIdentifier(*configuration, *object_info), + getUniqueStoragePathIdentifier(*configuration, object_info, object_storage), object_info->getFileFormat().value_or(configuration->format), format_settings, context_); @@ -788,7 +822,11 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade { ProfileEvents::increment(ProfileEvents::ObjectStorageReadObjects); compression_method = chooseCompressionMethod(object_info->getFileName(), configuration->compression_method); - read_buf = createReadBuffer(object_info->relative_path_with_metadata, object_storage, context_, log); + read_buf = createReadBuffer( + object_info->relative_path_with_metadata, + getResolvedStorageFromObjectInfo(object_info, object_storage), + context_, + log); } Block initial_header = read_from_format_info.format_header; @@ -1567,11 +1605,13 @@ StorageObjectStorageSource::ReadTaskIterator::ReadTaskIterator( size_t max_threads_count, bool is_archive_, ObjectStoragePtr object_storage_, + const std::string & table_location_, ContextPtr context_) : WithContext(context_) , callback(callback_) , is_archive(is_archive_) , object_storage(object_storage_) + , table_location(table_location_) { ThreadPool pool( CurrentMetrics::StorageObjectStorageThreads, @@ -1597,8 +1637,35 @@ StorageObjectStorageSource::ReadTaskIterator::ReadTaskIterator( { auto object = object_future.get(); if (object) + { + resolveIcebergObjectStorageIfNeeded(object); buffer.push_back(object); + } + } +} + +void StorageObjectStorageSource::ReadTaskIterator::resolveIcebergObjectStorageIfNeeded([[maybe_unused]] const ObjectInfoPtr & object) +{ +#if USE_AVRO + /// For Iceberg objects, resolve the storage from the raw metadata path + auto iceberg_info = std::dynamic_pointer_cast(object); + if (!iceberg_info || iceberg_info->getResolvedStorage()) + return; + + auto metadata_path = iceberg_info->getMetadataPath(); + if (!metadata_path) + return; + + /// Only secondary-storage files need resolving here (an ObjectStorage can't be shipped over the + /// wire); base-storage files keep the coordinator's key. + if (auto resolved = tryResolveObjectStorageForPath( + table_location, *metadata_path, object_storage, secondary_storages, getContext()); + resolved && resolved->first != object_storage) + { + iceberg_info->setResolvedStorage(resolved->first); + iceberg_info->relative_path_with_metadata.relative_path = resolved->second; } +#endif } ObjectInfoPtr StorageObjectStorageSource::ReadTaskIterator::next(size_t) @@ -1615,7 +1682,9 @@ ObjectInfoPtr StorageObjectStorageSource::ReadTaskIterator::next(size_t) if (!task || task->isEmpty()) return nullptr; + object_info = task->getObjectInfo(); + resolveIcebergObjectStorageIfNeeded(object_info); } else { @@ -1710,7 +1779,10 @@ StorageObjectStorageSource::ArchiveIterator::createArchiveReader(ObjectInfoPtr o /* path_to_archive */ object_info->getPath(), /* archive_read_function */ [=, this]() - { return createReadBuffer(object_info->relative_path_with_metadata, object_storage, getContext(), log); }, + { + auto storage = getResolvedStorageFromObjectInfo(object_info, object_storage); + return createReadBuffer(object_info->relative_path_with_metadata, storage, getContext(), log); + }, /* archive_size */ size); } @@ -1732,7 +1804,10 @@ ObjectInfoPtr StorageObjectStorageSource::ArchiveIterator::next(size_t processor } if (!archive_object->getObjectMetadata()) - archive_object->setObjectMetadata(object_storage->getObjectMetadata(archive_object->getPath(), /*with_tags=*/ false)); + { + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(archive_object, object_storage); + archive_object->setObjectMetadata(storage_to_use->getObjectMetadata(archive_object->getPath(), /*with_tags=*/ false)); + } archive_reader = createArchiveReader(archive_object); file_enumerator = archive_reader->firstFile(); @@ -1758,7 +1833,10 @@ ObjectInfoPtr StorageObjectStorageSource::ArchiveIterator::next(size_t processor return {}; if (!archive_object->getObjectMetadata()) - archive_object->setObjectMetadata(object_storage->getObjectMetadata(archive_object->getPath(), /*with_tags=*/ false)); + { + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(archive_object, object_storage); + archive_object->setObjectMetadata(storage_to_use->getObjectMetadata(archive_object->getPath(), /*with_tags=*/ false)); + } archive_reader = createArchiveReader(archive_object); if (!archive_reader->fileExists(path_in_archive)) diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSource.h b/src/Storages/ObjectStorage/StorageObjectStorageSource.h index 3e72bbd15b08..ce34580c195b 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSource.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageSource.h @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include namespace DB @@ -71,6 +73,14 @@ class StorageObjectStorageSource final : public ISource static std::string getUniqueStoragePathIdentifier( const StorageObjectStorageConfiguration & configuration, const ObjectInfo & object_info, bool include_connection_info = true); + /// Same as above, but objects read from a resolved (secondary) storage are identified + /// by that storage. Use this overload for schema/num-rows cache keys. + static std::string getUniqueStoragePathIdentifier( + const StorageObjectStorageConfiguration & configuration, + const ObjectInfoPtr & object_info, + const ObjectStoragePtr & object_storage, + bool include_connection_info = true); + protected: StorageID storage_id; const String name; @@ -148,7 +158,7 @@ class StorageObjectStorageSource final : public ISource std::future createReaderAsync(); - void addNumRowsToCache(const ObjectInfo & object_info, size_t num_rows); + void addNumRowsToCache(const ObjectInfoPtr & object_info, size_t num_rows); void lazyInitialize(); }; @@ -160,6 +170,7 @@ class StorageObjectStorageSource::ReadTaskIterator : public IObjectIterator, pri size_t max_threads_count, bool is_archive_, ObjectStoragePtr object_storage_, + const std::string & table_location_, ContextPtr context_); ObjectInfoPtr next(size_t) override; @@ -169,11 +180,19 @@ class StorageObjectStorageSource::ReadTaskIterator : public IObjectIterator, pri private: ObjectInfoPtr createObjectInfoInArchive(const std::string & path_to_archive, const std::string & path_in_archive); + /// For Iceberg objects: resolve which storage the file lives in (possibly a secondary storage) + /// from the raw metadata path and record it on the object. No-op for non-Iceberg objects. + void resolveIcebergObjectStorageIfNeeded(const ObjectInfoPtr & object); + ClusterFunctionReadTaskCallback callback; ObjectInfos buffer; std::atomic_size_t index = 0; bool is_archive; ObjectStoragePtr object_storage; + std::string table_location; +#if USE_AVRO + SecondaryStorages secondary_storages; /// For Iceberg: cache of storages for external file locations +#endif /// path_to_archive -> archive reader. std::unordered_map> archive_readers; std::mutex archive_readers_mutex; diff --git a/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp b/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp index 5a54601b171c..4b7a0a3c194a 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -11,6 +13,24 @@ namespace ErrorCodes extern const int LOGICAL_ERROR; } +namespace +{ + +String getSchedulingIdentifier(const ObjectInfoPtr & object_info, bool send_over_whole_archive) +{ + if (send_over_whole_archive && object_info->isArchive()) + return object_info->getIdentifierForPath(object_info->getPathToArchive()); + + /// For Iceberg objects addressed by an external (absolute) path, schedule by that metadata path + /// so the same physical file maps to a stable replica regardless of the coordinator's key. + if (auto metadata_path = getMetadataPathFromObjectInfo(object_info)) + return object_info->getIdentifierForPath(*metadata_path); + + return object_info->getIdentifier(); +} + +} + StorageObjectStorageStableTaskDistributor::StorageObjectStorageStableTaskDistributor( std::shared_ptr iterator_, std::vector && ids_of_nodes_, @@ -135,7 +155,7 @@ ObjectInfoPtr StorageObjectStorageStableTaskDistributor::getMatchingFileFromIter } else { - file_identifier = object_info->getIdentifier(); + file_identifier = getSchedulingIdentifier(object_info, send_over_whole_archive); } size_t file_replica_idx = getReplicaForFile(file_identifier); diff --git a/src/Storages/ObjectStorage/Utils.cpp b/src/Storages/ObjectStorage/Utils.cpp index 8fa02319c9ac..9433db516362 100644 --- a/src/Storages/ObjectStorage/Utils.cpp +++ b/src/Storages/ObjectStorage/Utils.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -18,6 +19,26 @@ #include #include #include +#include +#include +#include +#include +#include +#include +#include +#if USE_AWS_S3 +#include +#endif +#if USE_AVRO +#include +#endif +#if USE_AZURE_BLOB_STORAGE +#include +#endif +#if USE_HDFS +#include +#endif + namespace DB { @@ -27,6 +48,173 @@ namespace ErrorCodes extern const int BAD_ARGUMENTS; extern const int LOGICAL_ERROR; extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; + extern const int PATH_ACCESS_DENIED; +} + +namespace +{ + +#if USE_AVRO +std::string normalizeScheme(const std::string & scheme) +{ + auto scheme_lowercase = Poco::toLower(scheme); + + if (scheme_lowercase == "s3a" || scheme_lowercase == "s3n" || scheme_lowercase == "gs" || scheme_lowercase == "gcs" || scheme_lowercase == "oss") + scheme_lowercase = "s3"; + else if (scheme_lowercase == "wasb" || scheme_lowercase == "wasbs" || scheme_lowercase == "abfss") + scheme_lowercase = "abfs"; + + return scheme_lowercase; +} + +std::string factoryTypeForScheme(const std::string & normalized_scheme) +{ + if (normalized_scheme == "s3") return "s3"; + if (normalized_scheme == "abfs") return "azure"; + if (normalized_scheme == "hdfs") return "hdfs"; + if (normalized_scheme == "file") return "local"; + return ""; +} + +#if USE_AWS_S3 +/// For s3:// URIs (generic), bucket needs to match. +/// For explicit http(s):// URIs, both bucket and endpoint must match. +bool s3URIMatches(const S3::URI & target_uri, const std::string & base_bucket, const std::string & base_endpoint, const std::string & target_scheme_normalized) +{ + bool bucket_matches = (target_uri.bucket == base_bucket); + bool endpoint_matches = (target_uri.endpoint == base_endpoint); + bool is_generic_s3_uri = (target_scheme_normalized == "s3"); + return bucket_matches && (endpoint_matches || is_generic_s3_uri); +} + +bool sameEndpoint(const std::string & a, const std::string & b) +{ + SchemeAuthorityKey pa(a); + SchemeAuthorityKey pb(b); + if (pa.authority.empty() || pb.authority.empty()) + return false; + return pa.scheme == pb.scheme && pa.authority == pb.authority; +} +#endif + +#if USE_AZURE_BLOB_STORAGE +/// Storage account in an Azure service URL: the first host label for a `*.core.*` endpoint +/// (`acc.blob.core.windows.net` -> `acc`), otherwise the first path segment (Azurite `host:port/acc` -> `acc`). +std::string azureAccountFromServiceUrl(const std::string & url) +{ + auto scheme_end = url.find("://"); + if (scheme_end == std::string::npos) + return ""; + auto host_begin = scheme_end + 3; + auto path_begin = url.find('/', host_begin); + std::string host = url.substr(host_begin, path_begin == std::string::npos ? std::string::npos : path_begin - host_begin); + + if (host.find(".core.") != std::string::npos) + return Poco::toLower(host.substr(0, host.find('.'))); + + if (path_begin == std::string::npos) + return ""; + auto seg_end = url.find('/', path_begin + 1); + return Poco::toLower(url.substr(path_begin + 1, seg_end == std::string::npos ? std::string::npos : seg_end - path_begin - 1)); +} +#endif + +std::pair getOrCreateStorageAndKey( + const std::string & cache_key, + const std::string & key_to_use, + const std::string & storage_type, + SecondaryStorages & secondary_storages, + const ContextPtr & context, + std::function configure_fn, + const std::string & supersedes_prefix = {}) +{ + std::lock_guard lock(secondary_storages.mutex); + if (auto it = secondary_storages.storages.find(cache_key); it != secondary_storages.storages.end()) + return {it->second, key_to_use}; + + Poco::AutoPtr cfg(new Poco::Util::MapConfiguration); + const std::string config_prefix = "object_storages." + cache_key; + + cfg->setString(config_prefix + ".object_storage_type", storage_type); + + configure_fn(*cfg, config_prefix); + + /// Create under lock to avoid duplicate creation and wasted work + ObjectStoragePtr storage = ObjectStorageFactory::instance().create(cache_key, *cfg, config_prefix, context, /*skip_access_check*/ true); + + /// Drop entries this one supersedes (same endpoint/bucket, older credential generation), so the + /// cache holds at most one storage per logical target instead of growing once per rotation. In-flight + /// readers keep their own `shared_ptr`, so an evicted storage stays alive until they are done with it. + if (!supersedes_prefix.empty()) + std::erase_if(secondary_storages.storages, [&](const auto & entry) { return entry.first.starts_with(supersedes_prefix); }); + + secondary_storages.storages.emplace(cache_key, storage); + return {storage, key_to_use}; +} + +/// A path is absolute if `SchemeAuthorityKey` assigns it a scheme (`scheme://...` +/// or the RFC 8089 `scheme:/path` form such as `file:/var/...`) or a key rooted at +/// '/'. Reuse the same parser so this predicate cannot drift from the resolution +/// logic in `tryResolveObjectStorageForPath`, which decomposes the path the same way. +bool isAbsolutePath(const std::string & path) +{ + if (path.empty()) + return false; + + SchemeAuthorityKey decomposed{path}; + return !decomposed.scheme.empty() || decomposed.key.starts_with('/'); +} + +#endif // USE_AVRO + +} + +SchemeAuthorityKey::SchemeAuthorityKey(const std::string & uri) +{ + if (uri.empty()) + return; + + if (auto scheme_sep = uri.find("://"); scheme_sep != std::string_view::npos) + { + scheme = Poco::toLower(uri.substr(0, scheme_sep)); + auto rest = uri.substr(scheme_sep + 3); // skip :// + + // authority is up to next '/' + auto slash = rest.find('/'); + if (slash == std::string_view::npos) + { + /// Bad URI: missing path component after authority. + /// Exception will be thrown when looking up non-existing object in the storage, so we can just return here. + authority = std::string(rest); + key = "/"; + return; + } + authority = std::string(rest.substr(0, slash)); + /// For file:// URIs, the path is absolute, so we need to keep the leading '/' + /// e.g. file:///home/user/data -> scheme="file", authority="", key="/home/user/data" + if (scheme == "file") + key = std::string(rest.substr(slash)); + else + key = std::string(rest.substr(++slash)); + return; + } + + /// Check for scheme:/path (common for file: https://datatracker.ietf.org/doc/html/rfc8089#appendix-B) + if (auto colon = uri.find(':'); colon != std::string_view::npos && colon > 0) + { + auto after_colon = uri.substr(colon + 1); + + if (!after_colon.empty() && after_colon[0] == '/') + { + scheme = Poco::toLower(uri.substr(0, colon)); + authority = ""; // No authority + key = std::string(after_colon); + return; + } + } + + // Relative path (paths starting with '/' without a scheme are now handled by the caller) + key = std::string(uri); } namespace DataLakeStorageSetting @@ -348,5 +536,442 @@ extern const SettingsUInt64 max_download_buffer_size; extern const SettingsBool use_cache_for_count_from_files; extern const SettingsString filesystem_cache_name; extern const SettingsUInt64 filesystem_cache_boundary_alignment; +extern const SettingsBool object_storage_propagate_credentials_to_other_storages; } + +#if USE_AVRO +/// Resolve an absolute metadata path directly to its (object storage, key) by parsing the URI. +/// The storage may be `base_storage` or a secondary one. Returns std::nullopt for paths that must +/// instead go through `path_resolver`: relative paths and bare local-fs absolute base paths. +std::optional> tryResolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + SecondaryStorages & secondary_storages, + const DB::ContextPtr & context) +{ + if (!isAbsolutePath(path)) + return std::nullopt; // Relative path always belongs to base storage + + auto ensure_local_path_inside_user_files = [&](const std::string & local_path) + { + /// clickhouse-local does not restrict local paths (its `user_files_path` is intentionally empty). + if (context->getApplicationType() != Context::ApplicationType::SERVER) + return; + + const auto target_path = std::filesystem::path(local_path).lexically_normal(); + const auto user_files_path = std::filesystem::path(context->getUserFilesPath()).lexically_normal(); + + if (user_files_path.empty() || !fileOrSymlinkPathStartsWith(target_path.string(), user_files_path.string())) + throw DB::Exception( + DB::ErrorCodes::PATH_ACCESS_DENIED, + "File URI '{}' is outside of allowed `user_files` path '{}'", + local_path, + user_files_path.string()); + }; + + SchemeAuthorityKey table_location_decomposed{table_location}; + SchemeAuthorityKey target_decomposed{path}; + + if (target_decomposed.scheme.empty() && target_decomposed.key.starts_with('/')) + { + if (base_storage->getType() == ObjectStorageType::Local) + ensure_local_path_inside_user_files(target_decomposed.key); + + return std::nullopt; + } + + const std::string base_scheme_normalized = normalizeScheme(table_location_decomposed.scheme); + const std::string target_scheme_normalized = normalizeScheme(target_decomposed.scheme); + + /// `file://` paths must stay inside `user_files`. + /// Without this check, metadata could drive reads from arbitrary local paths. + if (target_scheme_normalized == "file") + { + ensure_local_path_inside_user_files(target_decomposed.key); + } + + // For S3 URIs, use S3::URI to properly handle all kinds of URIs, e.g. https://s3.amazonaws.com/bucket/... == s3://bucket/... + #if USE_AWS_S3 + if (target_scheme_normalized == "s3" || target_scheme_normalized == "https" || target_scheme_normalized == "http") + { + std::string normalized_path = path; + if (target_decomposed.scheme == "s3a" || target_decomposed.scheme == "s3n" || target_decomposed.scheme == "oss") + { + normalized_path = "s3://" + target_decomposed.authority + "/" + target_decomposed.key; + } + else if (target_decomposed.scheme == "gcs") + { + normalized_path = "gs://" + target_decomposed.authority + "/" + target_decomposed.key; + } + /// Paths from metadata already have correct encoding; disable Poco::URI + /// percent-decoding so that keys like `col=12%3A00%3A00` are preserved as-is. + S3::URI s3_uri(normalized_path, /*allow_archive_path_syntax*/ false, + /*keep_presigned_query_parameters*/ true, /*uri_style*/ S3UriStyle::AUTO, + /*enable_url_encoding*/ false); + + std::string key_to_use = s3_uri.key; + + bool use_base_storage = false; + if (base_storage->getType() == ObjectStorageType::S3) + { + if (auto s3_storage = std::dynamic_pointer_cast(base_storage)) + { + const std::string base_bucket = s3_storage->getObjectsNamespace(); + const std::string base_endpoint = s3_storage->getDescription(); + + if (s3URIMatches(s3_uri, base_bucket, base_endpoint, target_scheme_normalized)) + use_base_storage = true; + } + } + + if (!use_base_storage && (base_scheme_normalized == "s3" || base_scheme_normalized == "https" || base_scheme_normalized == "http")) + { + std::string normalized_table_location = table_location; + if (table_location_decomposed.scheme == "s3a" || table_location_decomposed.scheme == "s3n" || table_location_decomposed.scheme == "oss") + { + normalized_table_location = "s3://" + table_location_decomposed.authority + "/" + table_location_decomposed.key; + } + else if (table_location_decomposed.scheme == "gcs") + { + normalized_table_location = "gs://" + table_location_decomposed.authority + "/" + table_location_decomposed.key; + } + S3::URI base_s3_uri(normalized_table_location, /*allow_archive_path_syntax*/ false, + /*keep_presigned_query_parameters*/ true, /*uri_style*/ S3UriStyle::AUTO, + /*enable_url_encoding*/ false); + + /// The path matches the table's `location` but not the base storage, so its raw key + /// is not valid there: return nullopt to remap it through `IcebergPathResolver`. + if (s3URIMatches(s3_uri, base_s3_uri.bucket, base_s3_uri.endpoint, target_scheme_normalized)) + return std::nullopt; + } + + if (use_base_storage) + return std::make_pair(base_storage, key_to_use); + + /// Construct the endpoint for this storage, then build the cache key from it. + /// A generic `s3://bucket/...` inherits one from the base storage. + const bool endpoint_explicit = (target_decomposed.scheme == "http" || target_decomposed.scheme == "https"); + + std::string endpoint_to_use; + + /// Build an endpoint that keeps the bucket: `ObjectStorageFactory` re-parses the endpoint + /// from the config to determine the bucket, and reads use keys relative to the bucket, so + /// an endpoint without the bucket would address the wrong one (see `getS3URI`). + auto make_endpoint_with_bucket = [&]() -> std::string + { + if (s3_uri.endpoint.empty()) + return "https://" + s3_uri.bucket + ".s3.amazonaws.com"; + + const auto scheme_end = s3_uri.endpoint.find("://"); + if (s3_uri.is_virtual_hosted_style && scheme_end != std::string::npos) + { + /// Virtual-hosted style: https://s3.region.amazonaws.com -> https://bucket.s3.region.amazonaws.com + return s3_uri.endpoint.substr(0, scheme_end + 3) + s3_uri.bucket + "." + s3_uri.endpoint.substr(scheme_end + 3); + } + + /// Path style: http://minio:9000 -> http://minio:9000/bucket + return s3_uri.endpoint + "/" + s3_uri.bucket; + }; + + if (endpoint_explicit) + { + endpoint_to_use = make_endpoint_with_bucket(); + } + else + { + std::string base_endpoint; + if (base_storage->getType() == ObjectStorageType::S3) + base_endpoint = base_storage->getDescription(); + + if (!base_endpoint.empty()) + { + if (base_endpoint.find(".s3.") != std::string::npos && base_endpoint.find(".amazonaws.com") != std::string::npos) + { + /// AWS-style: https://oldbucket.s3.us-east-1.amazonaws.com -> https://newbucket.s3.us-east-1.amazonaws.com + size_t s3_pos = base_endpoint.find(".s3."); + size_t scheme_end = base_endpoint.find("://"); + if (scheme_end != std::string::npos) + { + std::string scheme = base_endpoint.substr(0, scheme_end + 3); + std::string suffix = base_endpoint.substr(s3_pos); + + /// Trim path after endpoint + size_t slash_pos = suffix.find('/', 1); + if (slash_pos != std::string::npos) + suffix = suffix.substr(0, slash_pos); + endpoint_to_use = scheme + s3_uri.bucket + suffix; + } + } + else + { + /// Path-style (e.g. minio): http://host:port/oldbucket -> http://host:port/newbucket + size_t scheme_end = base_endpoint.find("://"); + if (scheme_end != std::string::npos) + { + size_t path_start = base_endpoint.find('/', scheme_end + 3); + if (path_start != std::string::npos) + base_endpoint = base_endpoint.substr(0, path_start); + } + if (!base_endpoint.empty() && base_endpoint.back() == '/') + base_endpoint.pop_back(); + endpoint_to_use = base_endpoint + "/" + s3_uri.bucket; + } + } + + /// Fallback: base storage is not S3 + if (endpoint_to_use.empty()) + endpoint_to_use = make_endpoint_with_bucket(); + } + + const bool propagate_creds = context->getSettingsRef()[Setting::object_storage_propagate_credentials_to_other_storages]; + + /// Decide whether the base storage's S3 credentials apply to this target + bool reuse_base_credentials = false; + if (base_storage->getType() == ObjectStorageType::S3) + reuse_base_credentials = propagate_creds || !endpoint_explicit + || sameEndpoint(base_storage->getDescription(), s3_uri.endpoint); + + String access_key_id; + String secret_access_key; + String session_token; + String region; + if (reuse_base_credentials) + { + if (auto s3_storage = std::dynamic_pointer_cast(base_storage)) + { + if (auto s3_client = s3_storage->tryGetS3StorageClient()) + { + const auto credentials = s3_client->getCredentials(); + access_key_id = credentials.GetAWSAccessKeyId(); + secret_access_key = credentials.GetAWSSecretKey(); + session_token = credentials.GetSessionToken(); + region = s3_client->getRegion(); + } + } + } + + /// `configure_fn` runs only on a cache miss, so every input that shapes the created storage must + /// be part of the cache key. Include the credential-propagation flag and, when credentials are + /// propagated, a fingerprint of that generation: when the base storage rotates its (temporary) + /// credentials or a different user queries the table, the fingerprint changes and a fresh secondary + /// storage is built, instead of a cache hit silently reusing an expired or foreign token. + std::string storage_cache_key = "s3://" + s3_uri.bucket + "@" + endpoint_to_use + + "#propagate=" + (propagate_creds ? "1" : "0"); + + /// When credentials are propagated, the older generations of the same target become unreachable + /// once the fingerprint changes; pass their common prefix so they are evicted instead of leaking. + std::string supersedes_prefix; + if (!access_key_id.empty() || !session_token.empty()) + { + supersedes_prefix = storage_cache_key + "#cred="; + + SipHash creds_hash; + creds_hash.update(access_key_id); + creds_hash.update(secret_access_key); + creds_hash.update(session_token); + storage_cache_key = supersedes_prefix + std::to_string(creds_hash.get64()); + } + + return getOrCreateStorageAndKey( + storage_cache_key, + key_to_use, + "s3", + secondary_storages, + context, + [&](Poco::Util::MapConfiguration & cfg, const std::string & config_prefix) + { + cfg.setString(config_prefix + ".endpoint", endpoint_to_use); + + /// Apply the credentials captured above (the exact generation the cache key fingerprints). + if (!access_key_id.empty()) + cfg.setString(config_prefix + ".access_key_id", access_key_id); + if (!secret_access_key.empty()) + cfg.setString(config_prefix + ".secret_access_key", secret_access_key); + if (!session_token.empty()) + cfg.setString(config_prefix + ".session_token", session_token); + if (!region.empty()) + cfg.setString(config_prefix + ".region", region); + }, + supersedes_prefix); + } + #endif + + #if USE_HDFS + if (target_scheme_normalized == "hdfs") + { + bool use_base_storage = false; + + // Check if base_storage matches (only if it's HDFS) + if (base_storage->getType() == ObjectStorageType::HDFS) + { + if (auto hdfs_storage = std::dynamic_pointer_cast(base_storage)) + { + const std::string base_url = hdfs_storage->getDescription(); + // Extract endpoint from base URL (hdfs://namenode:port/path -> hdfs://namenode:port) + std::string base_endpoint; + if (auto pos = base_url.find('/', base_url.find("//") + 2); pos != std::string::npos) + base_endpoint = base_url.substr(0, pos); + else + base_endpoint = base_url; + + // For HDFS, compare endpoints (namenode addresses) + std::string target_endpoint = target_scheme_normalized + "://" + target_decomposed.authority; + + if (base_endpoint == target_endpoint) + use_base_storage = true; + + // Also check if table_location matches + if (!use_base_storage && base_scheme_normalized == "hdfs") + { + if (table_location_decomposed.authority == target_decomposed.authority) + use_base_storage = true; + } + } + } + + if (use_base_storage) + return std::make_pair(base_storage, target_decomposed.key); + } + #endif + + /// Fallback for schemes not handled above (e.g., abfs, file) + if (base_scheme_normalized == target_scheme_normalized && table_location_decomposed.authority == target_decomposed.authority) + return std::make_pair(base_storage, target_decomposed.key); + + const std::string type_for_factory = factoryTypeForScheme(target_scheme_normalized); + if (type_for_factory.empty()) + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Unsupported storage scheme '{}' in path '{}'", target_scheme_normalized, path); + + /// For `file://` URIs the authority is always empty, so using just `"file://"` as the + /// cache key would cause every directory to share a single `LocalObjectStorage` instance + /// whose root (`key_prefix`) is set to the parent directory of the first file ever seen. + /// To avoid this, include the parent directory of the target file in the cache key so that + /// each directory gets its own storage instance with the correct root. + std::string file_dir_path; // only set for file:// URIs + std::string cache_key; + if (target_scheme_normalized == "file") + { + std::filesystem::path fs_path(target_decomposed.key); + file_dir_path = fs_path.parent_path().string(); + if (file_dir_path.empty() || file_dir_path == "/") + file_dir_path = "/"; + else if (file_dir_path.back() != '/') + file_dir_path += '/'; + cache_key = "file://" + file_dir_path; + } + else + { + cache_key = target_scheme_normalized + "://" + target_decomposed.authority; + } + + /// Handle storage types that need new storage creation + return getOrCreateStorageAndKey( + cache_key, + target_decomposed.key, + type_for_factory, + secondary_storages, + context, + [&](Poco::Util::MapConfiguration & cfg, const std::string & config_prefix) + { + if (target_scheme_normalized == "file") + { + cfg.setString(config_prefix + ".path", file_dir_path); + } + else if (target_scheme_normalized == "abfs") + { + std::string container_name; + std::string account_name; + const auto & authority = target_decomposed.authority; + + auto at_pos = authority.find('@'); + if (at_pos != std::string::npos) + { + container_name = authority.substr(0, at_pos); + account_name = authority.substr(at_pos + 1); + /// Remove .dfs.core.windows.net suffix if present + auto suffix_pos = account_name.find('.'); + if (suffix_pos != std::string::npos) + account_name = account_name.substr(0, suffix_pos); + } + else + container_name = authority; + + cfg.setString(config_prefix + ".container_name", container_name); + if (!account_name.empty()) + cfg.setString(config_prefix + ".account_name", account_name); + +#if USE_AZURE_BLOB_STORAGE + /// Copy credentials from base Azure storage if available + if (base_storage->getType() == ObjectStorageType::Azure) + { + if (auto azure_storage = std::dynamic_pointer_cast(base_storage)) + { + const auto & conn_params = azure_storage->getConnectionParameters(); + const auto & auth_method = azure_storage->getAzureBlobStorageAuthMethod(); + + /// The base credentials/endpoint identify the base account and cannot authenticate a + /// different one, so reject cross-account paths instead of silently serving them from + /// the base account. The base account comes from the base service URL, so this works + /// regardless of how the base was authenticated. + const std::string target_account = Poco::toLower(account_name); + const std::string base_account = azureAccountFromServiceUrl(conn_params.getConnectionURL()); + + if (!target_account.empty() && !base_account.empty() && target_account != base_account) + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "Iceberg metadata references Azure storage account '{}', which differs from the table's " + "base account '{}'. Reading across Azure accounts is not supported; configure access to " + "account '{}'.", + account_name, base_account, account_name); + + if (std::holds_alternative(auth_method)) + { + cfg.setString(config_prefix + ".connection_string", + std::get(auth_method).toUnderType()); + } + else + { + const auto & endpoint = conn_params.endpoint; + if (!endpoint.storage_account_url.empty()) + cfg.setString(config_prefix + ".storage_account_url", endpoint.storage_account_url); + if (account_name.empty() && !endpoint.account_name.empty()) + cfg.setString(config_prefix + ".account_name", endpoint.account_name); + /// The accounts are the same (checked above), so the base shared key authenticates + /// this container too. Without it `getAuthMethod` would silently fall back to + /// managed identity for the secondary storage. + if (!endpoint.account_key.empty()) + cfg.setString(config_prefix + ".account_key", endpoint.account_key); + } + } + } +#endif + } + else if (target_scheme_normalized == "hdfs") + { + // HDFS endpoint must end with '/' + auto endpoint = target_scheme_normalized + "://" + target_decomposed.authority; + if (!endpoint.empty() && endpoint.back() != '/') + endpoint.push_back('/'); + cfg.setString(config_prefix + ".endpoint", endpoint); + } + }); +} + +std::pair resolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + SecondaryStorages & secondary_storages, + const DB::ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver) +{ + if (auto resolved = tryResolveObjectStorageForPath(table_location, path, base_storage, secondary_storages, context)) + return *resolved; + /// Relative paths only: map via path_resolver (table_location -> table_root translation). + return {base_storage, path_resolver.resolve(Iceberg::IcebergPathFromMetadata::deserialize(path))}; +} + +#endif + } diff --git a/src/Storages/ObjectStorage/Utils.h b/src/Storages/ObjectStorage/Utils.h index 931ebcbed9ac..8679d80eee87 100644 --- a/src/Storages/ObjectStorage/Utils.h +++ b/src/Storages/ObjectStorage/Utils.h @@ -4,11 +4,38 @@ #include #include +#include +#include +#include + namespace DB { class IObjectStorage; +#if USE_AVRO +/// Thread-safe wrapper for secondary object storages map +/// (now only used for Iceberg) +struct SecondaryStorages +{ + mutable std::mutex mutex; + std::map storages; +}; +#endif + +// A URI split into components +// s3://bucket/a/b -> scheme="s3", authority="bucket", path="/a/b" +// file:///var/x -> scheme="file", authority="", path="/var/x" +// /abs/p -> scheme="", authority="", path="/abs/p" +struct SchemeAuthorityKey +{ + explicit SchemeAuthorityKey(const std::string & uri); + + std::string scheme; + std::string authority; + std::string key; +}; + std::optional checkAndGetNewFileOnInsertIfNeeded( const IObjectStorage & object_storage, const StorageObjectStorageConfiguration & configuration, @@ -65,6 +92,30 @@ struct ParseFromDiskResult ParseFromDiskResult parseFromDisk(ASTs args, bool with_structure, ContextPtr context, const fs::path & prefix); +#if USE_AVRO +namespace Iceberg { class IcebergPathResolver; } + +/// Resolve an absolute metadata path directly to its (object storage, key) by parsing the URI. +/// The storage may be `base_storage` or a secondary one. Returns std::nullopt for paths that must +/// instead go through `path_resolver`: relative paths and bare local-fs absolute base paths. +std::optional> tryResolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + SecondaryStorages & secondary_storages, + const DB::ContextPtr & context); + +/// Resolve a metadata path to (object storage, key) for reading. Absolute paths resolve directly via +/// `tryResolveObjectStorageForPath`; relative paths are mapped via `path_resolver`. +std::pair resolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + SecondaryStorages & secondary_storages, + const DB::ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver); +#endif + void expandPaimonKeeperMacrosIfNeeded( const StorageFactory::Arguments & args, const DataLakeStorageSettingsPtr & storage_settings); diff --git a/src/Storages/StorageURL.cpp b/src/Storages/StorageURL.cpp index 9c061ab0cf80..e57a7f6bd5e7 100644 --- a/src/Storages/StorageURL.cpp +++ b/src/Storages/StorageURL.cpp @@ -830,10 +830,10 @@ std::function IStorageURLBase::getReadPOSTDataCallback( namespace { - class ReadBufferIterator : public IReadBufferIterator, WithContext + class URLReadBufferIterator : public IReadBufferIterator, WithContext { public: - ReadBufferIterator( + URLReadBufferIterator( const std::vector & urls_to_check_, std::optional format_, const CompressionMethod & compression_method_, @@ -1063,7 +1063,7 @@ std::pair IStorageURLBase::getTableStructureAndForma else urls_to_check = {uri}; - ReadBufferIterator read_buffer_iterator(urls_to_check, format, compression_method, headers, format_settings, context); + URLReadBufferIterator read_buffer_iterator(urls_to_check, format, compression_method, headers, format_settings, context); if (format) return {readSchemaFromFormat(*format, format_settings, read_buffer_iterator, context), *format}; return detectFormatAndReadSchema(format_settings, read_buffer_iterator, context); diff --git a/tests/integration/test_storage_iceberg_multistorage/__init__.py b/tests/integration/test_storage_iceberg_multistorage/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/integration/test_storage_iceberg_multistorage/configs/config.d/cluster.xml b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/cluster.xml new file mode 100644 index 000000000000..54c08b27abe8 --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/cluster.xml @@ -0,0 +1,20 @@ + + + + + + node1 + 9000 + + + node2 + 9000 + + + node3 + 9000 + + + + + diff --git a/tests/integration/test_storage_iceberg_multistorage/configs/config.d/named_collections.xml b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/named_collections.xml new file mode 100644 index 000000000000..516e4ba63a3a --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/named_collections.xml @@ -0,0 +1,15 @@ + + + + http://minio1:9001/root/ + minio + ClickHouse_Minio_P@ssw0rd + + + devstoreaccount1 + Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== + + + + + diff --git a/tests/integration/test_storage_iceberg_multistorage/configs/config.d/query_log.xml b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/query_log.xml new file mode 100644 index 000000000000..a63e91f41fbc --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/configs/config.d/query_log.xml @@ -0,0 +1,6 @@ + + + system + query_log
+ + diff --git a/tests/integration/test_storage_iceberg_multistorage/configs/users.d/users.xml b/tests/integration/test_storage_iceberg_multistorage/configs/users.d/users.xml new file mode 100644 index 000000000000..1455f61d5257 --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/configs/users.d/users.xml @@ -0,0 +1,17 @@ + + + + + 1 + + + + + + default + 1 + + + diff --git a/tests/integration/test_storage_iceberg_multistorage/test.py b/tests/integration/test_storage_iceberg_multistorage/test.py new file mode 100644 index 000000000000..874c15a310dc --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/test.py @@ -0,0 +1,1034 @@ +import pytest +import pyspark +import os +import shutil +import tempfile +import time +import json +import avro.datafile +import avro.io + +from helpers.cluster import ClickHouseCluster +from helpers.s3_tools import ( + LocalUploader, + S3Uploader, + AzureUploader, + LocalDownloader, + S3Downloader, + prepare_s3_bucket, +) +from helpers.iceberg_utils import ( + get_uuid_str, + default_upload_directory, + default_download_directory, +) + +def get_spark(): + builder = ( + pyspark.sql.SparkSession.builder.appName("test_storage_iceberg_multistorage") + .config( + "spark.sql.catalog.spark_catalog", + "org.apache.iceberg.spark.SparkSessionCatalog", + ) + .config("spark.sql.catalog.local", "org.apache.iceberg.spark.SparkCatalog") + .config("spark.sql.catalog.spark_catalog.type", "hadoop") + .config("spark.sql.catalog.spark_catalog.warehouse", "/var/lib/clickhouse/user_files/iceberg_data") + .config( + "spark.sql.extensions", + "org.apache.iceberg.spark.extensions.IcebergSparkSessionExtensions", + ) + .master("local") + ) + return builder.getOrCreate() + + +@pytest.fixture(scope="package") +def started_cluster(): + try: + cluster = ClickHouseCluster(__file__, with_spark=True) + cluster.add_instance( + "node1", + main_configs=[ + "configs/config.d/query_log.xml", + "configs/config.d/cluster.xml", + "configs/config.d/named_collections.xml", + ], + user_configs=["configs/users.d/users.xml"], + with_minio=True, + with_azurite=True, + stay_alive=True, + ) + + cluster.start() + + prepare_s3_bucket(cluster) + + cluster.spark_session = get_spark() + + cluster.default_s3_uploader = S3Uploader(cluster.minio_client, cluster.minio_bucket) + cluster.default_s3_downloader = S3Downloader(cluster.minio_client, cluster.minio_bucket) + + cluster.azure_container_name = "mycontainer" + cluster.blob_service_client.create_container(cluster.azure_container_name) + cluster.default_azure_uploader = AzureUploader(cluster.blob_service_client, cluster.azure_container_name) + + cluster.default_local_uploader = LocalUploader(cluster.instances["node1"]) + cluster.default_local_downloader = LocalDownloader(cluster.instances["node1"]) + + # Create extra S3 buckets for test_four_different_locations + for i in range(1, 4): + bucket_name = f"{cluster.minio_bucket}-storage{i}" + if not cluster.minio_client.bucket_exists(bucket_name): + cluster.minio_client.make_bucket(bucket_name) + + yield cluster + + finally: + cluster.shutdown() + + +def modify_avro_file(avro_path: str, field_path: list, modifier_func) -> None: + """ + Modify a field in an AVRO file, preserving the rest of it as is. + + field_path: list of keys to navigate to the field + modifier_func: function that takes old value and returns new value + """ + with open(avro_path, 'rb') as f: + reader = avro.datafile.DataFileReader(f, avro.io.DatumReader()) + schema = reader.datum_reader.writers_schema + # Preserve all file metadata (partition-spec, format-version, etc.) + metadata = dict(reader.meta) + records = list(reader) + reader.close() + + for record in records: + obj = record + for key in field_path[:-1]: + if obj is None or key not in obj: + break + obj = obj[key] + else: + if obj and field_path[-1] in obj: + obj[field_path[-1]] = modifier_func(obj[field_path[-1]]) + + with open(avro_path, 'wb') as f: + writer = avro.datafile.DataFileWriter(f, avro.io.DatumWriter(), schema) + for key, value in metadata.items(): + if not key.startswith('avro.'): + writer.set_meta(key, value) + for record in records: + writer.append(record) + writer.close() + + +def get_absolute_path(storage_type: str, cluster, relative_path: str) -> str: + """Convert relative path to absolute path for given storage type.""" + relative_path = relative_path.lstrip("/") + + if storage_type == "s3": + return f"s3a://{cluster.minio_bucket}/{relative_path}" + elif storage_type.startswith("s3:"): # s3:bucket_name format + bucket = storage_type.split(":")[1] + return f"s3a://{bucket}/{relative_path}" + elif storage_type.startswith("url:"): # url:bucket_name format - explicit http://endpoint/bucket/... URL + bucket = storage_type.split(":")[1] + return f"http://{cluster.minio_host}:{cluster.minio_port}/{bucket}/{relative_path}" + elif storage_type == "azure": + return f"abfs://{cluster.azure_container_name}@{cluster.azurite_account}/{relative_path}" + elif storage_type.startswith("azure:"): # azure:container_name format + container = storage_type.split(":")[1] + return f"abfs://{container}@{cluster.azurite_account}/{relative_path}" + elif storage_type == "local": + return f"file:///{relative_path}" + else: + raise ValueError(f"Unknown storage type: {storage_type}") + + +def get_uploader(storage_type: str, cluster): + if storage_type == "s3": + return cluster.default_s3_uploader + elif storage_type.startswith("s3:") or storage_type.startswith("url:"): + bucket = storage_type.split(":")[1] + return S3Uploader(cluster.minio_client, bucket) + elif storage_type == "azure": + return cluster.default_azure_uploader + elif storage_type.startswith("azure:"): + container = storage_type.split(":")[1] + return AzureUploader(cluster.blob_service_client, container) + elif storage_type == "local": + return cluster.default_local_uploader + else: + raise ValueError(f"Unknown storage type: {storage_type}") + + +def get_table_function(metadata_storage: str): + if metadata_storage == "s3" or metadata_storage.startswith("s3:"): + return "icebergS3" + elif metadata_storage == "azure" or metadata_storage.startswith("azure:"): + return "icebergAzure" + elif metadata_storage == "local": + return "icebergLocal" + else: + raise ValueError(f"Unknown storage type: {metadata_storage}") + + +def get_query_args(metadata_storage: str, cluster, table_path: str): + """Get query arguments for the iceberg table function.""" + minio_url = f"http://{cluster.minio_host}:{cluster.minio_port}" + if metadata_storage == "s3": + return f"s3, filename='{table_path}/', format=Parquet, url='{minio_url}/{cluster.minio_bucket}/'" + elif metadata_storage.startswith("s3:"): + bucket = metadata_storage.split(":")[1] + return f"s3, filename='{table_path}/', format=Parquet, url='{minio_url}/{bucket}/'" + elif metadata_storage == "azure": + return f"azure, container='{cluster.azure_container_name}', storage_account_url='{cluster.env_variables['AZURITE_STORAGE_ACCOUNT_URL']}', blob_path='{table_path}/', format=Parquet" + elif metadata_storage.startswith("azure:"): + container = metadata_storage.split(":")[1] + return f"azure, container='{container}', storage_account_url='{cluster.env_variables['AZURITE_STORAGE_ACCOUNT_URL']}', blob_path='{table_path}/', format=Parquet" + elif metadata_storage == "local": + return f"local, path='/{table_path}', format=Parquet" + else: + raise ValueError(f"Unknown storage type: {metadata_storage}") + + +def find_files(directory: str, suffix: str) -> list: + """Find files ending with given suffix.""" + result = [] + for root, _, files in os.walk(directory): + for f in files: + if f.endswith(suffix): + result.append(os.path.join(root, f)) + return result + + +def path_modifier(old_path: str, new_storage: str, cluster, base_path: str): + """Create a new absolute path for a different storage location.""" + # Extract just the filename/relative portion + if "://" in old_path: + # Parse out the path part after protocol://bucket/ + parts = old_path.split("/") + # Find where the actual path starts (after bucket) + for i, part in enumerate(parts): + if base_path.split("/")[0] in part or "var" in part: + relative = "/".join(parts[i:]) + break + else: + relative = parts[-1] + else: + relative = old_path.lstrip("/") + + return get_absolute_path(new_storage, cluster, relative) + + +# ============================================================================= +# Tests +# ============================================================================= + +STORAGE_TYPES = ["s3", "azure", "local"] + +def _get_type_family(t): + if t.startswith("s3"): + return "s3" + elif t.startswith("azure"): + return "azure" + return t + +def _generate_valid_combinations(): + """ + Generate valid storage combinations. + Rule: all components must be same type family as metadata, OR local. + Local doesn't need credentials, so S3+local and Azure+local work. + But S3+Azure doesn't work (credentials aren't interchangeable). + """ + combinations = [] + for metadata in STORAGE_TYPES: + main_family = _get_type_family(metadata) + for manifest_list in STORAGE_TYPES: + if _get_type_family(manifest_list) not in (main_family, "local"): + continue + for manifest in STORAGE_TYPES: + if _get_type_family(manifest) not in (main_family, "local"): + continue + for data in STORAGE_TYPES: + if _get_type_family(data) not in (main_family, "local"): + continue + combinations.append((metadata, manifest_list, manifest, data)) + return combinations + +VALID_COMBINATIONS = _generate_valid_combinations() + +@pytest.mark.parametrize("metadata_storage,manifest_list_storage,manifest_storage,data_storage", VALID_COMBINATIONS) +def test_multi_storage_combinations(started_cluster, metadata_storage, manifest_list_storage, manifest_storage, data_storage): + """ + Test Iceberg table with all components in different storage locations. + """ + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_combo_{get_uuid_str()}" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + # Upload to default S3 first + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + # Download all files + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, TABLE_NAME) + os.makedirs(host_path, exist_ok=True) + + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}/", host_path) + + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}" + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + # Step 1: Modify manifest files to point to data_storage + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], + lambda p: path_modifier(p, data_storage, started_cluster, base_path)) + + # Step 2: Modify manifest-list files to point to manifest_storage + manifest_list_files = [f for f in find_files(metadata_dir, ".avro") if os.path.basename(f).startswith("snap-")] + for ml in manifest_list_files: + modify_avro_file(ml, ["manifest_path"], + lambda p: path_modifier(p, manifest_storage, started_cluster, base_path)) + + # Step 3: Modify metadata.json to point to manifest_list_storage + for mj in find_files(metadata_dir, ".metadata.json"): + with open(mj, 'r') as f: + data = json.load(f) + + data["location"] = get_absolute_path(metadata_storage, started_cluster, base_path) + + # Update snapshot manifest-list paths + if "snapshots" in data: + for snap in data["snapshots"]: + if "manifest-list" in snap: + snap["manifest-list"] = path_modifier(snap["manifest-list"], manifest_list_storage, started_cluster, base_path) + + with open(mj, 'w') as f: + json.dump(data, f, indent=2) + + # Step 4: Upload to respective storages + # Metadata files (*.metadata.json, version-hint.text) + meta_uploader = get_uploader(metadata_storage, started_cluster) + for f in find_files(metadata_dir, ".metadata.json") + find_files(metadata_dir, "version-hint.text"): + rel = os.path.relpath(f, host_path) + meta_uploader.upload_file(f, f"{base_path}/{rel}") + + # Manifest-list files + ml_uploader = get_uploader(manifest_list_storage, started_cluster) + for f in manifest_list_files: + rel = os.path.relpath(f, host_path) + ml_uploader.upload_file(f, f"{base_path}/{rel}") + + # Manifest files + m_uploader = get_uploader(manifest_storage, started_cluster) + for f in manifest_files: + rel = os.path.relpath(f, host_path) + m_uploader.upload_file(f, f"{base_path}/{rel}") + + # Data files + d_uploader = get_uploader(data_storage, started_cluster) + if os.path.exists(data_dir): + for f in find_files(data_dir, ".parquet"): + rel = os.path.relpath(f, host_path) + d_uploader.upload_file(f, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + + func = get_table_function(metadata_storage) + args = get_query_args(metadata_storage, started_cluster, base_path) + + assert instance.query(f"SELECT * FROM {func}({args}) ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + + +# S3 is the primary use case for cross-bucket access. +# Azure cross-container: not supported (account_key not extractable from credential object). +def test_four_different_s3_buckets(started_cluster): + """S3: each component in a different bucket (metadata, manifest-list, manifest, data).""" + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_four_buckets_{get_uuid_str()}" + buckets = [ + started_cluster.minio_bucket, + f"{started_cluster.minio_bucket}-storage1", + f"{started_cluster.minio_bucket}-storage2", + f"{started_cluster.minio_bucket}-storage3", + ] + + metadata_storage = f"s3:{buckets[0]}" + manifest_list_storage = f"s3:{buckets[1]}" + manifest_storage = f"s3:{buckets[2]}" + data_storage = f"s3:{buckets[3]}" + + uploaders = {f"s3:{b}": S3Uploader(started_cluster.minio_client, b) for b in buckets} + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, name STRING, score INT) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'Alice', 100), (2, 'Bob', 85), (3, 'Carol', 92)") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, TABLE_NAME) + os.makedirs(host_path, exist_ok=True) + + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}/", host_path) + + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}" + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], + lambda p: path_modifier(p, data_storage, started_cluster, base_path)) + + manifest_list_files = [f for f in find_files(metadata_dir, ".avro") if os.path.basename(f).startswith("snap-")] + for ml in manifest_list_files: + modify_avro_file(ml, ["manifest_path"], + lambda p: path_modifier(p, manifest_storage, started_cluster, base_path)) + + for mj in find_files(metadata_dir, ".metadata.json"): + with open(mj, 'r') as f: + data = json.load(f) + data["location"] = get_absolute_path(metadata_storage, started_cluster, base_path) + if "snapshots" in data: + for snap in data["snapshots"]: + if "manifest-list" in snap: + snap["manifest-list"] = path_modifier(snap["manifest-list"], manifest_list_storage, started_cluster, base_path) + with open(mj, 'w') as f: + json.dump(data, f, indent=2) + + for f in find_files(metadata_dir, ".metadata.json") + find_files(metadata_dir, "version-hint.text"): + rel = os.path.relpath(f, host_path) + uploaders[metadata_storage].upload_file(f, f"{base_path}/{rel}") + + for f in manifest_list_files: + rel = os.path.relpath(f, host_path) + uploaders[manifest_list_storage].upload_file(f, f"{base_path}/{rel}") + + for f in manifest_files: + rel = os.path.relpath(f, host_path) + uploaders[manifest_storage].upload_file(f, f"{base_path}/{rel}") + + if os.path.exists(data_dir): + for f in find_files(data_dir, ".parquet"): + rel = os.path.relpath(f, host_path) + uploaders[data_storage].upload_file(f, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + result = instance.query(f"SELECT * FROM icebergS3(s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{buckets[0]}/') ORDER BY id") + + assert result == "1\tAlice\t100\n2\tBob\t85\n3\tCarol\t92\n" + + +# Regression test: the bucket from an explicit path-style URL must be preserved when creating +# the secondary storage; otherwise reads are issued against the wrong bucket. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3348134710 +def test_explicit_http_urls_different_buckets(started_cluster): + """S3: components referenced via explicit `http://endpoint/bucket/...` URLs in different buckets.""" + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_explicit_urls_{get_uuid_str()}" + buckets = [ + started_cluster.minio_bucket, + f"{started_cluster.minio_bucket}-storage1", + f"{started_cluster.minio_bucket}-storage2", + f"{started_cluster.minio_bucket}-storage3", + ] + + metadata_storage = f"s3:{buckets[0]}" + manifest_list_storage = f"url:{buckets[1]}" + manifest_storage = f"url:{buckets[2]}" + data_storage = f"url:{buckets[3]}" + + uploaders = { + metadata_storage: S3Uploader(started_cluster.minio_client, buckets[0]), + manifest_list_storage: S3Uploader(started_cluster.minio_client, buckets[1]), + manifest_storage: S3Uploader(started_cluster.minio_client, buckets[2]), + data_storage: S3Uploader(started_cluster.minio_client, buckets[3]), + } + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, name STRING, score INT) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'Alice', 100), (2, 'Bob', 85), (3, 'Carol', 92)") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, TABLE_NAME) + os.makedirs(host_path, exist_ok=True) + + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}/", host_path) + + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}" + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], + lambda p: path_modifier(p, data_storage, started_cluster, base_path)) + + manifest_list_files = [f for f in find_files(metadata_dir, ".avro") if os.path.basename(f).startswith("snap-")] + for ml in manifest_list_files: + modify_avro_file(ml, ["manifest_path"], + lambda p: path_modifier(p, manifest_storage, started_cluster, base_path)) + + for mj in find_files(metadata_dir, ".metadata.json"): + with open(mj, 'r') as f: + data = json.load(f) + data["location"] = get_absolute_path(metadata_storage, started_cluster, base_path) + if "snapshots" in data: + for snap in data["snapshots"]: + if "manifest-list" in snap: + snap["manifest-list"] = path_modifier(snap["manifest-list"], manifest_list_storage, started_cluster, base_path) + with open(mj, 'w') as f: + json.dump(data, f, indent=2) + + for f in find_files(metadata_dir, ".metadata.json") + find_files(metadata_dir, "version-hint.text"): + rel = os.path.relpath(f, host_path) + uploaders[metadata_storage].upload_file(f, f"{base_path}/{rel}") + + for f in manifest_list_files: + rel = os.path.relpath(f, host_path) + uploaders[manifest_list_storage].upload_file(f, f"{base_path}/{rel}") + + for f in manifest_files: + rel = os.path.relpath(f, host_path) + uploaders[manifest_storage].upload_file(f, f"{base_path}/{rel}") + + if os.path.exists(data_dir): + for f in find_files(data_dir, ".parquet"): + rel = os.path.relpath(f, host_path) + uploaders[data_storage].upload_file(f, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + result = instance.query(f"SELECT * FROM icebergS3(s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{buckets[0]}/') ORDER BY id") + + assert result == "1\tAlice\t100\n2\tBob\t85\n3\tCarol\t92\n" + + +# Regression test: external data files in different buckets under the same object key +# used to share one num-rows cache entry, returning the wrong `count()`. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3356426404 +def test_num_rows_cache_no_collision_across_buckets(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + base_bucket = started_cluster.minio_bucket + # The same object key for both tables, each in its own bucket. + shared_key = f"shared_count_cache_{get_uuid_str()}/data/part-0.parquet" + + def prepare_table(table_name, values_sql, data_bucket): + spark.sql(f"CREATE TABLE {table_name} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {table_name} VALUES {values_sql}") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{table_name}/", f"/iceberg_data/default/{table_name}/") + + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, table_name) + os.makedirs(host_path, exist_ok=True) + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{table_name}/", host_path) + + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{table_name}" + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + data_files = find_files(data_dir, ".parquet") + assert len(data_files) == 1, f"Expected a single data file, got: {data_files}" + + # Point the data file to the same object key in a different bucket. + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], lambda _: f"s3a://{data_bucket}/{shared_key}") + # Drop the statistics so that `count()` is not answered from metadata. + modify_avro_file(mf, ["data_file", "value_counts"], lambda _: None) + + for mj in find_files(metadata_dir, ".metadata.json"): + with open(mj, 'r') as f: + data = json.load(f) + for snap in data.get("snapshots", []): + snap.get("summary", {}).pop("total-records", None) + with open(mj, 'w') as f: + json.dump(data, f, indent=2) + + for f in manifest_files + find_files(metadata_dir, ".metadata.json"): + rel = os.path.relpath(f, host_path) + started_cluster.default_s3_uploader.upload_file(f, f"{base_path}/{rel}") + + S3Uploader(started_cluster.minio_client, data_bucket).upload_file(data_files[0], shared_key) + + shutil.rmtree(temp_dir) + return base_path + + # An entry is reused only for files older than it, so upload everything before querying. + base_path_a = prepare_table( + f"test_count_cache_a_{get_uuid_str()}", "(1, 'a'), (2, 'b'), (3, 'c')", f"{base_bucket}-storage1" + ) + base_path_b = prepare_table( + f"test_count_cache_b_{get_uuid_str()}", "(1, 'a'), (2, 'b'), (3, 'c'), (4, 'd'), (5, 'e')", f"{base_bucket}-storage2" + ) + # Margin for the second-resolution `last_modified` comparison. + time.sleep(3) + + def count(base_path, marker): + result = instance.query( + f"SELECT /* {marker} */ count() FROM icebergS3(s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/') " + "SETTINGS optimize_trivial_count_query = 1, optimize_count_from_files = 1, use_cache_for_count_from_files = 1" + ).strip() + instance.query("SYSTEM FLUSH LOGS") + cache_lookups = int(instance.query( + "SELECT ProfileEvents['SchemaInferenceCacheHits'] + ProfileEvents['SchemaInferenceCacheMisses'] " + f"FROM system.query_log WHERE type = 'QueryFinish' AND query LIKE '%{marker}%' AND query NOT LIKE '%query_log%' " + "ORDER BY event_time_microseconds DESC LIMIT 1" + ).strip()) + return result, cache_lookups + + # The first query populates the num-rows cache; the second one must not reuse its entry. + count_a, cache_lookups_a = count(base_path_a, "count_cache_marker_a") + count_b, cache_lookups_b = count(base_path_b, "count_cache_marker_b") + assert count_a == "3" + assert count_b == "5" + # Both queries must actually consult the num-rows cache. + assert cache_lookups_a >= 1 + assert cache_lookups_b >= 1 + + +def _download_table_for_relocation(started_cluster, table_name): + """Download a table's on-disk files to a fresh temp dir for rewriting/relocation. Returns + (temp_dir, host_path, base_path); the caller is responsible for `shutil.rmtree(temp_dir)`.""" + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, table_name) + os.makedirs(host_path, exist_ok=True) + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{table_name}/", host_path) + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{table_name}" + return temp_dir, host_path, base_path + + +def _move_files_to_bucket(started_cluster, files, bucket, host_path, base_path): + """Upload each file to `bucket` under its table-relative path and delete the stale base-bucket copy, + so the file ends up living only on the secondary storage.""" + uploader = S3Uploader(started_cluster.minio_client, bucket) + for f in files: + rel = os.path.relpath(f, host_path) + uploader.upload_file(f, f"{base_path}/{rel}") + started_cluster.minio_client.remove_object(started_cluster.minio_bucket, f"{base_path}/{rel}") + + +def relocate_manifest_lists_to_bucket(started_cluster, table_name, manifest_list_bucket): + """Move the table's manifest lists to `manifest_list_bucket`; the stale base-bucket copies are + deleted so a read that wrongly resolves the external path against the base storage cannot succeed.""" + manifest_list_storage = f"s3:{manifest_list_bucket}" + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + metadata_dir = os.path.join(host_path, "metadata") + + manifest_list_files = [f for f in find_files(metadata_dir, ".avro") if os.path.basename(f).startswith("snap-")] + for mj in find_files(metadata_dir, ".metadata.json"): + with open(mj, 'r') as f: + data = json.load(f) + for snap in data.get("snapshots", []): + if "manifest-list" in snap: + snap["manifest-list"] = path_modifier(snap["manifest-list"], manifest_list_storage, started_cluster, base_path) + with open(mj, 'w') as f: + json.dump(data, f, indent=2) + rel = os.path.relpath(mj, host_path) + started_cluster.default_s3_uploader.upload_file(mj, f"{base_path}/{rel}") + + _move_files_to_bucket(started_cluster, manifest_list_files, manifest_list_bucket, host_path, base_path) + + shutil.rmtree(temp_dir) + return base_path + + +def _rewrite_manifests_and_reupload(started_cluster, host_path, base_path, file_path_modifier): + """Rewrite every manifest's `data_file.file_path` via `file_path_modifier` and re-upload the + manifests to the base bucket. Manifest lists and metadata.json are left untouched.""" + metadata_dir = os.path.join(host_path, "metadata") + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], file_path_modifier) + rel = os.path.relpath(mf, host_path) + started_cluster.default_s3_uploader.upload_file(mf, f"{base_path}/{rel}") + + +def relocate_data_files_to_bucket(started_cluster, table_name, data_bucket): + """Move the table's data files to `data_bucket`; manifests are rewritten to point there and the + stale base-bucket copies are deleted so the data lives only on the secondary storage.""" + data_storage = f"s3:{data_bucket}" + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + data_dir = os.path.join(host_path, "data") + + _rewrite_manifests_and_reupload(started_cluster, host_path, base_path, + lambda p: path_modifier(p, data_storage, started_cluster, base_path)) + + _move_files_to_bucket(started_cluster, find_files(data_dir, ".parquet"), data_bucket, host_path, base_path) + + shutil.rmtree(temp_dir) + return base_path + + +def relocate_data_files_within_base_bucket(started_cluster, table_name, external_prefix): + """Rewrite the table's data-file references to absolute URIs in the SAME base bucket but under + `external_prefix` (outside the table directory), and move the parquet files there. Returns `base_path`.""" + base_bucket = started_cluster.minio_bucket + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + data_dir = os.path.join(host_path, "data") + + def to_external(old_path): + filename = old_path.rstrip("/").rsplit("/", 1)[-1] + return f"s3a://{base_bucket}/{external_prefix}/{filename}" + + _rewrite_manifests_and_reupload(started_cluster, host_path, base_path, to_external) + + for f in find_files(data_dir, ".parquet"): + filename = os.path.basename(f) + started_cluster.default_s3_uploader.upload_file(f, f"{external_prefix}/{filename}") + rel = os.path.relpath(f, host_path) + started_cluster.minio_client.remove_object(base_bucket, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + return base_path + + +# Regression test: the `OPTIMIZE TABLE ... MANIFEST` threshold pre-check used to read the +# current manifest list from the base storage only and failed when it lived in another bucket. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3613986714 +@pytest.mark.skip( + reason="Manifest-only compaction (`OPTIMIZE TABLE ... MANIFEST` and the " + "`iceberg_manifest_min_count_to_compact` setting) is not part of antalya-26.6, so the " + "external-manifest-list support this test covers has nothing to exercise here. " + "Re-enable together with the manifest compaction feature." +) +def test_optimize_manifest_with_external_manifest_list(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_optimize_external_ml_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + # Three appends so the manifest list is above the compaction threshold below. + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (2, 'beta')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + base_path = relocate_manifest_lists_to_bucket(started_cluster, TABLE_NAME, f"{base_bucket}-storage1") + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + + def count_metadata_files(): + return sum( + 1 for obj in started_cluster.minio_client.list_objects(base_bucket, prefix=f"{base_path}/metadata/", recursive=True) + if obj.object_name.endswith(".json") + ) + + metadata_files_before = count_metadata_files() + + instance.query( + f"OPTIMIZE TABLE {TABLE_NAME} MANIFEST", + settings={ + "allow_experimental_iceberg_compaction": 1, + "iceberg_manifest_min_count_to_compact": 2, + }, + ) + + # The compaction must actually commit new metadata, not early-return "below threshold". + assert count_metadata_files() > metadata_files_before + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + instance.query(f"DROP TABLE {TABLE_NAME}") + + +# Regression test: `generateManifestList` used to reread the parent snapshot's manifest list from +# the base storage only, so INSERT failed when the current manifest list lived in another bucket. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3613986717 +def test_insert_with_external_manifest_list(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_insert_external_ml_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + base_path = relocate_manifest_lists_to_bucket(started_cluster, TABLE_NAME, f"{base_bucket}-storage1") + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + instance.query(f"INSERT INTO {TABLE_NAME} VALUES (3, 'gamma')", settings={"allow_insert_into_iceberg": 1}) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + instance.query(f"DROP TABLE {TABLE_NAME}") + + +# Same as `test_insert_with_external_manifest_list`, but through `ALTER TABLE ... DELETE`. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3613986717 +def test_mutation_with_external_manifest_list(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_mutation_external_ml_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + base_path = relocate_manifest_lists_to_bucket(started_cluster, TABLE_NAME, f"{base_bucket}-storage1") + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + instance.query(f"ALTER TABLE {TABLE_NAME} DELETE WHERE id = 2", settings={"allow_insert_into_iceberg": 1}) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n3\tgamma\n" + instance.query(f"DROP TABLE {TABLE_NAME}") + + +# Regression test: `_path` predicate pushdown and bucket splitting must operate on the same +# absolute path that Iceberg rows expose for external files. Before the fix, the iterator-side +# filter evaluated `namespace/key` while rows exposed the raw metadata URI, so a `_path` +# predicate silently discarded external files, and `cluster_table_function_split_granularity = +# 'bucket'` sliced the object info to a plain one, losing the resolved storage. +def test_external_path_virtual_column_filter(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_path_filter_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + data_bucket = f"{base_bucket}-storage1" + data_storage = f"s3:{data_bucket}" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + temp_dir = tempfile.mkdtemp() + host_path = os.path.join(temp_dir, TABLE_NAME) + os.makedirs(host_path, exist_ok=True) + default_download_directory(started_cluster, "s3", f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}/", host_path) + + base_path = f"var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}" + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + # Point the data files at another bucket; metadata stays in the base bucket. + manifest_files = [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")] + for mf in manifest_files: + modify_avro_file(mf, ["data_file", "file_path"], + lambda p: path_modifier(p, data_storage, started_cluster, base_path)) + + for f in manifest_files: + rel = os.path.relpath(f, host_path) + started_cluster.default_s3_uploader.upload_file(f, f"{base_path}/{rel}") + + data_uploader = S3Uploader(started_cluster.minio_client, data_bucket) + for f in find_files(data_dir, ".parquet"): + rel = os.path.relpath(f, host_path) + data_uploader.upload_file(f, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + + paths = instance.query(f"SELECT DISTINCT _path FROM icebergS3({args})").strip().splitlines() + assert len(paths) == 1 + external_path = paths[0] + # `_path` must expose the external location, not a key inside the base bucket. + assert data_bucket in external_path + + # Filtering by the very value the rows expose must select the file, not discard it. + assert instance.query( + f"SELECT count() FROM icebergS3({args}) WHERE _path = '{external_path}'" + ).strip() == "3" + + # The same through the cluster function; bucket splitting must keep the resolved storage. + assert instance.query( + f"SELECT count() FROM icebergS3Cluster(cluster_simple, {args}) WHERE _path = '{external_path}' " + "SETTINGS skip_unavailable_shards = 1, cluster_table_function_split_granularity = 'bucket'" + ).strip() == "3" + + +# Regression test: `DROP TABLE` with `iceberg_delete_data_on_drop = 1` used to delete only the base +# storage subtree, leaving data files that live in another bucket behind. `IcebergMetadata::drop` now +# also walks the current metadata graph and deletes files that resolve to a secondary storage. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3621619550 +def test_delete_data_on_drop_removes_external_files(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_drop_external_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + data_bucket = f"{base_bucket}-storage1" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + # Data files live in another bucket; metadata / manifests stay in the base bucket. + base_path = relocate_data_files_to_bucket(started_cluster, TABLE_NAME, data_bucket) + + def count_objects(bucket, prefix): + return sum(1 for _ in started_cluster.minio_client.list_objects(bucket, prefix=prefix, recursive=True)) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + + assert count_objects(data_bucket, f"{base_path}/data/") > 0 + assert count_objects(base_bucket, f"{base_path}/") > 0 + + # `SYNC` waits for the background drop (which runs `IcebergMetadata::drop`) to finish. + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + # Both the base subtree and the external data files must be gone. + assert count_objects(base_bucket, f"{base_path}/") == 0 + assert count_objects(data_bucket, f"{base_path}/data/") == 0 + + +# Regression test: `remove_orphan_files` scans and deletes only within the base storage, so it cannot +# clean orphans that live in another bucket / account. Rather than silently report a partial cleanup +# as complete, it now fails closed when the metadata graph references files outside the base storage. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3621619560 +def test_remove_orphan_files_rejects_external_paths(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_orphan_external_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + data_bucket = f"{base_bucket}-storage1" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + base_path = relocate_data_files_to_bucket(started_cluster, TABLE_NAME, data_bucket) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + # Sanity check: the data really is external and readable. + assert instance.query(f"SELECT count() FROM {TABLE_NAME}").strip() == "3" + + # `remove_orphan_files` must refuse rather than silently skip the external data files. + error = instance.query_and_get_error( + f"ALTER TABLE {TABLE_NAME} EXECUTE remove_orphan_files(older_than = '2020-01-01 00:00:00', dry_run = 1)", + settings={"allow_insert_into_iceberg": 1, "allow_iceberg_remove_orphan_files": 1}, + ) + assert "outside the table's base directory" in error + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + +# Regression test: a data file referenced by an absolute URI elsewhere in the SAME base bucket resolves +# to the base storage but outside `table_path`. It used to land in `reachable` instead of `external_files`, +# so `remove_orphan_files` did not fail closed on it. It now does. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3632505967 +def test_remove_orphan_files_rejects_same_bucket_external_paths(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_orphan_same_bucket_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + external_prefix = f"external_data/{TABLE_NAME}" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + # Data files live elsewhere in the SAME bucket; metadata / manifests stay in the table directory. + base_path = relocate_data_files_within_base_bucket(started_cluster, TABLE_NAME, external_prefix) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + # Sanity check: the data really is outside the table directory yet readable through the base storage. + assert instance.query(f"SELECT count() FROM {TABLE_NAME}").strip() == "3" + + error = instance.query_and_get_error( + f"ALTER TABLE {TABLE_NAME} EXECUTE remove_orphan_files(older_than = '2020-01-01 00:00:00', dry_run = 1)", + settings={"allow_insert_into_iceberg": 1, "allow_iceberg_remove_orphan_files": 1}, + ) + assert "outside the table's base directory" in error + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + +# Regression test: `DROP TABLE` with `iceberg_delete_data_on_drop = 1` used to leak data files that +# resolve to the base storage but live outside `table_path` (an absolute URI elsewhere in the same +# bucket): they landed in `reachable` instead of `external_files`. They are now deleted on drop. +# https://github.com/ClickHouse/ClickHouse/pull/90740#discussion_r3632505967 +def test_delete_data_on_drop_removes_same_bucket_external_files(started_cluster): + instance = started_cluster.instances["node1"] + spark = started_cluster.spark_session + + TABLE_NAME = f"test_drop_same_bucket_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + external_prefix = f"external_data/{TABLE_NAME}" + + spark.sql(f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg OPTIONS('format-version'='2')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + # Data files live elsewhere in the SAME bucket; metadata / manifests stay in the table directory. + base_path = relocate_data_files_within_base_bucket(started_cluster, TABLE_NAME, external_prefix) + + def count_objects(bucket, prefix): + return sum(1 for _ in started_cluster.minio_client.list_objects(bucket, prefix=prefix, recursive=True)) + + minio_url = f"http://{started_cluster.minio_host}:{started_cluster.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{base_bucket}/'" + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergS3({args})") + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n3\tgamma\n" + + # The data really lives outside the table directory but in the same bucket. + assert count_objects(base_bucket, f"{base_path}/") > 0 + assert count_objects(base_bucket, f"{external_prefix}/") > 0 + + # `SYNC` waits for the background drop (which runs `IcebergMetadata::drop`) to finish. + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + # Both the table directory and the same-bucket external data files must be gone. + assert count_objects(base_bucket, f"{base_path}/") == 0 + assert count_objects(base_bucket, f"{external_prefix}/") == 0 diff --git a/tests/integration/test_storage_iceberg_schema_evolution/test_array_evolved_with_struct.py b/tests/integration/test_storage_iceberg_schema_evolution/test_array_evolved_with_struct.py index 5cb1c02a0c07..9a60da2b2301 100644 --- a/tests/integration/test_storage_iceberg_schema_evolution/test_array_evolved_with_struct.py +++ b/tests/integration/test_storage_iceberg_schema_evolution/test_array_evolved_with_struct.py @@ -55,7 +55,7 @@ def execute_spark_query(query: str): execute_spark_query( f""" - INSERT INTO {TABLE_NAME} VALUES (ARRAY(named_struct('name', 'Singapore', 'zip', 12345), named_struct('name', 'Moscow', 'zip', 54321)), ARRAY(1,2)); + INSERT INTO {TABLE_NAME} VALUES (ARRAY(named_struct('city', 'Singapore', 'zip', 12345), named_struct('city', 'Moscow', 'zip', 54321)), ARRAY(1,2)); """ ) diff --git a/tests/queries/0_stateless/04214_iceberg_alter_drop_column_stale_metadata_cache_104295.sh b/tests/queries/0_stateless/04214_iceberg_alter_drop_column_stale_metadata_cache_104295.sh index 97c4555c6858..fef8c1873807 100755 --- a/tests/queries/0_stateless/04214_iceberg_alter_drop_column_stale_metadata_cache_104295.sh +++ b/tests/queries/0_stateless/04214_iceberg_alter_drop_column_stale_metadata_cache_104295.sh @@ -30,8 +30,7 @@ CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -# Isolated work dir for the `IcebergLocal` table. Passed to `clickhouse local` -# as `--user_files_path` so the `IcebergLocal` access check accepts the path. +# Isolated work dir for the `IcebergLocal` table. WORK_DIR="${CLICKHOUSE_TMP}/iceberg_alter_drop_column_104295_${CLICKHOUSE_TEST_UNIQUE_NAME}" rm -rf "${WORK_DIR}" mkdir -p "${WORK_DIR}" @@ -49,4 +48,4 @@ INSERT INTO t0 (c1, c0) VALUES (1, 1); ALTER TABLE t0 DROP COLUMN c0; INSERT INTO t0 (c1) SELECT 2; SELECT c1 FROM t0 ORDER BY c1; -" -- --user_files_path="${WORK_DIR}" +" diff --git a/tests/queries/0_stateless/data_minio/field_ids_complex_test/metadata/v1.metadata.json b/tests/queries/0_stateless/data_minio/field_ids_complex_test/metadata/v1.metadata.json index 8d367d20f041..a983881af8f0 100644 --- a/tests/queries/0_stateless/data_minio/field_ids_complex_test/metadata/v1.metadata.json +++ b/tests/queries/0_stateless/data_minio/field_ids_complex_test/metadata/v1.metadata.json @@ -1,7 +1,7 @@ { "format-version" : 2, "table-uuid" : "d4b695ca-ceeb-4537-8a2a-eee90dc6e313", - "location" : "s3a://test/field_ids_struct_test/metadata/field_ids_complex_test", + "location" : "s3a://test/field_ids_complex_test", "last-sequence-number" : 1, "last-updated-ms" : 1757661733693, "last-column-id" : 9, @@ -96,7 +96,7 @@ "total-position-deletes" : "0", "total-equality-deletes" : "0" }, - "manifest-list" : "s3a://test/field_ids_struct_test/metadata/field_ids_complex_test/metadata/snap-607752583403487091-1-140c8dff-1d83-4841-bc40-9aa85205b555.avro", + "manifest-list" : "s3a://test/field_ids_complex_test/metadata/snap-607752583403487091-1-140c8dff-1d83-4841-bc40-9aa85205b555.avro", "schema-id" : 0 } ], "statistics" : [ ], diff --git a/tests/queries/0_stateless/data_minio/field_ids_struct_test/metadata/v1.metadata.json b/tests/queries/0_stateless/data_minio/field_ids_struct_test/metadata/v1.metadata.json index 2d149abb44e7..d6c9079228ac 100644 --- a/tests/queries/0_stateless/data_minio/field_ids_struct_test/metadata/v1.metadata.json +++ b/tests/queries/0_stateless/data_minio/field_ids_struct_test/metadata/v1.metadata.json @@ -1,7 +1,7 @@ { "format-version" : 2, "table-uuid" : "149ecc15-7afc-4311-86b3-3a4c8d4ec08e", - "location" : "s3a://test/field_ids_struct_test/metadata/field_ids_struct_test", + "location" : "s3a://test/field_ids_struct_test", "last-sequence-number" : 1, "last-updated-ms" : 1753959190403, "last-column-id" : 6, @@ -84,7 +84,7 @@ "total-position-deletes" : "0", "total-equality-deletes" : "0" }, - "manifest-list" : "s3a://test/field_ids_struct_test/metadata/field_ids_struct_test/metadata/snap-2512638186869817292-1-ec467367-15a4-4610-8ea8-cf76797afb03.avro", + "manifest-list" : "s3a://test/field_ids_struct_test/metadata/snap-2512638186869817292-1-ec467367-15a4-4610-8ea8-cf76797afb03.avro", "schema-id" : 0 } ], "statistics" : [ ], diff --git a/tests/queries/0_stateless/data_minio/field_ids_table_test/metadata/v1.metadata.json b/tests/queries/0_stateless/data_minio/field_ids_table_test/metadata/v1.metadata.json index 32225eb618ad..1ddc3492cc82 100644 --- a/tests/queries/0_stateless/data_minio/field_ids_table_test/metadata/v1.metadata.json +++ b/tests/queries/0_stateless/data_minio/field_ids_table_test/metadata/v1.metadata.json @@ -1,7 +1,7 @@ { "format-version" : 2, "table-uuid" : "8f1f9ae2-18bb-421e-b640-ec2f85e67bce", - "location" : "s3a://test/field_ids_table_test/metadata/field_ids_table_test", + "location" : "s3a://test/field_ids_table_test", "last-sequence-number" : 1, "last-updated-ms" : 1752481476160, "last-column-id" : 1, @@ -56,7 +56,7 @@ "total-position-deletes" : "0", "total-equality-deletes" : "0" }, - "manifest-list" : "s3a://test/field_ids_table_test/metadata/field_ids_table_test/metadata/snap-2811410366534688344-1-3b002f99-b012-4041-9a97-db477fcc7115.avro", + "manifest-list" : "s3a://test/field_ids_table_test/metadata/snap-2811410366534688344-1-3b002f99-b012-4041-9a97-db477fcc7115.avro", "schema-id" : 0 } ], "statistics" : [ ],