diff --git a/src/Core/ProtocolDefines.h b/src/Core/ProtocolDefines.h index 4941b6179c76..135033080ab3 100644 --- a/src/Core/ProtocolDefines.h +++ b/src/Core/ProtocolDefines.h @@ -49,7 +49,8 @@ static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_FILE static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_COMPACTION = 7; static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_READ_SOURCE_INDEX = 8; static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_IDENTITY_PARTITION_COLUMNS = 9; -static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION = DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_IDENTITY_PARTITION_COLUMNS; +static constexpr auto DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH = 10; +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 dc978ae678fc..ba487bd945ef 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -691,6 +691,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 0a3ddb973f7b..a7e7a51b58a2 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -449,7 +449,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 e812f591c57a..60e305f28f4e 100644 --- a/src/Databases/DataLake/DatabaseDataLake.cpp +++ b/src/Databases/DataLake/DatabaseDataLake.cpp @@ -821,7 +821,9 @@ StoragePtr DatabaseDataLake::tryGetTableImpl(const String & name, ContextPtr con static_credentials->addCredentialsToEngineArgs(args); static_credentials_applied = true; } - 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 b671cb447531..31c2748fa590 100644 --- a/src/IO/S3/URI.cpp +++ b/src/IO/S3/URI.cpp @@ -18,10 +18,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); } }; @@ -33,7 +35,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) @@ -45,9 +47,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()) { @@ -93,7 +95,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 a8122aa94f40..7200fc600b33 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 71a4a6f956ed..f4a45da846f5 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 637091ae4aac..d919c008cdc7 100644 --- a/src/Interpreters/IcebergMetadataLog.cpp +++ b/src/Interpreters/IcebergMetadataLog.cpp @@ -107,13 +107,17 @@ void insertRowToLogTableImpl( 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 & element) { element = 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 f383336c9596..7430ebdc1b49 100644 --- a/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp @@ -195,7 +195,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE file_sequence_number = file_sequence_number_value.safeGet(); } - 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: /// { @@ -284,7 +284,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, @@ -332,7 +332,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE } return std::make_shared( FileContentType::POSITION_DELETE, - file_path_key, + file_path_from_metadata, row_index, status, sequence_number, @@ -364,7 +364,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 8347164b9a46..ebeb87fbb185 100644 --- a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h @@ -55,6 +55,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 beb714c61a23..71655e80992e 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp @@ -109,6 +109,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; @@ -150,6 +153,7 @@ static bool isCurrentManifestListAboveThreshold( Poco::JSON::Object::Ptr metadata_object, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, ContextPtr context, size_t threshold) { @@ -176,8 +180,16 @@ static bool isCurrentManifestListAboveThreshold( return false; auto filename = IcebergPathFromMetadata::deserialize(current_manifest_list_path); - RelativePathWithMetadata object_info(persistent_table_components.path_resolver.resolve(filename)); - auto manifest_list_buf = createReadBuffer(object_info, object_storage, context, log); + /// The manifest list may live outside the table's own storage, so read it from the storage its path resolves to. + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, + filename.serialize(), + object_storage, + secondary_storages, + context, + persistent_table_components.path_resolver); + RelativePathWithMetadata object_info(key_in_storage); + auto manifest_list_buf = createReadBuffer(object_info, storage_to_use, context, log); AvroForIcebergDeserializer manifest_list_deserializer( std::move(manifest_list_buf), filename, getFormatSettings(context)); return manifest_list_deserializer.rows() > threshold; @@ -188,6 +200,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) @@ -235,7 +248,8 @@ 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); @@ -243,8 +257,9 @@ static Plan getPlan( plan.manifest_file_to_first_snapshot[manifest_file.manifest_file_path] = snapshot.snapshot_id; if (!plan.manifest_file_lineage.contains(manifest_file.manifest_file_path)) plan.manifest_file_lineage[manifest_file.manifest_file_path] = {manifest_file.added_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)) { @@ -253,38 +268,50 @@ 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), + raw_metadata_path, 0, - Iceberg::getIdentityPartitionColumnValues(*data_file, *persistent_table_components.schema_processor)); + Iceberg::getIdentityPartitionColumnValues(*data_file, *persistent_table_components.schema_processor), + resolved_storage, + resolved_key); /// One DataFilePlan per source *data file*, keyed by the data file's own path. /// Keying by the manifest path made every data file after the first in a /// manifest reuse the first file's plan, so writeDataFiles rewrote only one /// file per manifest and the rest of the manifest's data silently disappeared /// from the compacted table. The map still deduplicates the same data file /// referenced from multiple snapshots' manifest lists. - const auto & data_file_path = data_file->parsed_entry->file_path_key; std::shared_ptr data_file_ptr; - if (!plan.path_to_data_file.contains(data_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[data_file_path] = data_file_ptr; + plan.path_to_data_file[path_identifier] = data_file_ptr; } else { - data_file_ptr = plan.path_to_data_file[data_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()); @@ -302,7 +329,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); @@ -318,7 +345,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; { @@ -351,10 +379,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( @@ -423,7 +456,7 @@ static bool writeConsolidatedManifestFile( int metadata_version, Poco::JSON::Object::Ptr metadata_object, const PersistentTableComponents & persistent_table_components, - ObjectStoragePtr object_storage, ContextPtr context, + ObjectStoragePtr object_storage, SecondaryStorages & secondary_storages, ContextPtr context, SharedHeader sample_block_, String write_format, CompressionMethod compression_method, @@ -627,7 +660,8 @@ static bool writeConsolidatedManifestFile( std::unordered_set delete_manifest_paths; auto current_manifest_list = getManifestList( - object_storage, persistent_table_components, context, IcebergPathFromMetadata::deserialize(current_manifest_list_path), log); + object_storage, persistent_table_components, context, IcebergPathFromMetadata::deserialize(current_manifest_list_path), log, + secondary_storages); for (const auto & manifest_file : current_manifest_list) { @@ -641,8 +675,16 @@ static bool writeConsolidatedManifestFile( /// A manifest-only rewrite cannot round-trip per-file `key_metadata` (data-file encryption keys), so reject rather than silently dropping it and making an encrypted table unreadable. { - RelativePathWithMetadata key_metadata_object_info(persistent_table_components.path_resolver.resolve(manifest_file.manifest_file_path)); - auto key_metadata_buf = createReadBuffer(key_metadata_object_info, object_storage, context, log); + /// The manifest file may live outside the table's own storage, so read it from the storage its path resolves to. + auto [manifest_storage_to_use, manifest_key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, + manifest_file.manifest_file_path.serialize(), + object_storage, + secondary_storages, + context, + persistent_table_components.path_resolver); + RelativePathWithMetadata key_metadata_object_info(manifest_key_in_storage); + auto key_metadata_buf = createReadBuffer(key_metadata_object_info, manifest_storage_to_use, context, log); AvroForIcebergDeserializer key_metadata_deserializer(std::move(key_metadata_buf), manifest_file.manifest_file_path, getFormatSettings(context)); if (key_metadata_deserializer.hasPath(c_data_file_key_metadata)) { @@ -656,7 +698,8 @@ static bool writeConsolidatedManifestFile( } 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); for (const auto & data_file : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { @@ -892,6 +935,7 @@ static bool writeConsolidatedManifestFile( path_resolver, metadata_object, object_storage, + secondary_storages, context, consolidated_manifest_paths, new_snapshot.snapshot, @@ -1013,7 +1057,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"); @@ -1119,6 +1163,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, @@ -1298,6 +1343,7 @@ static void writeMetadataFiles( path_resolver, metadata_object, object_storage, + secondary_storages, context, renamed_manifest_entries, new_snapshots[i].snapshot, @@ -1325,28 +1371,63 @@ 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)); } } void compactIcebergManifests( const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage_, + std::shared_ptr secondary_storages_, const DataLakeStorageSettings & data_lake_settings, SharedHeader sample_block_, ContextPtr context_, @@ -1399,7 +1480,7 @@ void compactIcebergManifests( /// Cheap pre-check: read just the current manifest list to decide whether the table is above the configured threshold. if (!isCurrentManifestListAboveThreshold( - metadata_object, persistent_table_components, object_storage_, context_, min_count_to_compact)) + metadata_object, persistent_table_components, object_storage_, *secondary_storages_, context_, min_count_to_compact)) { LOG_INFO(log, "Manifest compaction is not needed (manifest list is within threshold {})", min_count_to_compact); @@ -1411,6 +1492,7 @@ void compactIcebergManifests( metadata_object, persistent_table_components, object_storage_, + *secondary_storages_, context_, sample_block_, write_format, @@ -1439,6 +1521,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_, @@ -1452,12 +1535,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_, @@ -1466,9 +1551,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 e6e3bb17dbbd..d9d96229014e 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 @@ -24,6 +25,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_, @@ -33,6 +35,7 @@ void compactIcebergTable( void compactIcebergManifests( const PersistentTableComponents & persistent_table_components, DB::ObjectStoragePtr object_storage_, + std::shared_ptr secondary_storages_, const DataLakeStorageSettings & data_lake_settings, DB::SharedHeader sample_block_, DB::ContextPtr context_, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp index e6c3c25f2909..78fc0f8412ca 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 @@ -344,16 +345,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( @@ -365,7 +384,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) { @@ -374,17 +394,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); } } } @@ -408,23 +432,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 (...) { @@ -434,32 +469,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; @@ -471,12 +516,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; } @@ -634,13 +679,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 (...) @@ -665,7 +715,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('/')) @@ -753,10 +804,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) { @@ -803,7 +856,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{ @@ -835,7 +888,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); @@ -848,7 +902,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 53710d72093d..2be7f5067f10 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp @@ -1,4 +1,4 @@ -#include +#include #include "config.h" #include @@ -12,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; } @@ -55,12 +58,15 @@ String computePartitionId(const Row & partition_key_value) IcebergDataObjectInfo::IcebergDataObjectInfo( Iceberg::ProcessedManifestFileEntryPtr data_manifest_file_entry_, - const String & resolved_storage_path_, + const String & metadata_path_, Int32 schema_id_relevant_to_iterator_, - std::vector> identity_partition_columns_) - : ObjectInfo(RelativePathWithMetadata(resolved_storage_path_)) + std::vector> identity_partition_columns_, + 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, @@ -72,7 +78,11 @@ IcebergDataObjectInfo::IcebergDataObjectInfo( data_manifest_file_entry_->parsed_entry->record_count, data_manifest_file_entry_->parsed_entry->file_size_in_bytes, std::move(identity_partition_columns_)} + , 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_) @@ -91,13 +101,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) @@ -128,7 +140,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); @@ -137,12 +172,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 { @@ -154,7 +189,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()) @@ -212,6 +247,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 3caf8de87958..73edd3d9c5b4 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{}; @@ -33,6 +37,9 @@ struct IcebergObjectSerializableInfo std::optional file_size_in_bytes; std::vector> identity_partition_columns; + /// 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); @@ -45,6 +52,7 @@ struct IcebergObjectSerializableInfo #if USE_AVRO #include +#include #include @@ -63,11 +71,15 @@ struct IcebergDataObjectInfo : public ObjectInfo, std::enable_shared_from_this> identity_partition_columns_); + std::vector> identity_partition_columns_, + ObjectStoragePtr resolved_storage_ = nullptr, + const String & resolved_key_ = ""); explicit IcebergDataObjectInfo(const RelativePathWithMetadata & path_); explicit IcebergDataObjectInfo(const RelativePathWithMetadata & path_, const Iceberg::IcebergObjectSerializableInfo & info_); @@ -77,7 +89,9 @@ 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; } @@ -90,8 +104,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 ada57d40ae42..6a5acbf13053 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include @@ -251,7 +252,8 @@ void SingleThreadIcebergKeysIterator::schedulePrefetchIfPossible() path = manifest_list_entry.manifest_file_path, bytes = manifest_list_entry.manifest_file_byte_size]() { - return Iceberg::getManifestFile(object_storage, persistent_components, local_context, log, path, bytes); + return Iceberg::getManifestFile( + object_storage, persistent_components, local_context, log, path, bytes, *secondary_storages); }; prefetched_manifest = PrefetchedManifest{index, prefetch_runner(std::move(fetch), Priority{})}; return; @@ -272,7 +274,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(makeManifestFilterDag(filter_dag_, local_context_)) , local_context(local_context_) @@ -280,6 +283,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_) , prefetch_runner(threadPoolCallbackRunnerUnsafe( getIOThreadPool().get(), DB::ThreadName::ICEBERG_ITERATOR)) @@ -295,7 +299,8 @@ 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")) , object_storage(std::move(object_storage_)) , local_context(local_context_) @@ -310,9 +315,11 @@ IcebergIterator::IcebergIterator( filter_dag_, table_snapshot_, data_snapshot_, - persistent_components_) + persistent_components_, + secondary_storages_) , blocking_queue(100) , callback(std::move(callback_)) + , secondary_storages(secondary_storages_) { /// Decoding any manifest reads settings from the context, so a missing one is fatal either way. if (!local_context) @@ -419,7 +426,8 @@ void IcebergIterator::decodeDeleteManifests() local_context, logger, manifest_list_entry.manifest_file_path, - manifest_list_entry.manifest_file_byte_size); + manifest_list_entry.manifest_file_byte_size, + *secondary_storages); auto manifest_file_iterator = Iceberg::ManifestFileIterator::create( manifest_file_cacheable_part.deserializer, @@ -469,12 +477,22 @@ 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, - Iceberg::getIdentityPartitionColumnValues(*manifest_file_entry, *persistent_components.schema_processor)); + 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, + Iceberg::getIdentityPartitionColumnValues(*manifest_file_entry, *persistent_components.schema_processor), + 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)) { @@ -510,7 +528,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()); } } @@ -527,7 +545,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()) @@ -539,6 +557,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 bb86db7cfe77..8fda0156d706 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h @@ -28,6 +28,7 @@ #include #include #include +#include namespace DB { @@ -45,7 +46,8 @@ class SingleThreadIcebergKeysIterator const ActionsDAG * filter_dag_, TableStateSnapshotPtr table_snapshot_, IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components); + PersistentTableComponents persistent_components, + std::shared_ptr secondary_storages_); ~SingleThreadIcebergKeysIterator(); @@ -62,6 +64,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; @@ -88,7 +92,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; @@ -119,6 +124,7 @@ class IcebergIterator : public IObjectIterator std::exception_ptr deletes_exception TSA_GUARDED_BY(deletes_mutex); 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 2af675f6cf10..035353dd43e7 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp @@ -6,6 +6,7 @@ #if USE_AVRO +#include #include #include #include @@ -78,6 +79,7 @@ #include #include #include +#include #include #include @@ -244,6 +246,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) @@ -302,7 +305,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); } } @@ -432,7 +435,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, @@ -477,6 +480,7 @@ bool IcebergMetadata::optimize( snapshots_info, persistent_components, object_storage, + secondary_storages, data_lake_settings, format_settings, sample_block, @@ -515,6 +519,7 @@ bool IcebergMetadata::optimizeManifestFiles( compactIcebergManifests( persistent_components, object_storage, + secondary_storages, data_lake_settings, sample_block, context, @@ -677,6 +682,7 @@ void IcebergMetadata::mutate( metadata_snapshot, storage_id, object_storage, + *secondary_storages, data_lake_settings, persistent_components, write_format, @@ -782,7 +788,7 @@ Pipe IcebergMetadata::executeCommand( checkTableRootIsQueriedPath("expire_snapshots"); 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") { @@ -796,7 +802,7 @@ Pipe IcebergMetadata::executeCommand( checkTableRootIsQueriedPath("remove_orphan_files"); return Iceberg::executeRemoveOrphanFiles( - args, context, object_storage_, data_lake_settings, persistent_components); + args, context, object_storage_, data_lake_settings, persistent_components, *secondary_storages); } else { @@ -1144,7 +1150,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}) @@ -1181,7 +1187,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; @@ -1212,7 +1218,7 @@ bool IcebergMetadata::supportsLazyMaterialization(StorageMetadataPtr storage_met 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.areAllDataFilesEligibleForLazyMaterialization(table_state_snapshot->schema_id)) return false; @@ -1250,7 +1256,8 @@ 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); /// Live delete files make an exact metadata-only count impossible: /// - the record count of an equality delete file is the number of delete predicates, @@ -1304,7 +1311,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 {}; @@ -1344,7 +1351,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 @@ -1401,7 +1409,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()); @@ -1411,9 +1419,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; @@ -1440,7 +1453,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, @@ -1527,7 +1540,7 @@ SinkToStoragePtr IcebergMetadata::write( if (context->getSettingsRef()[Setting::allow_insert_into_iceberg]) { checkTableRootIsQueriedPath("INSERT"); - 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 { @@ -1540,25 +1553,62 @@ 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; + + /// Skipped rather than refused: this runs after the table is already marked as dropped, so + /// throwing here only makes `DatabaseCatalog` retry the drop forever. + if (persistent_components.table_root_was_derived) { - /// Skipped rather than refused: this runs after the table is already marked as dropped, so - /// throwing here only makes `DatabaseCatalog` retry the drop forever. - if (persistent_components.table_root_was_derived) + LOG_WARNING( + log, + "Keeping the data of the Iceberg table at '{}': it is below the queried path '{}', which also covers " + "other tables. Drop it while querying the table directory itself to delete the data.", + persistent_components.path_resolver.getTableRoot(), + persistent_components.table_path); + 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 storage = external_files[i].first; + StoredObjects batch; + while (i < external_files.size() && external_files[i].first.get() == storage.get()) { - LOG_WARNING( - log, - "Keeping the data of the Iceberg table at '{}': it is below the queried path '{}', which also covers " - "other tables. Drop it while querying the table directory itself to delete the data.", - persistent_components.path_resolver.getTableRoot(), - persistent_components.table_path); - return; + batch.emplace_back(external_files[i].second); + ++i; } - - auto files = listFiles(*object_storage, persistent_components.table_path, persistent_components.table_path, ""); - for (const auto & file : files) - object_storage->removeObjectIfExists(StoredObject(file)); + /// 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 efd9c0d60029..cc02deb86459 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h @@ -29,6 +29,7 @@ #include #include #include +#include namespace DB @@ -140,6 +141,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 optimizeManifestFiles( const StorageMetadataPtr & metadata_snapshot, @@ -224,7 +227,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 7461e40de7dd..f0641b476df3 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); +} + namespace { std::string_view trimTrailingSlashes(std::string_view str) diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h index 0a30b6427004..3d8cf433308d 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 a83d8b5f3278..ec8c5d59663d 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 @@ -724,6 +725,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, @@ -924,8 +926,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(); @@ -1001,7 +1004,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_) @@ -1012,6 +1016,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] = getLatestMetadataFileAndVersionWithCatalog( object_storage, @@ -1450,7 +1455,7 @@ bool IcebergStorageSink::initializeMetadata() { generateManifestList( persistent_table_components.path_resolver, - metadata, object_storage, context, + metadata, object_storage, *secondary_storages, context, manifest_entries, new_snapshot, manifest_entry_sizes, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h index efad297beefb..14ce994eaf29 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h @@ -43,6 +43,8 @@ namespace DB { +struct SecondaryStorages; + String removeEscapedSlashes(const String & json_str); String stringifyJSON(const Poco::Dynamic::Var & json, unsigned indent = 0); @@ -124,6 +126,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, @@ -148,7 +151,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; @@ -192,6 +196,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 8ea4c06e835e..453e1646a1ee 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp @@ -16,6 +16,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 3bb6890d7f48..89b3cc079b77 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp @@ -361,6 +361,7 @@ static bool writeMetadataFiles( const DataFileWriteResultWithStats & delete_files, const DataFileWriteResultWithStats & data_files, ObjectStoragePtr object_storage, + SecondaryStorages & secondary_storages, ContextPtr context, FileNamesGenerator & filename_generator, const Iceberg::IcebergPathResolver & path_resolver, @@ -525,6 +526,7 @@ static bool writeMetadataFiles( path_resolver, metadata, object_storage, + secondary_storages, context, manifest_entries, new_snapshot, @@ -612,7 +614,8 @@ void validateSnapshotDataFileFormatsForMutation( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + SecondaryStorages & secondary_storages) { if (!metadata->has(f_current_snapshot_id) || metadata->isNull(f_current_snapshot_id)) return; @@ -638,11 +641,12 @@ void validateSnapshotDataFileFormatsForMutation( return; auto manifest_list_path = IcebergPathFromMetadata::deserialize(current_snapshot->getValue(f_manifest_list)); - auto manifest_list_entries = getManifestList(object_storage, persistent_table_components, context, manifest_list_path, log); + auto manifest_list_entries + = getManifestList(object_storage, persistent_table_components, context, manifest_list_path, log, secondary_storages); for (const auto & manifest_list_entry : manifest_list_entries) { auto files_handle = getManifestFileEntriesHandle( - object_storage, persistent_table_components, context, log, manifest_list_entry, current_schema_id); + object_storage, persistent_table_components, context, log, manifest_list_entry, current_schema_id, secondary_storages); for (const auto & file_entry : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { @@ -666,6 +670,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, @@ -731,7 +736,7 @@ void mutate( /// metadata version, so a concurrent writer that commits non-Parquet /// data files between iterations is caught by the next retry. validateSnapshotDataFileFormatsForMutation( - metadata, object_storage, persistent_table_components, context, log, static_cast(current_schema_id)); + metadata, object_storage, persistent_table_components, context, log, static_cast(current_schema_id), secondary_storages); TableStateSnapshot current_iceberg_snapshot; current_iceberg_snapshot.metadata_file_path = metadata_path; @@ -770,6 +775,7 @@ void mutate( mutation_files->delete_file, 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 2cd2d33187d5..c6a4c3ffc1c3 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 { @@ -35,6 +36,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 b16516209dd3..59eed8ba9a91 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 ade8c7053f39..88750a90cc02 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 @@ -14,6 +19,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -24,7 +30,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; @@ -38,14 +45,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); @@ -62,11 +69,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; @@ -79,7 +87,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)); } } } @@ -91,8 +99,12 @@ 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); /// version-hint.text is not a metadata path: it is a fixed object under the storage root. @@ -109,14 +121,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); } } @@ -127,7 +139,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, @@ -151,40 +164,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 9b3a648938b9..cd767df88f53 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp @@ -76,7 +76,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; @@ -85,7 +86,11 @@ 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 @@ -98,8 +103,8 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( std::this_thread::sleep_for(std::chrono::milliseconds(400)); }); - 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}; @@ -120,7 +125,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, @@ -128,7 +134,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, @@ -153,7 +160,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; @@ -162,14 +170,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 @@ -184,7 +196,7 @@ ManifestFileCacheKeys getManifestList( local_context, [&] { return manifest_list_deserializer.getMetadataContent(); }, DB::IcebergMetadataLogLevel::ManifestListMetadata, - persistent_table_components.path_resolver.getTableRoot(), + persistent_table_components.table_path, filename, std::nullopt, std::nullopt); @@ -242,14 +254,14 @@ ManifestFileCacheKeys getManifestList( Int32 partition_spec_id = static_cast( manifest_list_deserializer.getValueFromRowByName(i, f_partition_spec_id, TypeIndex::Int32).safeGet()); manifest_file_cache_keys.emplace_back( - manifest_file_name, manifest_length, added_sequence_number, added_snapshot_id.safeGet(), content_type, - partition_spec_id); + manifest_file_name, static_cast(manifest_length), added_sequence_number, added_snapshot_id.safeGet(), + content_type, partition_spec_id); insertRowToLogTable( local_context, [&] { return 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 497f559d7c95..3e2b052ab880 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp @@ -45,12 +45,13 @@ #include #include #include +#include #if USE_AVRO #include -#include #include +#include #include #include #include @@ -113,7 +114,6 @@ static constexpr size_t MAX_LIST_RETRIES = 5; namespace DB::Iceberg { - using namespace DB; /// Best-effort heuristic based on ClickHouse naming conventions. @@ -1649,3 +1649,29 @@ PartitionColumnValues getIdentityPartitionColumnValues( } #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 267c6e1f1e02..fd3c7541e755 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h @@ -1,9 +1,7 @@ #pragma once +#include #include "config.h" - -#if USE_AVRO - #include #include #include @@ -17,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 2a9545913bf1..2bdee786dc61 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()); @@ -166,24 +173,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)); } } } diff --git a/src/Storages/ObjectStorage/IObjectIterator.h b/src/Storages/ObjectStorage/IObjectIterator.h index 22f776a6ac35..48749c1b3469 100644 --- a/src/Storages/ObjectStorage/IObjectIterator.h +++ b/src/Storages/ObjectStorage/IObjectIterator.h @@ -59,6 +59,10 @@ struct ObjectInfo /// Sorted absolute row indexes within the file, see FormatFilterInfo::rows_to_read. std::shared_ptr> rows_to_read; + /// 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(bool include_file_bucket_info = true) const; String getIdentifierForPath(const String & path, bool include_file_bucket_info = true) const; }; diff --git a/src/Storages/ObjectStorage/ReadBufferIterator.cpp b/src/Storages/ObjectStorage/ReadBufferIterator.cpp index 40802570831e..10895685c18d 100644 --- a/src/Storages/ObjectStorage/ReadBufferIterator.cpp +++ b/src/Storages/ObjectStorage/ReadBufferIterator.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -44,9 +45,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()); } @@ -58,7 +59,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()); } @@ -83,7 +84,8 @@ std::optional ReadBufferIterator::tryGetColumnsFromCache( /// plain string overload would drop it and could validate one shard using another's metadata. auto metadata_object = object_info->relative_path_with_metadata; metadata_object.relative_path = path; - auto meta = object_storage->tryGetObjectMetadata(metadata_object, /*with_tags=*/ false); + auto storage_to_use = getResolvedStorageFromObjectInfo(object_info, object_storage); + auto meta = storage_to_use->tryGetObjectMetadata(metadata_object, /*with_tags=*/ false); if (meta) object_info->setObjectMetadata(*meta); } @@ -100,7 +102,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; } @@ -111,7 +113,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. @@ -127,13 +129,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) @@ -153,8 +155,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 & settings_ref = context->getSettingsRef(); @@ -282,7 +287,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 b0093bde0cdd..61930596b8d6 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp @@ -58,6 +58,7 @@ #include #include #include +#include #if ENABLE_DISTRIBUTED_CACHE #include #include @@ -379,6 +380,29 @@ std::string StorageObjectStorageSource::getUniqueStoragePathIdentifier( return result; } +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, @@ -403,11 +427,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) @@ -653,6 +683,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())) @@ -669,7 +701,7 @@ Chunk StorageObjectStorageSource::generate() chunk, read_from_format_info.requested_virtual_columns, { - .path = path, + .path = path_for_virtual_column, .storage_id = storage_id, .size = object_size, .filename = &filename, @@ -883,7 +915,7 @@ Chunk StorageObjectStorageSource::generate() && !format_filter_info->filter_actions_dag && !hasAttachedDeletes(*reader.getObjectInfo()) && !reader.getObjectInfo()->rows_to_read) - addNumRowsToCache(*reader.getObjectInfo(), total_rows_in_file); + addNumRowsToCache(reader.getObjectInfo(), total_rows_in_file); total_rows_in_file = 0; @@ -905,11 +937,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); @@ -970,9 +1002,11 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade auto metadata_object = object_info->relative_path_with_metadata; metadata_object.relative_path = path; + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(object_info, object_storage); + if (query_settings.ignore_non_existent_file) { - auto metadata = object_storage->tryGetObjectMetadata(metadata_object, with_tags); + auto metadata = storage_to_use->tryGetObjectMetadata(metadata_object, with_tags); if (!metadata) return {}; @@ -980,7 +1014,7 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade } else { - object_info->setObjectMetadata(object_storage->getObjectMetadata(metadata_object, with_tags)); + object_info->setObjectMetadata(storage_to_use->getObjectMetadata(metadata_object, with_tags)); } } @@ -1039,7 +1073,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_); @@ -1117,7 +1151,12 @@ 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, std::nullopt, !headers_requested); + object_info->relative_path_with_metadata, + getResolvedStorageFromObjectInfo(object_info, object_storage), + context_, + log, + std::nullopt, + !headers_requested); } Block initial_header = read_from_format_info.format_header; @@ -2050,11 +2089,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, @@ -2080,7 +2121,10 @@ StorageObjectStorageSource::ReadTaskIterator::ReadTaskIterator( { auto object = object_future.get(); if (object) + { + resolveIcebergObjectStorageIfNeeded(object); buffer.push_back(object); + } } } @@ -2096,6 +2140,30 @@ static size_t getKnownArchiveSize(const ObjectInfoPtr & object_info) return object_metadata->size_bytes; } +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) { size_t current_index = index.fetch_add(1, std::memory_order_relaxed); @@ -2110,7 +2178,9 @@ ObjectInfoPtr StorageObjectStorageSource::ReadTaskIterator::next(size_t) if (!task || task->isEmpty()) return nullptr; + object_info = task->getObjectInfo(); + resolveIcebergObjectStorageIfNeeded(object_info); } else { @@ -2210,7 +2280,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); } @@ -2232,7 +2305,10 @@ ObjectInfoPtr StorageObjectStorageSource::ArchiveIterator::next(size_t processor } if (!archive_object->getObjectMetadata()) - archive_object->setObjectMetadata(object_storage->getObjectMetadata(archive_object->relative_path_with_metadata, /*with_tags=*/ false)); + { + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(archive_object, object_storage); + archive_object->setObjectMetadata(storage_to_use->getObjectMetadata(archive_object->relative_path_with_metadata, /*with_tags=*/ false)); + } archive_reader = createArchiveReader(archive_object); file_enumerator = archive_reader->firstFile(); @@ -2258,7 +2334,10 @@ ObjectInfoPtr StorageObjectStorageSource::ArchiveIterator::next(size_t processor return {}; if (!archive_object->getObjectMetadata()) - archive_object->setObjectMetadata(object_storage->getObjectMetadata(archive_object->relative_path_with_metadata, /*with_tags=*/ false)); + { + ObjectStoragePtr storage_to_use = getResolvedStorageFromObjectInfo(archive_object, object_storage); + archive_object->setObjectMetadata(storage_to_use->getObjectMetadata(archive_object->relative_path_with_metadata, /*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 ec9b155d0860..2f2870d3afed 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 @@ -75,6 +77,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; @@ -160,7 +170,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(); }; @@ -172,6 +182,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; @@ -184,11 +195,19 @@ class StorageObjectStorageSource::ReadTaskIterator : public IObjectIterator, pri const std::string & path_in_archive, std::optional read_source_index); + /// 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 dd3b6a1544f3..1905e948dcf2 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -19,6 +21,11 @@ String getSchedulingIdentifier(const ObjectInfoPtr & object_info, bool send_over 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(); } diff --git a/src/Storages/ObjectStorage/Utils.cpp b/src/Storages/ObjectStorage/Utils.cpp index a3baa40beb1e..341f134f30cc 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 { @@ -28,6 +49,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 @@ -363,5 +551,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 2e4566e61014..f9e3a6a0f6e1 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, @@ -69,6 +96,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/ObjectStorage/tests/gtest_storage_object_storage_archive.cpp b/src/Storages/ObjectStorage/tests/gtest_storage_object_storage_archive.cpp index 295dd645286a..03db0d0519d4 100644 --- a/src/Storages/ObjectStorage/tests/gtest_storage_object_storage_archive.cpp +++ b/src/Storages/ObjectStorage/tests/gtest_storage_object_storage_archive.cpp @@ -98,6 +98,7 @@ TEST(StorageObjectStorageArchive, DistributedArchiveRejectsUnknownSize) /*max_threads_count=*/1, /*is_archive_=*/true, object_storage, + /*table_location_=*/"", context); try 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..a45c12519e04 --- /dev/null +++ b/tests/integration/test_storage_iceberg_multistorage/test.py @@ -0,0 +1,1070 @@ +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 read_avro_records(avro_path: str) -> list: + """Read all records of an AVRO file without modifying it.""" + with open(avro_path, 'rb') as f: + reader = avro.datafile.DataFileReader(f, avro.io.DatumReader()) + records = list(reader) + reader.close() + return records + + +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, kept_values_sql, kept_rows, data_bucket): + # A second append writes a second data file, and deleting one of its two rows + # merge-on-read leaves a live v2 position delete file. Per the Iceberg spec a delete file + # is applied only to the data file(s) it references, so the first data file -- the one + # relocated below -- keeps no deletes, while the table's exact row count is no longer + # derivable from the manifests: "Calculating `total_record_count` for a table with equality + # deletes or v2 position delete files requires reading data." That is what forces `count()` + # to open the first data file and therefore to consult the num-rows cache this test + # measures. The second file needs a row that survives: a predicate matching every row of a + # file is satisfied by dropping the file from the manifest, with no delete file written. + spark.sql( + f""" + CREATE TABLE {table_name} (id INT, value STRING) + USING iceberg + TBLPROPERTIES ( + 'format-version' = '2', + 'write.delete.mode' = 'merge-on-read' + ) + """ + ) + spark.sql(f"INSERT INTO {table_name} VALUES {kept_values_sql}") + spark.sql(f"INSERT INTO {table_name} VALUES (998, 'survivor'), (999, 'deleted')") + spark.sql(f"DELETE FROM {table_name} WHERE id = 999") + + 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-")] + + # `content` 0 is DATA and 1 is POSITION DELETES; `status` 2 is DELETED. Pick the live data + # file holding the kept rows -- the delete file references the other one, not this. + live_entries = [ + r["data_file"] for mf in manifest_files for r in read_avro_records(mf) if r["status"] != 2 + ] + assert any(e["content"] == 1 for e in live_entries), \ + "Expected a live position delete file; is `write.delete.mode` still merge-on-read?" + kept_paths = {e["file_path"] for e in live_entries if e["content"] == 0 and e["record_count"] == kept_rows} + assert len(kept_paths) == 1, f"Expected one data file with {kept_rows} records, got: {live_entries}" + relocated_path = next(iter(kept_paths)) + + # Point that data file -- and only it -- at the same object key in a different bucket. + for mf in manifest_files: + modify_avro_file( + mf, + ["data_file", "file_path"], + lambda p: f"s3a://{data_bucket}/{shared_key}" if p == relocated_path else p, + ) + # `value_counts` is optional per the spec, and a table without it is an ordinary table + # written by a writer that collects no column statistics. Drop it so the per-file row + # count cannot be answered from the manifest either: that shortcut returns before the + # num-rows cache is consulted, which is the thing under test here. + modify_avro_file(mf, ["data_file", "value_counts"], lambda _: None) + + for f in manifest_files: + rel = os.path.relpath(f, host_path) + started_cluster.default_s3_uploader.upload_file(f, f"{base_path}/{rel}") + + relocated_basename = os.path.basename(relocated_path) + local_relocated = [f for f in find_files(data_dir, ".parquet") if os.path.basename(f) == relocated_basename] + assert len(local_relocated) == 1, f"Could not find {relocated_basename} under {data_dir}" + S3Uploader(started_cluster.minio_client, data_bucket).upload_file(local_relocated[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')", 3, 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')", 5, 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. + # Each table holds its relocated file plus the one row of the second file that survived the + # delete, so a reused entry from table `a` would show up as 4 rather than 6 for table `b`. + 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 == "4" + assert count_b == "6" + # 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 +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" : [ ],