diff --git a/docs/en/antalya/protocol.md b/docs/en/antalya/protocol.md index be10b9014a311..d66326b0f1cb5 100644 --- a/docs/en/antalya/protocol.md +++ b/docs/en/antalya/protocol.md @@ -14,15 +14,21 @@ upstream ClickHouse cannot reach, defined in `src/Core/AntalyaProtocol.h`. Both the name string of their `Hello`, on every connection: ```text -client -> server "ClickHouse client (antalya:1)" -server -> client "ClickHouse (antalya:1)" +client -> server "ClickHouse client (antalya:2)" +server -> client "ClickHouse (antalya:2)" ``` Each side parses the peer's suffix, caps the value with `min(own, peer)` and keeps the result. `0` means the peer is not an Antalya build. Negotiation is per hop and not transitive: initiator to worker and worker to worker negotiate independently. -Version 1 is the advertisement itself. Nothing is gated on it yet. +Version 1 is the advertisement itself. + +Version 2 lets the `*Cluster` read-task payload (the `ReadTaskResponse` body) carry the Iceberg +delete-file paths as absolute URIs, as the manifest spells them, so that a worker can read files +outside the table location, including files on another object storage. For a worker below version 2 +the initiator strips the scheme and authority as before, and refuses to dispatch a data file that +needs another storage or whose delete-file keys differ from the resolved keys. ## Adding an Antalya-only wire change {#adding-a-wire-change} diff --git a/docs/reference/interfaces/specs/NativeProtocol.mdx b/docs/reference/interfaces/specs/NativeProtocol.mdx index b042ad61c0823..3480f44075162 100644 --- a/docs/reference/interfaces/specs/NativeProtocol.mdx +++ b/docs/reference/interfaces/specs/NativeProtocol.mdx @@ -146,7 +146,7 @@ When a feature is active, its fields **must** be present on the wire. The protoc | JWT_IN_INTERSERVER | 54476 | ClientInfo | Adds a JWT-presence UInt8 + optional `String jwt` at the tail of ClientInfo. External clients (no JWT) send byte `0x00`. (Spelled `DBMS_MIN_REVISON_WITH_JWT_IN_INTERSERVER` in C++ — note the typo in the constant name.) | | QUERY_PLAN_SERIALIZATION | 54477 | ServerHello, QueryPlan packet | ServerHello appends `VarUInt query_plan_serialization_version` after server settings. Also introduces `ClientPacket::QueryPlan` (code `13`) for inter-server delivery of pre-built query plans — external clients never send. | | PARALLEL_BLOCK_MARSHALLING | 54478 | Block (Column) | Server may wrap columns in `ColumnBLOB` (compressed inline) for parallel processing. Gated on the query having compression enabled AND `rows > 1`; otherwise the regular column wire format applies. Clients that never enable compression on outgoing Query packets see no wire change. | -| VERSIONED_CLUSTER_FUNCTION_PROTOCOL | 54479 | ServerHello | Adds `VarUInt cluster_function_protocol_version` at the tail of ServerHello. Used for `*Cluster` table functions (`s3Cluster`, etc.). Current value: `8` (`DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION`); version `7` is reserved for a private-repository feature (Iceberg compaction), and `8` adds an optional `read_source_index` to the inter-server cluster read-task payload (the `ReadTaskResponse` body, which stays unspecified here — see below). External clients decode and ignore. | +| VERSIONED_CLUSTER_FUNCTION_PROTOCOL | 54479 | ServerHello | Adds `VarUInt cluster_function_protocol_version` at the tail of ServerHello. Used for `*Cluster` table functions (`s3Cluster`, etc.). Current value: `9` (`DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION`); version `7` is reserved for a private-repository feature (Iceberg compaction), `8` adds an optional `read_source_index` to the inter-server cluster read-task payload (the `ReadTaskResponse` body, which stays unspecified here — see below), and `9` adds Iceberg identity partition columns. External clients decode and ignore. | | OUT_OF_ORDER_BUCKETS_IN_AGGREGATION | 54480 | BlockInfo | Adds field 3 (`out_of_order_buckets: Vec`) to BlockInfo's field-tagged stream. Decoded as `[VarUInt count][Int32]*count`. External clients don't emit this themselves; the decoder reads any non-empty list the server sends. | | COMPRESSED_LOGS_PROFILE_EVENTS_COLUMNS | 54481 | Log, ProfileEvents, TableColumns | Server may wrap [`Log`](#log), [`ProfileEvents`](#profileevents), and [`TableColumns`](#tablecolumns) packet bodies in the [compression frame](/reference/interfaces/specs/NativeFormat#compression-frame). At this version all three bodies travel through the same optionally-compressed output path, which becomes a real compression frame only when the query has `compression = true`. Clients that never enable compression on outgoing Query packets see no wire change. | | REPLICATED_SERIALIZATION | 54482 | Block (Column) | Server may emit columns with kind_stack `0x04 = REPLICATED` — a dictionary-style compact form for repeated values — see [kind_stack and sparse encoding](/reference/interfaces/specs/NativeFormat#kind-stack-and-sparse-encoding). Below this version the writer expanded such columns before sending. Decoded via index lookup (`elements[indexes[i]]` per row); leaf types plus `Nullable`/`Array`/`Tuple`/`Map`/`Nested`/`LowCardinality` inners supported. | @@ -463,7 +463,7 @@ Server → Client. The reply to ClientHello on successful authentication. | 11 | nonce | UInt64 | inter-server | INTERSERVER_SECRET_V2 (v54462) | 8-byte LE random nonce. The server's inter-server query-signing scheme uses it. External clients MUST decode it (to keep the stream aligned) and SHOULD ignore the value. | | 12 | server_settings | Setting[] | universal | SERVER_SETTINGS (v54474) | Server's non-default settings broadcast. Format: zero or more `(String key, VarUInt flags, String value)` triples, terminated by an empty key. Same as the [Query packet's settings list](#setting). | | 13 | query_plan_serialization_version | VarUInt | universal | QUERY_PLAN_SERIALIZATION (v54477) | Server's supported query-plan serialization version. External clients decode and ignore. | -| 14 | cluster_function_protocol_version | VarUInt | universal | VERSIONED_CLUSTER_FUNCTION_PROTOCOL (v54479) | Server's `*Cluster` table-function protocol version. Current: `8`. The value gates additive fields in the inter-server cluster read-task payload (the otherwise-unspecified `ReadTaskResponse` body); version `7` is reserved for a private-repository feature (Iceberg compaction), and `8` adds an optional `read_source_index`. External clients do not participate in cluster reads — they decode and ignore this field. | +| 14 | cluster_function_protocol_version | VarUInt | universal | VERSIONED_CLUSTER_FUNCTION_PROTOCOL (v54479) | Server's `*Cluster` table-function protocol version. Current: `9`. The value gates additive fields in the inter-server cluster read-task payload (the otherwise-unspecified `ReadTaskResponse` body); version `7` is reserved for a private-repository feature (Iceberg compaction), `8` adds an optional `read_source_index`, and `9` adds Iceberg identity partition columns. External clients do not participate in cluster reads — they decode and ignore this field. | **Rule** — an element of `password_complexity_rules`: diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index c014ea12a7a46..9d57a3fe68b2d 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -18,10 +18,13 @@ #include #include #include +#include #include +#include #include #include #include +#include #include @@ -40,6 +43,23 @@ namespace ErrorCodes namespace { + String normalizeAccessURI(std::string_view uri) + { + if (uri.empty()) + return {}; + + try + { + Poco::URI parsed{String{uri}}; + parsed.normalize(); + return parsed.toString(); + } + catch (const Poco::Exception &) + { + return {}; + } + } + AccessRights mixAccessRightsFromUserAndRoles(const User & user, const EnabledRolesInfo & roles_info) { AccessRights res = user.access; @@ -1100,13 +1120,16 @@ bool ContextAccess::isGrantedWithFilter(const ContextPtr & context, const Access if (isGranted(context, flags, parameter)) return true; - if (!to_check_by_filter.empty()) + const String normalized_filter_target = parameter == AccessTypeObjects::toStringSource(AccessTypeObjects::Source::FILE) + ? std::filesystem::path{String{to_check_by_filter}}.lexically_normal().string() + : normalizeAccessURI(to_check_by_filter); + if (!normalized_filter_target.empty()) { auto access_rights = getAccessRights(); auto filters = access_rights->getFilters(parameter); for (const auto & filter : filters) { - if (re2::RE2::FullMatch(to_check_by_filter, filter.path) && filter.access_flags.contains(flags)) + if (re2::RE2::FullMatch(normalized_filter_target, filter.path) && filter.access_flags.contains(flags)) return true; } } diff --git a/src/Access/ContextAccess.h b/src/Access/ContextAccess.h index 5ce4344e50050..a796a3cd43759 100644 --- a/src/Access/ContextAccess.h +++ b/src/Access/ContextAccess.h @@ -133,6 +133,7 @@ class ContextAccess : public std::enable_shared_from_this void checkGranteesAreAllowed(const std::vector & grantee_ids) const; /// Checks access of grants with parameter where a filter can be applied. + /// The URI checked against the filter is normalized first; an invalid URI cannot match a filter. /// For example, for `GRANT READ ON S3('s3://foo.*') TO user` calling `checkAccess(READ, "S3")` will throw an error /// because we are checking for `READ` permissions for all possible URLs. /// @@ -333,6 +334,7 @@ class ContextAccessWrapper : public std::enable_shared_from_this & grantee_ids) const { access->checkGranteesAreAllowed(grantee_ids); } /// Checks access of grants with parameter where a filter can be applied. + /// The URI checked against the filter is normalized first; an invalid URI cannot match a filter. ALWAYS_INLINE void checkAccessWithFilter(const AccessFlags & flags, std::string_view parameter, std::string_view filter) const { access->checkAccessWithFilter(context, flags, parameter, filter); } /// Checks access of grants with parameter where a filter can be applied, without throwing. diff --git a/src/Backups/BackupFactory.cpp b/src/Backups/BackupFactory.cpp index 6bf331ce6fa81..ba18a6183e2da 100644 --- a/src/Backups/BackupFactory.cpp +++ b/src/Backups/BackupFactory.cpp @@ -3,8 +3,6 @@ #include #include -#include - #include #include @@ -25,24 +23,6 @@ namespace { fmt::format_to(std::back_inserter(identity), ":{}:{}", component.size(), component); } - - /// Must normalize identically to `ITableFunction::getFunctionURINormalized`, or the same regex - /// grant matches a table function and not a backup. An empty result requires a whole-source grant. - String normalizeAccessURI(const String & uri) - { - if (uri.empty()) - return uri; - try - { - Poco::URI parsed(uri); - parsed.normalize(); - return parsed.toString(); - } - catch (const Poco::Exception &) - { - return ""; - } - } } @@ -82,7 +62,7 @@ void BackupFactory::checkSourceAccess(const BackupInfo & backup_info, ContextPtr if (auto target = it->second.source_access(backup_info, context, open_mode)) context->getAccess()->checkAccessWithFilter( - target->flags, AccessTypeObjects::toStringSource(target->source), normalizeAccessURI(target->uri)); + target->flags, AccessTypeObjects::toStringSource(target->source), target->uri); } BackupMutablePtr BackupFactory::createBackup(const CreateParams & params) const diff --git a/src/Client/Connection.cpp b/src/Client/Connection.cpp index deb5290a588cd..d5466dbe60db7 100644 --- a/src/Client/Connection.cpp +++ b/src/Client/Connection.cpp @@ -1284,7 +1284,7 @@ void Connection::sendData(const Block & block, const String & name, bool scalar) void Connection::sendClusterFunctionReadTaskResponse(const ClusterFunctionReadTaskResponse & response) { writeVarUInt(Protocol::Client::ReadTaskResponse, *out); - response.serialize(*out, worker_cluster_function_protocol_version); + response.serialize(*out, worker_cluster_function_protocol_version, server_antalya_protocol_version); out->finishChunk(); out->next(); } diff --git a/src/Common/maskSensitiveQueryParameters.cpp b/src/Common/maskSensitiveQueryParameters.cpp index b2c1503cd06fa..0c17107dac22b 100644 --- a/src/Common/maskSensitiveQueryParameters.cpp +++ b/src/Common/maskSensitiveQueryParameters.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -143,4 +144,11 @@ std::string maskSensitiveQueryParametersInURI(const std::string & uri) return result; } +std::string maskCredentialsInURI(const std::string & uri) +{ + std::string result = maskSensitiveQueryParametersInURI(uri); + maskURIPassword(&result); + return result; +} + } diff --git a/src/Common/maskSensitiveQueryParameters.h b/src/Common/maskSensitiveQueryParameters.h index bc2a0c01b709b..fb81b27988286 100644 --- a/src/Common/maskSensitiveQueryParameters.h +++ b/src/Common/maskSensitiveQueryParameters.h @@ -2,7 +2,6 @@ #include - namespace DB { @@ -21,4 +20,7 @@ namespace DB /// If the URI has no query string, the original string is returned unchanged. std::string maskSensitiveQueryParametersInURI(const std::string & uri); +/// Also accepts strings containing a URI; the first '?' starts the query string and any suffix belongs to it. +std::string maskCredentialsInURI(const std::string & uri); + } diff --git a/src/Common/tests/gtest_mask_sensitive_query_parameters.cpp b/src/Common/tests/gtest_mask_sensitive_query_parameters.cpp index e4430a1d817b6..ed9c61668eaab 100644 --- a/src/Common/tests/gtest_mask_sensitive_query_parameters.cpp +++ b/src/Common/tests/gtest_mask_sensitive_query_parameters.cpp @@ -2,6 +2,7 @@ #include +using DB::maskCredentialsInURI; using DB::maskSensitiveQueryParametersInURI; TEST(MaskSensitiveQueryParameters, NoQueryString) @@ -87,3 +88,20 @@ TEST(MaskSensitiveQueryParameters, EmptyQueryString) { EXPECT_EQ(maskSensitiveQueryParametersInURI("/?"), "/?"); } + +TEST(MaskCredentialsInURI, ObjectPathsUnchanged) +{ + EXPECT_EQ(maskCredentialsInURI(""), ""); + EXPECT_EQ(maskCredentialsInURI("s3://bucket/t/data/x.parquet"), "s3://bucket/t/data/x.parquet"); + EXPECT_EQ(maskCredentialsInURI("/table/data/x.parquet"), "/table/data/x.parquet"); +} + +TEST(MaskCredentialsInURI, PasswordAndQueryParameters) +{ + for (const auto & prefix : {"", "7:"}) + { + EXPECT_EQ( + maskCredentialsInURI(std::string(prefix) + "s3://alice:hunter2@minio:9000/bucket/f.parquet?X-Amz-Signature=abc"), + std::string(prefix) + "s3://alice:[HIDDEN]@minio:9000/bucket/f.parquet?X-Amz-Signature=[HIDDEN]"); + } +} diff --git a/src/Core/AntalyaProtocol.h b/src/Core/AntalyaProtocol.h index 53cf8d1ceb683..d422c1698b940 100644 --- a/src/Core/AntalyaProtocol.h +++ b/src/Core/AntalyaProtocol.h @@ -8,7 +8,9 @@ namespace DB { /// Bump for every Antalya-only wire protocol change. See `docs/en/antalya/protocol.md`. -static constexpr auto DBMS_ANTALYA_PROTOCOL_VERSION = 1; +/// 2: the `*Cluster` read-task payload carries Iceberg delete-file paths as absolute URIs. +static constexpr auto DBMS_ANTALYA_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH = 2; +static constexpr auto DBMS_ANTALYA_PROTOCOL_VERSION = DBMS_ANTALYA_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH; namespace AntalyaProtocol { diff --git a/src/Core/Settings.cpp b/src/Core/Settings.cpp index 141a354fc906a..b0cc01da03fd5 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -691,6 +691,10 @@ 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 the base storage's `S3` credentials when accessing Iceberg table files located in a different endpoint. +Ignored if `s3_allow_server_credentials_in_user_queries` is disabled. )", 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 db6f6fd3eafb9..6e44ec5efbc01 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -219,6 +219,7 @@ const VersionToSettingsChangesMap & getSettingsChangesHistory() addSettingsChanges(settings_changes_history, "26.6.2.20001.altinityantalya", { {"data_lake_delete_data_on_drop", false, false, "New setting that unifies dropping of data lake data; the released `iceberg_delete_data_on_drop` is kept as an alias for it."}, + {"object_storage_propagate_credentials_to_other_storages", false, false, "New setting, allowing to reuse the base storage's `S3` credentials for a secondary object storage, even if the target endpoint differs from the base one. Disabled by default. Ignored while `s3_allow_server_credentials_in_user_queries` is disabled."}, {"datalake_ignore_unsupported_table_properties", false, false, "New setting: allow `CREATE TABLE` and `SHOW CREATE TABLE` in a `DataLakeCatalog` to omit unsupported table properties."}, {"object_storage_cluster_fallback_to_local_if_empty", false, false, "New setting"}, {"use_puffin_files_cache", false, true, "Enables cache of parsed Puffin file content such as deletion vectors."}, diff --git a/src/Databases/DataLake/DatabaseDataLake.cpp b/src/Databases/DataLake/DatabaseDataLake.cpp index 6a648d362e878..c5c40e3bd9aa5 100644 --- a/src/Databases/DataLake/DatabaseDataLake.cpp +++ b/src/Databases/DataLake/DatabaseDataLake.cpp @@ -864,7 +864,8 @@ 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 a07fc73456e84..5d8fb880f64c0 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 49378cf46f69a..2524fb6efec59 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 b3c6074f00d3e..31b4022cf0816 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 file_meta_info = object->relative_path_with_metadata.file_meta_info; @@ -91,7 +89,7 @@ ObjectInfoPtr ClusterFunctionReadTaskResponse::getObjectInfo() const return object; } -void ClusterFunctionReadTaskResponse::serialize(WriteBuffer & out, size_t worker_protocol_version) const +void ClusterFunctionReadTaskResponse::serialize(WriteBuffer & out, size_t worker_protocol_version, UInt64 worker_antalya_protocol_version) const { auto protocol_version = std::min(static_cast(worker_protocol_version), static_cast(DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION)); @@ -188,7 +186,7 @@ void ClusterFunctionReadTaskResponse::serialize(WriteBuffer & out, size_t worker if (iceberg_info.has_value()) { writeVarUInt(1, out); - iceberg_info->serializeForClusterFunctionProtocol(out, protocol_version); + iceberg_info->serializeForClusterFunctionProtocol(out, protocol_version, worker_antalya_protocol_version); } else { diff --git a/src/Interpreters/ClusterFunctionReadTask.h b/src/Interpreters/ClusterFunctionReadTask.h index 78649e071483c..6d479258573ee 100644 --- a/src/Interpreters/ClusterFunctionReadTask.h +++ b/src/Interpreters/ClusterFunctionReadTask.h @@ -36,7 +36,8 @@ struct ClusterFunctionReadTaskResponse bool isEmpty() const { return path.empty(); } /// Serialize according to the protocol version. - void serialize(WriteBuffer & out, size_t worker_protocol_version) const; + /// `worker_antalya_protocol_version` is the Antalya protocol version negotiated with the worker. + void serialize(WriteBuffer & out, size_t worker_protocol_version, UInt64 worker_antalya_protocol_version) const; /// Deserialize. Protocol version will be received from `in` /// and the result will be deserialized accordingly. void deserialize(ReadBuffer & in); diff --git a/src/Interpreters/IcebergMetadataLog.cpp b/src/Interpreters/IcebergMetadataLog.cpp index 170dc0f8d7fa7..0d86764e27e1c 100644 --- a/src/Interpreters/IcebergMetadataLog.cpp +++ b/src/Interpreters/IcebergMetadataLog.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -108,14 +109,18 @@ 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, - .file_path = file_path.serialize(), + .table_path = normalized_table_path, + .file_path = maskCredentialsInURI(file_path.serialize()), .metadata_content = row, .row_in_file = row_in_file, .pruning_status = pruning_status}; diff --git a/src/Interpreters/tests/gtest_cluster_function_read_task.cpp b/src/Interpreters/tests/gtest_cluster_function_read_task.cpp index b471b53ed4b68..7dfc232b62df2 100644 --- a/src/Interpreters/tests/gtest_cluster_function_read_task.cpp +++ b/src/Interpreters/tests/gtest_cluster_function_read_task.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -38,7 +39,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsSchemaTransformOnProtocolBeforeData WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_INITIAL_PROCESSING_PROTOCOL_VERSION); + response.serialize(out, DBMS_CLUSTER_INITIAL_PROCESSING_PROTOCOL_VERSION, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -56,7 +57,7 @@ TEST(ClusterFunctionReadTaskResponse, AllowsEmptySchemaTransformOnProtocolBefore String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_INITIAL_PROCESSING_PROTOCOL_VERSION); + response.serialize(out, DBMS_CLUSTER_INITIAL_PROCESSING_PROTOCOL_VERSION, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); EXPECT_FALSE(serialized.empty()); } @@ -70,7 +71,7 @@ TEST(ClusterFunctionReadTaskResponse, RoundTripsSchemaTransformOnSupportedProtoc String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); ReadBufferFromString in(serialized); @@ -92,7 +93,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsNonEmptyExcludedRowsOnOldProtocol) WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -110,7 +111,7 @@ TEST(ClusterFunctionReadTaskResponse, AllowsEmptyExcludedRowsOnOldProtocol) String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); EXPECT_FALSE(serialized.empty()); } @@ -125,7 +126,7 @@ TEST(ClusterFunctionReadTaskResponse, RoundTripsExcludedRowsOnSupportedProtocol) String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_EXCLUDED_ROWS); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_EXCLUDED_ROWS, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); ReadBufferFromString in(serialized); @@ -155,7 +156,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsEqualityDeletesOnProtocolBeforeIceb WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -182,7 +183,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsPositionDeletesOnProtocolBeforeIceb WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -205,7 +206,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsIcebergInfoWithoutDeletesOnProtocol WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_DATA_LAKE_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -240,7 +241,7 @@ TEST(ClusterFunctionReadTaskResponse, RoundTripsIcebergDeletesOnSupportedProtoco String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); ReadBufferFromString in(serialized); @@ -266,7 +267,7 @@ TEST(ClusterFunctionReadTaskResponse, RejectsFileBucketInfoOnProtocolBeforeFileB WriteBufferFromString out(serialized); try { - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); FAIL() << "Expected exception"; } catch (const Exception & e) @@ -283,7 +284,7 @@ TEST(ClusterFunctionReadTaskResponse, AllowsMissingFileBucketInfoOnProtocolBefor String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_METADATA, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); EXPECT_FALSE(serialized.empty()); } @@ -298,7 +299,7 @@ TEST(ClusterFunctionReadTaskResponse, RoundTripsFileBucketInfoOnSupportedProtoco String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_FILE_BUCKETS_INFO, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); ReadBufferFromString in(serialized); diff --git a/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp b/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp index a9534abdd383a..60a2105f65532 100644 --- a/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Common/AvroForIcebergDeserializer.cpp @@ -207,7 +207,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: /// { @@ -312,7 +312,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, @@ -382,7 +382,7 @@ ParsedManifestFileEntryPtr AvroForIcebergDeserializer::createParsedManifestFileE return std::make_shared( FileContentType::POSITION_DELETE, - file_path_key, + file_path_from_metadata, row_index, status, sequence_number, @@ -416,7 +416,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/DataLakeConfiguration.h b/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h index 93341db594bea..0d7ba7bbbbc31 100644 --- a/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h +++ b/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h @@ -352,6 +352,12 @@ class DataLakeConfiguration : public BaseStorageConfiguration, public std::enabl return getMetadata()->getColumnMapperForCurrentSchema(storage_metadata_snapshot, context); } + void checkCanBeDropped(ContextPtr query_context) const override + { + if (auto metadata = tryGetMetadata()) + metadata->checkCanBeDropped(query_context); + } + void drop(bool delete_data) override { if (auto metadata = tryGetMetadata()) @@ -894,6 +900,7 @@ class StorageIcebergConfiguration : public StorageObjectStorageConfiguration, pu bool supportsPrewhere() const override { return getImpl().supportsPrewhere(); } + void checkCanBeDropped(ContextPtr query_context) const override { getImpl().checkCanBeDropped(query_context); } void drop(bool delete_data) override { getImpl().drop(delete_data); } protected: diff --git a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h index a14bce43bf4bc..b3cbd6c7a271b 100644 --- a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h @@ -127,6 +127,8 @@ class IDataLakeMetadata : boost::noncopyable virtual bool operator==(const IDataLakeMetadata & other) const = 0; + virtual std::string getTableLocation() const { return {}; } + /// Return iterator to `data files`. using FileProgressCallback = std::function; virtual ObjectIterator iterate( @@ -290,6 +292,8 @@ class IDataLakeMetadata : boost::noncopyable throwNotImplemented("truncate"); } + /// Checks with the `DROP` query context what `drop` will do in the background. + virtual void checkCanBeDropped(ContextPtr /*query_context*/) const { } /// `delete_data` is what `StorageObjectStorage::drop` resolved from `data_lake_delete_data_on_drop`. virtual void drop(bool /* delete_data */) { } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp index 461e4d55ef21b..9f45cad80df63 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,7 @@ namespace DB::ErrorCodes { extern const int BAD_ARGUMENTS; extern const int LOGICAL_ERROR; + extern const int CANNOT_UNLINK; extern const int ICEBERG_SPECIFICATION_VIOLATION; extern const int NOT_IMPLEMENTED; } @@ -109,8 +111,10 @@ 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; + std::vector external_files; FileNamesGenerator generator; Poco::JSON::Object::Ptr initial_metadata_object; + String initial_metadata_path; class ParititonEncoder { @@ -150,6 +154,7 @@ static bool isCurrentManifestListAboveThreshold( Poco::JSON::Object::Ptr metadata_object, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, ContextPtr context, size_t threshold) { @@ -176,8 +181,11 @@ 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); + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, current_manifest_list_path, object_storage, + external_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 +196,7 @@ static Plan getPlan( const DataLakeStorageSettings & data_lake_settings, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, const String & write_format, ContextPtr context, CompressionMethod compression_method) @@ -230,21 +239,25 @@ static Plan getPlan( } } plan.initial_metadata_object = initial_metadata_object; + plan.initial_metadata_path = metadata_file_path; std::vector all_positional_delete_files; std::unordered_map> manifest_files; + std::unordered_set referenced_file_paths; for (const auto & snapshot : snapshots_info) { - auto manifest_list = getManifestList(object_storage, persistent_table_components, context, snapshot.manifest_list_path, log); + referenced_file_paths.insert(snapshot.manifest_list_path); + auto manifest_list = getManifestList(object_storage, persistent_table_components, context, snapshot.manifest_list_path, log, external_storages); for (const auto & manifest_file : manifest_list) { plan.manifest_list_to_manifest_files[snapshot.manifest_list_path].push_back(manifest_file.manifest_file_path); if (!plan.manifest_file_to_first_snapshot.contains(manifest_file.manifest_file_path)) plan.manifest_file_to_first_snapshot[manifest_file.manifest_file_path] = snapshot.snapshot_id; + referenced_file_paths.insert(manifest_file.manifest_file_path); 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}; 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), external_storages); if (!manifest_files.contains(manifest_file.manifest_file_path)) { @@ -253,38 +266,45 @@ 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); + referenced_file_paths.insert(pos_delete_file->parsed_entry->file_path_key); + } for (const auto & data_file : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { + 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, external_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)); - /// 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; + Iceberg::getIdentityPartitionColumnValues(*data_file, *persistent_table_components.schema_processor), + resolved_storage, + resolved_key); std::shared_ptr data_file_ptr; - if (!plan.path_to_data_file.contains(data_file_path)) + /// Deduplicate by resolved identity: different metadata paths may name the same file. + 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,9 +322,25 @@ 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()); } } + + for (const auto & raw_path : referenced_file_paths) + { + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.table_location, + raw_path.serialize(), + object_storage, + external_storages, + context, + persistent_table_components.path_resolver); + + if (!isObjectInTableDirectory( + storage_to_use, key_in_storage, object_storage, persistent_table_components.table_path)) + plan.external_files.push_back(raw_path); + } + plan.history = std::move(snapshots_info); plan.need_optimize = !all_positional_delete_files.empty(); return plan; @@ -318,7 +354,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 external_storages) { ColumnMapperPtr column_mapper; { @@ -351,10 +388,13 @@ static void writeDataFiles( format_settings, // todo make compaction using same FormatParserSharedResources std::make_shared(context->getSettingsRef(), 1), - context); + context, + path_resolver, + external_storages); + ObjectStoragePtr storage_to_use = data_file->data_object_info->getResolvedStorage(object_storage); RelativePathWithMetadata relative_path(data_file->data_object_info->getPath()); - auto read_buffer = createReadBuffer(relative_path, object_storage, context, getLogger("IcebergCompaction")); + auto read_buffer = createReadBuffer(relative_path, storage_to_use, context, getLogger("IcebergCompaction")); const Settings & settings = context->getSettingsRef(); auto parser_shared_resources = std::make_shared( @@ -429,7 +469,8 @@ static bool writeConsolidatedManifestFile( CompressionMethod compression_method, const DataLakeStorageSettings & data_lake_settings, std::shared_ptr catalog, - const StorageID & table_id) + const StorageID & table_id, + ExternalStorageCache & external_storages) { auto log = getLogger("IcebergManifestConsolidation"); @@ -627,7 +668,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, + external_storages); for (const auto & manifest_file : current_manifest_list) { @@ -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), + external_storages); for (const auto & data_file : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { @@ -892,6 +935,7 @@ static bool writeConsolidatedManifestFile( path_resolver, metadata_object, object_storage, + external_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, ExternalStorageCache & external_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, @@ -1142,7 +1187,7 @@ static void writeMetadataFiles( "Iceberg compaction does not support this table history: live manifest {} is first referenced " "by snapshot {} which produced no rewritten snapshot (e.g. a delete-only snapshot). " "The table is left unchanged.", - manifest_entry->path.serialize(), + manifest_entry->path, snapshot_id); std::vector data_files_vec(data_filenames.begin(), data_filenames.end()); @@ -1298,6 +1343,7 @@ static void writeMetadataFiles( path_resolver, metadata_object, object_storage, + external_storages, context, renamed_manifest_entries, new_snapshots[i].snapshot, @@ -1325,23 +1371,95 @@ static void writeMetadataFiles( } } -static std::vector getOldFiles(ObjectStoragePtr object_storage, const String & table_path) +struct OldFiles { - auto metadata_files = listFiles(*object_storage, table_path, "metadata", ""); - auto data_files = listFiles(*object_storage, table_path, "data", ""); + std::vector metadata_files; + std::vector data_files; +}; - for (auto && data_file : data_files) - metadata_files.push_back(data_file); +/// Remove the old head after other metadata, then its dependencies, so a partial failure leaves it readable. +static OldFiles getOldFiles(ObjectStoragePtr object_storage, const String & table_path, const String & head_metadata_path) +{ + constexpr std::string_view metadata_suffix = ".metadata.json"; + + auto metadata_prefix_files = listFiles(*object_storage, table_path, "metadata", ""); + std::erase_if(metadata_prefix_files, [](const String & file) + { + return file.ends_with("metadata/version-hint.text"); + }); - return metadata_files; + /// A configured metadata path may differ from the listing key; compare filenames. + const String head_file_name = std::filesystem::path(head_metadata_path).filename(); + auto is_head = [&](const String & key) { return std::filesystem::path(key).filename() == head_file_name; }; + + std::vector ordered; + ordered.reserve(metadata_prefix_files.size()); + auto append_matching = [&](const auto & predicate) + { + for (const auto & key : metadata_prefix_files) + if (predicate(key)) + ordered.push_back(key); + }; + + append_matching([&](const String & key) { return !is_head(key) && key.ends_with(metadata_suffix); }); + append_matching([&](const String & key) { return is_head(key); }); + append_matching([&](const String & key) { return !is_head(key) && !key.ends_with(metadata_suffix); }); + + return {std::move(ordered), listFiles(*object_storage, table_path, "data", "")}; } -static void clearOldFiles(ObjectStoragePtr object_storage, const std::vector & old_files) +/// The replacement is `v0.metadata.json`, so the old head remains current until higher versions are removed. +/// Stop on metadata deletion failures to preserve its dependencies. +static void clearOldFiles(ObjectStoragePtr object_storage, const OldFiles & old_files) { - for (const auto & metadata_file : old_files) + auto log = getLogger("IcebergCompaction"); + + for (const auto & key : old_files.metadata_files) + { + LOG_DEBUG(log, "Removing old file during compaction: {}", key); + try + { + object_storage->removeObjectIfExists(StoredObject(key)); + } + catch (Exception & e) + { + e.addMessage( + "while removing '{}', one of the metadata files the compaction replaced. The table is still at its " + "previous state and its data is intact; the compacted files written next to it are orphaned and the " + "old files that were not reached are left in place", + key); + throw; + } + } + + std::vector not_removed; + for (const auto & key : old_files.data_files) { - object_storage->removeObjectIfExists(StoredObject(metadata_file)); + LOG_DEBUG(log, "Removing old file during compaction: {}", key); + try + { + object_storage->removeObjectIfExists(StoredObject(key)); + } + catch (...) + { + tryLogCurrentException(log, "Failed to remove the old file " + key); + not_removed.push_back(key); + } } + + if (not_removed.empty()) + return; + + const size_t total = not_removed.size(); + if (total > 10) + not_removed.resize(10); + + throw Exception( + ErrorCodes::CANNOT_UNLINK, + "The table has been compacted, but {} of the files it replaced could not be removed and have to be deleted " + "by hand (the log names all of them): {}", + total, + fmt::join(not_removed, ", ")); } void compactIcebergManifests( @@ -1352,7 +1470,8 @@ void compactIcebergManifests( ContextPtr context_, const String & write_format, std::shared_ptr catalog, - const StorageID & table_id) + const StorageID & table_id, + ExternalStorageCache & external_storages) { auto log = getLogger("IcebergManifestCompaction"); LOG_INFO(log, "Starting manifest-only compaction for Iceberg table"); @@ -1399,7 +1518,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_, external_storages, context_, min_count_to_compact)) { LOG_INFO(log, "Manifest compaction is not needed (manifest list is within threshold {})", min_count_to_compact); @@ -1417,7 +1536,8 @@ void compactIcebergManifests( persistent_table_components.metadata_compression_method, data_lake_settings, catalog, - table_id)) + table_id, + external_storages)) { // Invalidate metadata cache so the next reader picks up the new state if (persistent_table_components.metadata_cache) @@ -1439,6 +1559,7 @@ void compactIcebergTable( IcebergHistory snapshots_info, const PersistentTableComponents & persistent_table_components, ObjectStoragePtr object_storage_, + std::shared_ptr external_storages_, const DataLakeStorageSettings & data_lake_settings, const std::optional & format_settings_, SharedHeader sample_block_, @@ -1452,12 +1573,32 @@ void compactIcebergTable( data_lake_settings, persistent_table_components, object_storage_, + *external_storages_, write_format, context_, persistent_table_components.metadata_compression_method); + + /// External files may be shared with other tables through `add_files`; compaction cannot safely delete them. + if (!plan.external_files.empty()) + { + constexpr size_t max_files_to_name = 10; + auto named = plan.external_files; + if (named.size() > max_files_to_name) + named.resize(max_files_to_name); + + throw Exception( + ErrorCodes::NOT_IMPLEMENTED, + "OPTIMIZE is not supported for Iceberg tables that reference files outside the table's base " + "directory (found {} such file(s), including: {}): compaction deletes the files it replaces, " + "and a file outside the table location may be owned or shared by something else. " + "Aborting to avoid deleting it", + plan.external_files.size(), + fmt::join(named, ", ")); + } + if (plan.need_optimize) { - auto old_files = getOldFiles(object_storage_, persistent_table_components.table_path); + auto old_files = getOldFiles(object_storage_, persistent_table_components.table_path, plan.initial_metadata_path); writeDataFiles( plan, sample_block_, @@ -1466,8 +1607,9 @@ 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); + persistent_table_components.metadata_compression_method, + external_storages_); + writeMetadataFiles(plan, persistent_table_components.path_resolver, object_storage_, *external_storages_, context_, sample_block_, write_format, persistent_table_components.table_path); clearOldFiles(object_storage_, old_files); } } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h index e6e3bb17dbbd0..6378891c1310c 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.h @@ -5,6 +5,8 @@ #include #include #include +#include +#include namespace DB::Iceberg @@ -24,6 +26,7 @@ void compactIcebergTable( IcebergHistory snapshots_info, const PersistentTableComponents & persistent_table_components, DB::ObjectStoragePtr object_storage_, + std::shared_ptr external_storages_, const DataLakeStorageSettings & data_lake_settings, const std::optional & format_settings_, DB::SharedHeader sample_block_, @@ -38,7 +41,8 @@ void compactIcebergManifests( DB::ContextPtr context_, const String & write_format, std::shared_ptr catalog, - const StorageID & table_id); + const StorageID & table_id, + ExternalStorageCache & external_storages); #endif } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp index e6c3c25f29093..022e731eea29a 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.cpp @@ -25,11 +25,15 @@ #include #include #include +#include +#include #include #include #include +#include + #include #include @@ -39,6 +43,7 @@ namespace DB namespace ErrorCodes { extern const int BAD_ARGUMENTS; +extern const int FILE_DOESNT_EXIST; extern const int LOGICAL_ERROR; extern const int LIMIT_EXCEEDED; extern const int NOT_IMPLEMENTED; @@ -142,6 +147,7 @@ Pipe resultToPipe(const ExpireSnapshotsResult & result) add("deleted_manifest_files_count", result.deleted_manifest_files_count); add("deleted_manifest_lists_count", result.deleted_manifest_lists_count); add("deleted_statistics_files_count", result.deleted_statistics_files_count); + add("failed_deletions_count", result.failed_deletions_count); add("dry_run", result.dry_run ? 1 : 0); const size_t rows = columns[0]->size(); @@ -344,16 +350,48 @@ std::pair, Strings> applyRetentionPolicy( // File collection helpers // --------------------------------------------------------------------------- +Iceberg::IcebergPathFromMetadata resolveFileIdentity( + const Iceberg::IcebergPathFromMetadata & path, + const ObjectStoragePtr & object_storage, + const PersistentTableComponents & persistent_table_components, + const ContextPtr & context, + ExternalStorageCache & external_storages) +{ + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.path_resolver.getTableLocation(), + path.serialize(), object_storage, external_storages, context, + persistent_table_components.path_resolver); + return Iceberg::IcebergPathFromMetadata::makeStorageIdentity(storage, key); +} + +Iceberg::IcebergPathFromMetadata resolveExistingFileIdentity( + const Iceberg::IcebergPathFromMetadata & path, + const ObjectStoragePtr & object_storage, + const PersistentTableComponents & persistent_table_components, + const ContextPtr & context, + ExternalStorageCache & external_storages) +{ + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.path_resolver.getTableLocation(), + path.serialize(), object_storage, external_storages, context, + persistent_table_components.path_resolver); + if (!storage->exists(StoredObject(key))) + throw Exception(ErrorCodes::FILE_DOESNT_EXIST, "Iceberg metadata file {} does not exist", path); + 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, + ExternalStorageCache & external_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, external_storages)); } void collectRetainedFiles( @@ -365,7 +403,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, + ExternalStorageCache & external_storages) { for (UInt32 i = 0; i < retained_snapshots->size(); ++i) { @@ -374,29 +413,66 @@ 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(resolveExistingFileIdentity( + manifest_list_path, object_storage, persistent_table_components, context, external_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, external_storages); for (const auto & manifest_entry : manifest_keys) { - retained_manifest_paths.insert(manifest_entry.manifest_file_path); + retained_manifest_paths.insert(resolveExistingFileIdentity( + manifest_entry.manifest_file_path, object_storage, persistent_table_components, context, external_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, external_storages); + collectAllFilePaths( + entries_handle, object_storage, persistent_table_components, context, external_storages, retained_data_file_paths); } } } -struct ExpiredFiles +enum class ExpiredFileRole : uint8_t +{ + DataFile, + PositionDelete, + EqualityDelete, + Manifest, + ManifestList, +}; + +struct ExpiredFile +{ + Iceberg::IcebergPathFromMetadata path; + ExpiredFileRole role; +}; + +struct ExpiredFileCounts { - std::vector all_paths; Int64 data_files = 0; Int64 position_delete_files = 0; Int64 equality_delete_files = 0; Int64 manifest_files = 0; Int64 manifest_lists = 0; + + Int64 & of(ExpiredFileRole role) + { + switch (role) + { + case ExpiredFileRole::DataFile: return data_files; + case ExpiredFileRole::PositionDelete: return position_delete_files; + case ExpiredFileRole::EqualityDelete: return equality_delete_files; + case ExpiredFileRole::Manifest: return manifest_files; + case ExpiredFileRole::ManifestList: return manifest_lists; + } + } +}; + +struct ExpiredFiles +{ + /// Delete leaves before their manifests, and manifests before their manifest lists. + std::vector all_paths; + ExpiredFileCounts planned; }; ExpiredFiles collectExpiredFiles( @@ -408,81 +484,125 @@ ExpiredFiles collectExpiredFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + ExternalStorageCache & external_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 = resolveExistingFileIdentity( + manifest_list_path, object_storage, persistent_table_components, context, external_storages); + } + catch (...) + { + tryLogCurrentException(log, fmt::format("Failed to resolve manifest list {}", manifest_list_path)); + throw; + } + 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, external_storages); } catch (...) { - LOG_WARNING(log, "Failed to read manifest list {}, skipping", manifest_list_path); - continue; + tryLogCurrentException(log, fmt::format("Failed to read manifest list {}", manifest_list_path)); + throw; } for (const auto & manifest_entry : manifest_keys) { - if (retained_manifest_paths.contains(manifest_entry.manifest_file_path)) + Iceberg::IcebergPathFromMetadata manifest_id; + try + { + manifest_id = resolveExistingFileIdentity( + manifest_entry.manifest_file_path, object_storage, persistent_table_components, context, external_storages); + } + catch (...) + { + tryLogCurrentException(log, fmt::format("Failed to resolve manifest file {}", manifest_entry.manifest_file_path)); + throw; + } + 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, external_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, external_storages))) { - result.all_paths.push_back(entry->parsed_entry->file_path_key); - ++result.data_files; + result.all_paths.push_back({entry->parsed_entry->file_path_key, ExpiredFileRole::DataFile}); + ++result.planned.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, external_storages))) { - result.all_paths.push_back(entry->parsed_entry->file_path_key); - ++result.position_delete_files; + result.all_paths.push_back({entry->parsed_entry->file_path_key, ExpiredFileRole::PositionDelete}); + ++result.planned.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, external_storages))) { - result.all_paths.push_back(entry->parsed_entry->file_path_key); - ++result.equality_delete_files; + result.all_paths.push_back({entry->parsed_entry->file_path_key, ExpiredFileRole::EqualityDelete}); + ++result.planned.equality_delete_files; } } catch (...) { - LOG_WARNING(log, "Failed to read manifest file {}, skipping", manifest_entry.manifest_file_path); - continue; + tryLogCurrentException(log, fmt::format("Failed to read manifest file {}", manifest_entry.manifest_file_path)); + throw; } - seen_expired_manifest_paths.insert(manifest_entry.manifest_file_path); - result.all_paths.push_back(manifest_entry.manifest_file_path); - ++result.manifest_files; + seen_expired_manifest_paths.insert(manifest_id); + result.all_paths.push_back({manifest_entry.manifest_file_path, ExpiredFileRole::Manifest}); + ++result.planned.manifest_files; } - seen_expired_manifest_list_paths.insert(manifest_list_path); - result.all_paths.push_back(manifest_list_path); - ++result.manifest_lists; + seen_expired_manifest_list_paths.insert(manifest_list_id); + result.all_paths.push_back({manifest_list_path, ExpiredFileRole::ManifestList}); + ++result.planned.manifest_lists; } return result; } +std::vector collectExternalExpiredFiles( + const std::vector & files, + ObjectStoragePtr object_storage, + const PersistentTableComponents & persistent_table_components, + ContextPtr context, + ExternalStorageCache & external_storages) +{ + std::vector external; + for (const auto & [file_path, _] : files) + { + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + persistent_table_components.path_resolver.getTableLocation(), file_path.serialize(), object_storage, + external_storages, context, persistent_table_components.path_resolver); + + if (!isObjectInTableDirectory( + storage_to_use, key_in_storage, object_storage, persistent_table_components.table_path)) + external.push_back(file_path); + } + return external; +} + // --------------------------------------------------------------------------- // Snapshot partitioning // --------------------------------------------------------------------------- @@ -630,29 +750,68 @@ void updateMetadataForExpiration( metadata->set(Iceberg::f_last_updated_ms, ms.count()); } -void deleteExpiredFiles( - const std::vector & files_to_delete, +struct DeletionOutcome +{ + ExpiredFileCounts deleted; + Int64 failed = 0; +}; + +/// Keep a manifest or manifest list if any dependency survived deletion, so surviving files remain discoverable. +DeletionOutcome deleteExpiredFiles( + const std::vector & files_to_delete, const Iceberg::IcebergPathResolver & path_resolver, ObjectStoragePtr object_storage, - LoggerPtr log) + ContextPtr context, + LoggerPtr log, + ExternalStorageCache & external_storages) { - for (const auto & file_path : files_to_delete) + DeletionOutcome outcome; + bool manifest_incomplete = false; + bool manifest_list_incomplete = false; + + for (const auto & [file_path, role] : files_to_delete) { - try + const bool keep_anchor + = (role == ExpiredFileRole::Manifest && manifest_incomplete) + || (role == ExpiredFileRole::ManifestList && manifest_list_incomplete); + + if (keep_anchor) { - object_storage->removeObjectIfExists(StoredObject(path_resolver.resolve(file_path))); - LOG_DEBUG(log, "Deleted expired file {}", file_path); + ++outcome.failed; + LOG_WARNING(log, "Keeping {}: a file it references could not be deleted", file_path); } - catch (...) + else { - LOG_WARNING(log, "Failed to delete file {}: {}", file_path, getCurrentExceptionMessage(false)); + try + { + auto [storage_to_use, key_in_storage] = resolveObjectStorageForPath( + path_resolver.getTableLocation(), file_path.serialize(), object_storage, external_storages, context, + path_resolver); + storage_to_use->removeObjectIfExists(StoredObject(key_in_storage)); + ++outcome.deleted.of(role); + LOG_DEBUG(log, "Deleted expired file {}", file_path); + } + catch (...) + { + ++outcome.failed; + LOG_WARNING(log, "Failed to delete file {}: {}", file_path, getCurrentExceptionMessage(false)); + + if (role != ExpiredFileRole::Manifest && role != ExpiredFileRole::ManifestList) + manifest_incomplete = true; + manifest_list_incomplete = true; + } } + + if (role == ExpiredFileRole::Manifest) + manifest_incomplete = false; + else if (role == ExpiredFileRole::ManifestList) + manifest_list_incomplete = false; } + return outcome; } } - // --------------------------------------------------------------------------- // Public: expireSnapshots orchestration // --------------------------------------------------------------------------- @@ -665,7 +824,8 @@ ExpireSnapshotsResult expireSnapshots( const PersistentTableComponents & persistent_table_components, const String & write_format, std::shared_ptr catalog, - const String & table_name) + const String & table_name, + ExternalStorageCache & external_storages) { auto common_path = persistent_table_components.table_path; if (!common_path.starts_with('/')) @@ -753,20 +913,41 @@ 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, + external_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, + external_storages); + + auto external_expired_files = collectExternalExpiredFiles( + expired_files.all_paths, object_storage, persistent_table_components, context, external_storages); + if (!external_expired_files.empty()) + { + constexpr size_t max_files_to_name = 10; + auto named = external_expired_files; + if (named.size() > max_files_to_name) + named.resize(max_files_to_name); + + throw Exception( + ErrorCodes::NOT_IMPLEMENTED, + "expire_snapshots is not supported for Iceberg tables that reference files outside the table's " + "base directory (found {} such expired file(s), including: {}): expiration deletes the files it " + "drops from the metadata, and a file outside the table location may be owned or shared by " + "something else. Aborting to avoid deleting it", + external_expired_files.size(), + fmt::join(named, ", ")); + } if (options.dry_run) { LOG_INFO(log, "Dry-run mode: skip metadata commit and file deletion"); return ExpireSnapshotsResult{ - .deleted_data_files_count = expired_files.data_files, - .deleted_position_delete_files_count = expired_files.position_delete_files, - .deleted_equality_delete_files_count = expired_files.equality_delete_files, - .deleted_manifest_files_count = expired_files.manifest_files, - .deleted_manifest_lists_count = expired_files.manifest_lists, + .deleted_data_files_count = expired_files.planned.data_files, + .deleted_position_delete_files_count = expired_files.planned.position_delete_files, + .deleted_equality_delete_files_count = expired_files.planned.equality_delete_files, + .deleted_manifest_files_count = expired_files.planned.manifest_files, + .deleted_manifest_lists_count = expired_files.planned.manifest_lists, .dry_run = true, }; } @@ -803,15 +984,22 @@ 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); - LOG_INFO(log, "Expired {} snapshots, deleted {} files", partition.expired_snapshot_ids.size(), expired_files.all_paths.size()); + auto outcome = deleteExpiredFiles( + expired_files.all_paths, persistent_table_components.path_resolver, object_storage, context, log, external_storages); + LOG_INFO( + log, + "Expired {} snapshots, deleted {} files, failed to delete {} files", + partition.expired_snapshot_ids.size(), + static_cast(expired_files.all_paths.size()) - outcome.failed, + outcome.failed); return ExpireSnapshotsResult{ - .deleted_data_files_count = expired_files.data_files, - .deleted_position_delete_files_count = expired_files.position_delete_files, - .deleted_equality_delete_files_count = expired_files.equality_delete_files, - .deleted_manifest_files_count = expired_files.manifest_files, - .deleted_manifest_lists_count = expired_files.manifest_lists, + .deleted_data_files_count = outcome.deleted.data_files, + .deleted_position_delete_files_count = outcome.deleted.position_delete_files, + .deleted_equality_delete_files_count = outcome.deleted.equality_delete_files, + .deleted_manifest_files_count = outcome.deleted.manifest_files, + .deleted_manifest_lists_count = outcome.deleted.manifest_lists, + .failed_deletions_count = outcome.failed, .dry_run = false, }; } @@ -822,7 +1010,6 @@ ExpireSnapshotsResult expireSnapshots( UNREACHABLE(); } - // --------------------------------------------------------------------------- // Public: executeExpireSnapshots (entry point from ALTER TABLE ... EXECUTE) // --------------------------------------------------------------------------- @@ -835,7 +1022,8 @@ Pipe executeExpireSnapshots( const PersistentTableComponents & persistent_components, const String & write_format, std::shared_ptr catalog, - const String & table_name) + const String & table_name, + ExternalStorageCache & external_storages) { auto parsed = makeSchema().parse(args); auto options = buildOptions(parsed); @@ -848,7 +1036,8 @@ Pipe executeExpireSnapshots( persistent_components, write_format, catalog, - table_name); + table_name, + external_storages); return resultToPipe(result); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h index e3a682c6c56b7..da6a8ce1e2a2c 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsExecute.h @@ -10,6 +10,8 @@ #include #include #include +#include +#include namespace DB::Iceberg { @@ -22,7 +24,8 @@ ExpireSnapshotsResult expireSnapshots( const PersistentTableComponents & persistent_table_components, const String & write_format, std::shared_ptr catalog, - const String & table_name); + const String & table_name, + ExternalStorageCache & external_storages); Pipe executeExpireSnapshots( const ASTPtr & args, @@ -32,7 +35,8 @@ Pipe executeExpireSnapshots( const PersistentTableComponents & persistent_components, const String & write_format, std::shared_ptr catalog, - const String & table_name); + const String & table_name, + ExternalStorageCache & external_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsTypes.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsTypes.h index f9efae0dceb9a..a49fcc58e5d1f 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsTypes.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExpireSnapshotsTypes.h @@ -20,6 +20,8 @@ struct ExpireSnapshotsResult Int64 deleted_manifest_files_count = 0; Int64 deleted_manifest_lists_count = 0; Int64 deleted_statistics_files_count = 0; + /// The committed metadata no longer references these files, so subsequent expiration will not retry them. + Int64 failed_deletions_count = 0; bool dry_run = false; }; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.cpp new file mode 100644 index 0000000000000..91ad302794840 --- /dev/null +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.cpp @@ -0,0 +1,976 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#if USE_AWS_S3 +#include +#include +#include +#include +#include +#endif +#if USE_AVRO +#include +#include +#endif +#if USE_AZURE_BLOB_STORAGE +#include +#endif +#if USE_HDFS +#include +#endif + +namespace DB +{ + +namespace ErrorCodes +{ + extern const int BAD_ARGUMENTS; + extern const int PATH_ACCESS_DENIED; +} + +#if USE_AWS_S3 +namespace S3AuthSetting +{ + extern const S3AuthSettingsString access_key_id; + extern const S3AuthSettingsString secret_access_key; + extern const S3AuthSettingsString session_token; + extern const S3AuthSettingsString region; + extern const S3AuthSettingsS3UriStyle uri_style; +} +#endif + +namespace Setting +{ +extern const SettingsBool object_storage_propagate_credentials_to_other_storages; +} + +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; +} + +/// Local and HDFS storages are rooted at the table directory; paths outside it need a separate storage. +bool isPrefixScopedScheme(const std::string & normalized_scheme) +{ + return normalized_scheme == "file" || normalized_scheme == "hdfs"; +} + +bool keyIsInsidePrefix(const std::string & base_key, const std::string & target_key) +{ + if (base_key.empty()) + return true; + + auto base = std::filesystem::path(base_key).lexically_normal().string(); + const auto target = std::filesystem::path(target_key).lexically_normal().string(); + if (base.empty()) + return true; + if (!base.ends_with('/')) + base.push_back('/'); + return target.starts_with(base); +} + +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 ""; +} + +std::optional sourceForScheme(const std::string & normalized_scheme) +{ + if (normalized_scheme == "s3" || normalized_scheme == "http" || normalized_scheme == "https") + return AccessTypeObjects::Source::S3; + if (normalized_scheme == "abfs") + return AccessTypeObjects::Source::AZURE; + if (normalized_scheme == "hdfs") + return AccessTypeObjects::Source::HDFS; + if (normalized_scheme == "file") + return AccessTypeObjects::Source::FILE; + return std::nullopt; +} + +#if USE_AWS_S3 +S3::URI parseMetadataS3URI(const std::string & path, const SchemeAuthorityKey & decomposed) +{ + const auto normalized_path = decomposed.scheme == "s3a" || decomposed.scheme == "s3n" + ? "s3://" + decomposed.authority + "/" + decomposed.key : path; + /// Metadata keys are already encoded; percent-decoding would change keys such as `col=12%3A00%3A00`. + return S3::URI(normalized_path, /*allow_archive_path_syntax*/ false, + /*keep_presigned_query_parameters*/ true, /*uri_style*/ S3UriStyle::AUTO, + /*enable_url_encoding*/ false); +} + +/// `s3a` and `s3n` inherit the endpoint; `gs`, `gcs` and `oss` select a provider. +std::string s3ProviderFamily(const std::string & scheme) +{ + const auto scheme_lowercase = Poco::toLower(scheme); + if (scheme_lowercase == "s3" || scheme_lowercase == "s3a" || scheme_lowercase == "s3n") + return "s3"; + if (scheme_lowercase == "gs" || scheme_lowercase == "gcs") + return "gcs"; + if (scheme_lowercase == "oss") + return "oss"; + return ""; +} + +std::string s3BaseProviderFamily(const std::string & base_scheme, const std::string & base_endpoint) +{ + if (auto family = s3ProviderFamily(base_scheme); !family.empty()) + return family; + if (base_endpoint.empty()) + return ""; + const auto endpoint_lowercase = Poco::toLower(base_endpoint); + if (endpoint_lowercase.contains("storage.googleapis.com")) + return "gcs"; + if (endpoint_lowercase.contains(".aliyuncs.com")) + return "oss"; + return "s3"; +} + +bool s3ProviderFamiliesCompatible(const std::string & base_family, const std::string & target_family) +{ + if (base_family.empty() || target_family.empty()) + return true; + return base_family == target_family; +} + +bool s3URIMatches( + const S3::URI & target_uri, + const std::string & base_bucket, + const std::string & base_endpoint, + const std::string & target_scheme_normalized, + bool provider_families_compatible) +{ + 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") && provider_families_compatible; + 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 +/// Cloud endpoints encode the account in the host; Azurite encodes it in the first path segment. +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.contains(".core.")) + 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 splitAbfsAuthority(const std::string & authority) +{ + auto at_pos = authority.find('@'); + if (at_pos == std::string::npos) + return {authority, ""}; + + std::string account_name = authority.substr(at_pos + 1); + if (auto suffix_pos = account_name.find('.'); suffix_pos != std::string::npos) + account_name = account_name.substr(0, suffix_pos); + return {authority.substr(0, at_pos), account_name}; +} + +void checkRemoteHostIsAllowed(const ContextPtr & context, const std::string & url) +{ + context->getGlobalContext()->getRemoteHostFilter().checkURL(Poco::URI(url)); +} + +std::pair getOrCreateStorage( + const std::string & cache_key, + const std::string & key_to_use, + ExternalStorageCache & external_storages, + const std::function & create_fn, + const std::string & supersedes_prefix = {}) +{ + std::lock_guard lock(external_storages.mutex); + if (auto it = external_storages.storages.find(cache_key); it != external_storages.storages.end()) + return {it->second, key_to_use}; + + ObjectStoragePtr storage = create_fn(cache_key); + + /// Evict older credential generations; in-flight readers retain their own `shared_ptr`. + if (!supersedes_prefix.empty()) + std::erase_if(external_storages.storages, [&](const auto & entry) { return entry.first.starts_with(supersedes_prefix); }); + + external_storages.storages.emplace(cache_key, storage); + return {storage, key_to_use}; +} + +std::pair getOrCreateStorageAndKey( + const std::string & cache_key, + const std::string & key_to_use, + const std::string & storage_type, + ExternalStorageCache & external_storages, + const ContextPtr & context, + std::function configure_fn) +{ + return getOrCreateStorage( + cache_key, + key_to_use, + external_storages, + [&](const std::string & storage_name) -> ObjectStoragePtr + { + Poco::AutoPtr cfg(new Poco::Util::MapConfiguration); + const std::string config_prefix = "object_storages." + storage_name; + + cfg->setString(config_prefix + ".object_storage_type", storage_type); + + configure_fn(*cfg, config_prefix); + + return ObjectStorageFactory::instance().create( + storage_name, + *cfg, + config_prefix, + context, + /*skip_access_check*/ true); + }); +} + +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 + +/// Validate RFC 3986 schemes: object keys can contain colons without being URIs. +bool isUriScheme(std::string_view candidate) +{ + if (candidate.empty() || !isAlphaASCII(candidate.front())) + return false; + + for (char c : candidate.substr(1)) + if (!isAlphaNumericASCII(c) && c != '+' && c != '-' && c != '.') + return false; + + return true; +} + +} + +SchemeAuthorityKey::SchemeAuthorityKey(const std::string & uri) +{ + if (uri.empty()) + return; + + if (auto scheme_sep = uri.find("://"); + scheme_sep != std::string_view::npos && isUriScheme(std::string_view(uri).substr(0, scheme_sep))) + { + scheme = Poco::toLower(uri.substr(0, scheme_sep)); + auto rest = uri.substr(scheme_sep + 3); + + auto slash = rest.find('/'); + if (slash == std::string_view::npos) + { + authority = std::string(rest); + key = "/"; + return; + } + authority = std::string(rest.substr(0, slash)); + if (scheme == "file") + key = std::string(rest.substr(slash)); + else + key = std::string(rest.substr(++slash)); + return; + } + + if (auto colon = uri.find(':'); + colon != std::string_view::npos && colon > 0 && isUriScheme(std::string_view(uri).substr(0, colon))) + { + auto after_colon = uri.substr(colon + 1); + + if (!after_colon.empty() && after_colon[0] == '/') + { + scheme = Poco::toLower(uri.substr(0, colon)); + authority = ""; + key = std::string(after_colon); + return; + } + } + + key = std::string(uri); +} +#if USE_AVRO +namespace +{ +struct PathAccessCheck +{ + DB::AccessType access_type = DB::AccessType::READ; + bool granted = true; + bool openable = true; +}; +} + +static std::optional> tryResolveObjectStorageForPathImpl( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const DB::ContextPtr & context, + PathAccessCheck * access_check) +{ + if (!isAbsolutePath(path)) + return std::nullopt; + + auto check_local_path_inside_user_files = [&](const std::string & local_path) + { + /// `clickhouse-local` intentionally leaves `user_files_path` unrestricted. + 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(); + + /// The lexical check permits a symlink at the leaf; the canonical check prevents escapes through parent symlinks. + if (user_files_path.empty() + || !fileOrSymlinkPathStartsWith(target_path.string(), user_files_path.string()) + || !pathStartsWith(target_path.string(), user_files_path.string())) + { + if (access_check) + access_check->openable = false; + else + throw DB::Exception( + DB::ErrorCodes::PATH_ACCESS_DENIED, + "File URI '{}' is outside of allowed `user_files` path '{}'", + local_path, + user_files_path.string()); + } + }; + + /// Grants must match the file that is actually opened, so symlinks are resolved; `user_files` keeps its configured spelling. + auto local_path_for_grant_check = [&](const std::string & local_path) + { + const auto user_files_path = std::filesystem::path(context->getUserFilesPath()).lexically_normal(); + if (context->getApplicationType() != Context::ApplicationType::SERVER || user_files_path.empty()) + return std::filesystem::weakly_canonical(local_path).string(); + return (user_files_path / std::filesystem::relative(local_path, user_files_path)).lexically_normal().string(); + }; + + SchemeAuthorityKey table_location_decomposed{table_location}; + SchemeAuthorityKey target_decomposed{path}; + + if (target_decomposed.scheme.empty() && target_decomposed.key.starts_with('/')) + { + const bool base_is_local = base_storage->getType() == ObjectStorageType::Local; + + if (!base_is_local || keyIsInsidePrefix(table_location_decomposed.key, target_decomposed.key)) + return std::nullopt; + + target_decomposed.scheme = "file"; + } + + const std::string base_scheme_normalized = normalizeScheme(table_location_decomposed.scheme); + const std::string target_scheme_normalized = normalizeScheme(target_decomposed.scheme); + + if (target_scheme_normalized == "file") + { + /// RFC 8089 treats `localhost` as an empty authority. + std::string remote_authority; + if (!target_decomposed.authority.empty() && Poco::toLower(target_decomposed.authority) != "localhost") + remote_authority = target_decomposed.authority; + target_decomposed.authority.clear(); + + if (!remote_authority.empty()) + { + if (access_check) + access_check->openable = false; + else + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "File URI '{}' refers to host '{}', but `file://` paths are always read from the local " + "filesystem. Only an empty authority (`file:///path`) or `localhost` is supported", + path, + remote_authority); + } + + check_local_path_inside_user_files(target_decomposed.key); + } + + auto check_access = [&] + { + const auto source = sourceForScheme(target_scheme_normalized); + if (!source) + { + if (access_check) + { + access_check->openable = false; + return; + } + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, "Unsupported storage scheme '{}' in path '{}'", target_scheme_normalized, path); + } + + const std::string uri_to_check = target_scheme_normalized == "file" ? local_path_for_grant_check(target_decomposed.key) : path; + if (access_check) + access_check->granted = context->getAccess()->isGrantedWithFilter( + access_check->access_type, AccessTypeObjects::toStringSource(*source), uri_to_check); + else + context->getAccess()->checkAccessWithFilter( + DB::AccessType::READ, AccessTypeObjects::toStringSource(*source), uri_to_check); + }; + + #if USE_AWS_S3 + if (target_scheme_normalized == "s3" || target_scheme_normalized == "https" || target_scheme_normalized == "http") + { + auto s3_uri = parseMetadataS3URI(path, target_decomposed); + + std::string key_to_use = s3_uri.key; + + std::string base_storage_endpoint; + if (base_storage->getType() == ObjectStorageType::S3) + base_storage_endpoint = base_storage->getDescription(); + + const bool provider_families_compatible = s3ProviderFamiliesCompatible( + s3BaseProviderFamily(table_location_decomposed.scheme, base_storage_endpoint), + s3ProviderFamily(target_decomposed.scheme)); + + /// A table copied without rewriting metadata still uses its old `location`; re-root paths inside it. + if (base_scheme_normalized == "s3" || base_scheme_normalized == "https" || base_scheme_normalized == "http") + { + auto base_s3_uri = parseMetadataS3URI(table_location, table_location_decomposed); + + if (s3URIMatches(s3_uri, base_s3_uri.bucket, base_s3_uri.endpoint, target_scheme_normalized, provider_families_compatible) + && keyIsInsidePrefix(base_s3_uri.key, key_to_use)) + return std::nullopt; + } + + 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, provider_families_compatible)) + { + check_access(); + if (access_check != nullptr) + return std::nullopt; + return std::make_pair(base_storage, key_to_use); + } + } + } + + const bool endpoint_explicit = (target_decomposed.scheme == "http" || target_decomposed.scheme == "https"); + + std::string endpoint_to_use; + + S3UriStyle endpoint_uri_style = S3UriStyle::AUTO; + + 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) + { + return s3_uri.endpoint.substr(0, scheme_end + 3) + s3_uri.bucket + "." + s3_uri.endpoint.substr(scheme_end + 3); + } + + endpoint_uri_style = S3UriStyle::PATH; + return s3_uri.endpoint + "/" + s3_uri.bucket; + }; + + if (endpoint_explicit || !provider_families_compatible) + { + 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.contains(".s3.") && base_endpoint.contains(".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); + + 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 + { + 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; + endpoint_uri_style = S3UriStyle::PATH; + } + } + + if (endpoint_to_use.empty()) + endpoint_to_use = make_endpoint_with_bucket(); + } + + checkRemoteHostIsAllowed(context, endpoint_to_use); + check_access(); + if (access_check != nullptr) + return std::nullopt; + + /// Include session restrictions in the cache identity to prevent reuse of a privileged client. + const bool restricts_server_credentials = context->shouldRestrictUserQueryS3Credentials(); + + /// Propagating credentials must not bypass the session restriction on server-managed credentials. + const bool propagate_creds = !restricts_server_credentials + && context->getSettingsRef()[Setting::object_storage_propagate_credentials_to_other_storages]; + + bool reuse_base_credentials = false; + if (base_storage->getType() == ObjectStorageType::S3) + reuse_base_credentials = propagate_creds + || (provider_families_compatible + && (!endpoint_explicit || sameEndpoint(base_storage->getDescription(), s3_uri.endpoint))); + + String access_key_id; + String secret_access_key; + String session_token; + String region; + std::optional base_s3_settings; + if (reuse_base_credentials) + { + if (auto s3_storage = std::dynamic_pointer_cast(base_storage)) + { + base_s3_settings.emplace(s3_storage->getS3Settings()); + + 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(); + } + } + } + + std::string storage_cache_key = "s3://" + s3_uri.bucket + "@" + endpoint_to_use + + "#propagate=" + (propagate_creds ? "1" : "0") + + "#restrict=" + (restricts_server_credentials ? "1" : "0"); + + /// Credential rotation makes older cache entries unreachable; evict them by their common prefix. + 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 getOrCreateStorage( + storage_cache_key, + key_to_use, + external_storages, + [&](const std::string & storage_name) -> ObjectStoragePtr + { + S3Settings s3_settings; + if (base_s3_settings) + s3_settings = S3Settings(*base_s3_settings); + else + { + Poco::AutoPtr cfg(new Poco::Util::MapConfiguration); + const std::string config_prefix = "object_storages." + storage_name; + cfg->setString(config_prefix + ".endpoint", endpoint_to_use); + + s3_settings.loadFromConfigForObjectStorage( + *cfg, config_prefix, context->getSettingsRef(), Poco::URI(endpoint_to_use).getScheme(), + /*validate_settings*/ true); + } + + /// Use the credential generation captured in the cache key. + if (!access_key_id.empty()) + s3_settings.auth_settings[S3AuthSetting::access_key_id] = access_key_id; + if (!secret_access_key.empty()) + s3_settings.auth_settings[S3AuthSetting::secret_access_key] = secret_access_key; + if (!session_token.empty()) + s3_settings.auth_settings[S3AuthSetting::session_token] = session_token; + if (!region.empty()) + s3_settings.auth_settings[S3AuthSetting::region] = region; + + s3_settings.auth_settings[S3AuthSetting::uri_style] = endpoint_uri_style; + + S3::URI storage_uri( + endpoint_to_use, /*allow_archive_path_syntax*/ false, /*keep_presigned_query_parameters*/ true, + s3_settings.auth_settings[S3AuthSetting::uri_style]); + + if (!storage_uri.key.empty() && !storage_uri.key.ends_with('/')) + storage_uri.key.push_back('/'); + + /// `for_disk_s3 = false` enforces session restrictions on server credentials. + auto client = getClient(endpoint_to_use, s3_settings, context, /*for_disk_s3*/ false, storage_name); + + return std::make_shared( + std::move(client), + std::make_unique(std::move(s3_settings)), + storage_uri, + S3Capabilities{}, + createObjectStorageKeyGeneratorByPrefix(storage_uri.key), + storage_name); + }, + supersedes_prefix); + } + #endif + + #if USE_HDFS + if (target_scheme_normalized == "hdfs") + { + bool use_base_storage = false; + + if (base_storage->getType() == ObjectStorageType::HDFS) + { + if (auto hdfs_storage = std::dynamic_pointer_cast(base_storage)) + { + const std::string base_url = hdfs_storage->getDescription(); + 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; + + std::string target_endpoint = target_scheme_normalized + "://" + target_decomposed.authority; + + const bool inside_table_prefix = keyIsInsidePrefix(table_location_decomposed.key, target_decomposed.key); + + if (base_endpoint == target_endpoint && inside_table_prefix) + use_base_storage = true; + + if (!use_base_storage && base_scheme_normalized == "hdfs" && inside_table_prefix) + { + 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 + + if (base_scheme_normalized == target_scheme_normalized && table_location_decomposed.authority == target_decomposed.authority) + { + const bool inside_table_location = keyIsInsidePrefix(table_location_decomposed.key, target_decomposed.key); + + /// Prefix-scoped backends accept full keys; other backends need paths inside `location` re-rooted. + if (isPrefixScopedScheme(target_scheme_normalized)) + { + if (inside_table_location) + return std::make_pair(base_storage, target_decomposed.key); + } + else if (inside_table_location) + return std::nullopt; + else + { + check_access(); + if (access_check != nullptr) + return std::nullopt; + return std::make_pair(base_storage, target_decomposed.key); + } + } + + const std::string type_for_factory = factoryTypeForScheme(target_scheme_normalized); + if (type_for_factory.empty()) + { + if (access_check) + { + access_check->openable = false; + return std::nullopt; + } + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Unsupported storage scheme '{}' in path '{}'", target_scheme_normalized, path); + } + + /// Each local storage is rooted at one parent directory, so the directory must be part of its cache key. + std::string file_dir_path; + 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; + } + +#if USE_AZURE_BLOB_STORAGE + if (target_scheme_normalized == "abfs" && base_storage->getType() == ObjectStorageType::Azure) + { + if (auto azure_storage = std::dynamic_pointer_cast(base_storage)) + { + const auto [container_name, account_name] = splitAbfsAuthority(target_decomposed.authority); + const auto & conn_params = azure_storage->getConnectionParameters(); + + 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); + + checkRemoteHostIsAllowed(context, conn_params.getConnectionURL()); + check_access(); + if (access_check != nullptr) + return std::nullopt; + + /// Clone connection parameters to preserve SAS and workload identity authentication, which config cannot express. + return getOrCreateStorage( + cache_key, + target_decomposed.key, + external_storages, + [&](const std::string & storage_name) -> ObjectStoragePtr + { + auto params = conn_params; + params.endpoint.container_name = container_name; + params.endpoint.prefix.clear(); + + auto client = AzureBlobStorage::getContainerClient(params, /*readonly*/ true); + + return std::make_shared( + storage_name, + params.auth_method, + std::move(client), + AzureBlobStorage::getRequestSettings(context->getSettingsRef()), + params, + params.getContainer(), + /// `getServiceEndpoint` would expose the SAS token in the storage description. + params.getConnectionURL(), + /*common_key_prefix*/ ""); + }); + } + } +#endif + + if (!target_decomposed.authority.empty()) + checkRemoteHostIsAllowed(context, target_scheme_normalized + "://" + target_decomposed.authority + "/"); + + check_access(); + if (access_check != nullptr) + return std::nullopt; + + return getOrCreateStorageAndKey( + cache_key, + target_decomposed.key, + type_for_factory, + external_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") + { + const auto [container_name, account_name] = splitAbfsAuthority(target_decomposed.authority); + + cfg.setString(config_prefix + ".container_name", container_name); + if (!account_name.empty()) + cfg.setString(config_prefix + ".account_name", account_name); + } + else if (target_scheme_normalized == "hdfs") + { + auto endpoint = target_scheme_normalized + "://" + target_decomposed.authority; + if (!endpoint.empty() && endpoint.back() != '/') + endpoint.push_back('/'); + cfg.setString(config_prefix + ".endpoint", endpoint); + } + }); +} + +static std::optional> tryResolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const DB::ContextPtr & context) +{ + return tryResolveObjectStorageForPathImpl(table_location, path, base_storage, external_storages, context, /*access_check*/ nullptr); +} + +static PathAccessCheck checkPathAccess( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + const DB::ContextPtr & context, + DB::AccessType access_type) +{ + ExternalStorageCache unused_storages; + PathAccessCheck access_check{.access_type = access_type}; + tryResolveObjectStorageForPathImpl(table_location, path, base_storage, unused_storages, context, &access_check); + return access_check; +} + +bool isPathReadGranted( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + const DB::ContextPtr & context) +{ + return checkPathAccess(table_location, path, base_storage, context, DB::AccessType::READ).granted; +} + +bool isPathWriteGranted( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + const DB::ContextPtr & context) +{ + return checkPathAccess(table_location, path, base_storage, context, DB::AccessType::WRITE).granted; +} + +bool isPathReadable( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + const DB::ContextPtr & context) +{ + const auto access_check = checkPathAccess(table_location, path, base_storage, context, DB::AccessType::READ); + return access_check.granted && access_check.openable; +} + +std::pair resolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const DB::ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const DB::ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver) +{ + if (auto resolved = tryResolveObjectStorageForPath(table_location, path, base_storage, external_storages, context)) + return *resolved; + return {base_storage, path_resolver.resolve(Iceberg::IcebergPathFromMetadata::deserialize(path))}; +} + +bool requiresAbsolutePathForCluster( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver) +{ + auto [storage, key] = resolveObjectStorageForPath(table_location, path, base_storage, external_storages, context, path_resolver); + return storage != base_storage || key != SchemeAuthorityKey(path).key; +} + +bool isObjectInTableDirectory( + const ObjectStoragePtr & storage, + const std::string & key, + const ObjectStoragePtr & table_storage, + const std::string & table_path) +{ + if (storage != table_storage || !key.starts_with(table_path)) + return false; + return table_path.empty() || table_path.ends_with('/') + || (key.size() > table_path.size() && key[table_path.size()] == '/'); +} + +void resolveObjectStorageFromDataLakeMetadata( + const ObjectInfoPtr & object, + const std::string & table_location, + const ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const ContextPtr & context) +{ + auto iceberg_info = std::dynamic_pointer_cast(object); + if (!iceberg_info || iceberg_info->tryGetResolvedStorage()) + return; + + auto metadata_path = iceberg_info->getPathInDataLakeMetadata(); + if (!metadata_path) + return; + + /// Base-storage tasks already carry the key resolved by the coordinator. + if (auto resolved = tryResolveObjectStorageForPath(table_location, *metadata_path, base_storage, external_storages, context); + resolved && resolved->first != base_storage) + { + iceberg_info->setResolvedStorage(resolved->first); + iceberg_info->relative_path_with_metadata.relative_path = resolved->second; + } +} + +#endif + +} diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.h new file mode 100644 index 0000000000000..8962023b4fd85 --- /dev/null +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ExternalPathResolver.h @@ -0,0 +1,92 @@ +#pragma once + +#include "config.h" + +#include +#include + +#include +#include +#include +#include +#include + +namespace DB +{ + +struct ObjectInfo; +using ObjectInfoPtr = std::shared_ptr; + +struct SchemeAuthorityKey +{ + explicit SchemeAuthorityKey(const std::string & uri); + + std::string scheme; + std::string authority; + std::string key; +}; + +#if USE_AVRO + +namespace Iceberg { class IcebergPathResolver; } + +struct ExternalStorageCache +{ + mutable std::mutex mutex; + std::map storages; +}; + +/// Checks grants without opening storage. Unreadable paths remain visible in `system.iceberg_files`. +bool isPathReadGranted( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + const ContextPtr & context); + +/// Checks the `WRITE` grant the same way, for deleting a file referenced by the table. +bool isPathWriteGranted( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + const ContextPtr & context); + +/// Also checks path restrictions, for metadata-only answers that must not bypass a failing scan. +bool isPathReadable( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + const ContextPtr & context); + +std::pair resolveObjectStorageForPath( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver); + +/// Legacy workers open the transmitted object key directly, without resolving it against the table location. +bool requiresAbsolutePathForCluster( + const std::string & table_location, + const std::string & path, + const ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const ContextPtr & context, + const Iceberg::IcebergPathResolver & path_resolver); + +bool isObjectInTableDirectory( + const ObjectStoragePtr & storage, + const std::string & key, + const ObjectStoragePtr & table_storage, + const std::string & table_path); + +void resolveObjectStorageFromDataLakeMetadata( + const ObjectInfoPtr & object, + const std::string & table_location, + const ObjectStoragePtr & base_storage, + ExternalStorageCache & external_storages, + const ContextPtr & context); + +#endif + +} diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.cpp index 8adf207896e2d..fd399f4667d36 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,18 +12,23 @@ #include #include +#include +#include #include #include #include +#include +#include namespace DB::ErrorCodes { extern const int NOT_IMPLEMENTED; extern const int UNKNOWN_PROTOCOL; +extern const int ICEBERG_SPECIFICATION_VIOLATION; +extern const int PROTOCOL_VERSION_MISMATCH; } - using namespace DB::Iceberg; namespace DB::Iceberg @@ -72,10 +77,12 @@ 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, data_manifest_file_entry_->resolved_schema_id, @@ -89,7 +96,9 @@ 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_)) { + chassert(resolved_key_.empty() == (resolved_storage == nullptr)); } IcebergDataObjectInfo::IcebergDataObjectInfo(const RelativePathWithMetadata & path_) @@ -108,13 +117,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 external_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, external_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, external_storages); } void IcebergDataObjectInfo::addPositionDeleteObject(Iceberg::ProcessedManifestFileEntryPtr position_delete_object, const String & resolved_storage_path) @@ -146,11 +157,58 @@ bool hasIcebergPositionDeletes(const ObjectInfoPtr & object_info) return iceberg && !iceberg->info.position_deletes_objects.empty(); } +std::optional IcebergDataObjectInfo::getExternalLocalPath() const +{ + if (!info.requires_external_storage) + return std::nullopt; + + /// A scheme-less absolute path denotes a local file only on local storage; otherwise it is a bucket key. + const bool absolute_is_local = resolved_storage && resolved_storage->getType() == ObjectStorageType::Local; + + auto is_local = [&](const String & path) + { + SchemeAuthorityKey decomposed{path}; + return decomposed.scheme == "file" + || (absolute_is_local && decomposed.scheme.empty() && decomposed.key.starts_with('/')); + }; + + if (auto metadata_path = getPathInDataLakeMetadata(); metadata_path && is_local(*metadata_path)) + return metadata_path; + + for (const auto & delete_object : info.position_deletes_objects) + if (is_local(delete_object.file_path)) + return delete_object.file_path; + for (const auto & delete_object : info.equality_deletes_objects) + if (is_local(delete_object.file_path)) + return delete_object.file_path; + + return std::nullopt; +} + #endif -void IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version) const +void IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version, UInt64 antalya_protocol_version) const { checkVersion(protocol_version); + + if (requires_external_storage && antalya_protocol_version < DBMS_ANTALYA_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 Antalya protocol version >= {}, but has {}. ", + data_object_file_path_key, + DBMS_ANTALYA_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH, + antalya_protocol_version); + } + + auto path_for_protocol = [&](const String & path) -> String + { + if (antalya_protocol_version < DBMS_ANTALYA_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH) + return SchemeAuthorityKey(path).key; + return path; + }; + writeStringBinary(data_object_file_path_key.serialize(), out); writeVarInt(underlying_format_read_schema_id, out); writeVarInt(schema_id_relevant_to_iterator, out); @@ -160,12 +218,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 { @@ -177,7 +235,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()) diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h index c5062f74ecc5f..7af97ed16291e 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDataObjectInfo.h @@ -13,7 +13,6 @@ #include - namespace DB::Iceberg { @@ -26,6 +25,7 @@ void requireParquetDataFileForRowDeletes(const String & file_format, std::string struct IcebergObjectSerializableInfo { + /// Preserve the manifest spelling for `_path` and task identity; it may differ from the resolved storage key. IcebergPathFromMetadata data_object_file_path_key; Int32 underlying_format_read_schema_id{}; Int32 schema_id_relevant_to_iterator{}; @@ -39,7 +39,10 @@ struct IcebergObjectSerializableInfo std::optional file_size_in_bytes; std::vector> identity_partition_columns; - void serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version) const; + bool requires_external_storage = false; + + /// `antalya_protocol_version` is the version negotiated with the worker, see `AntalyaProtocol.h`. + void serializeForClusterFunctionProtocol(WriteBuffer & out, size_t protocol_version, UInt64 antalya_protocol_version) const; void deserializeForClusterFunctionProtocol(ReadBuffer & in, size_t protocol_version); private: @@ -51,9 +54,10 @@ struct IcebergObjectSerializableInfo #if USE_AVRO #include +#include +#include #include - namespace DB { @@ -66,14 +70,14 @@ struct IcebergDataObjectInfo : public ObjectInfo, std::enable_shared_from_this; - /// Full path to the data object file - /// It is used to filter position deletes objects by data file path. - /// It is also used to create a filter for the data object in the position delete transform. + /// Position deletes match the data path exactly as written in the manifest. explicit 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_); + 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_); @@ -83,7 +87,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 external_storages); std::optional getFileFormat() const override { return info.file_format; } @@ -96,11 +102,32 @@ struct IcebergDataObjectInfo : public ObjectInfo, std::enable_shared_from_this getPathInDataLakeMetadata() const override + { + if (info.data_object_file_path_key.empty()) + return std::nullopt; + return info.data_object_file_path_key.serialize(); + } + + std::optional getExternalLocalPath() const override; + + ObjectStoragePtr getResolvedStorage(const ObjectStoragePtr & default_storage) const override + { + return resolved_storage ? resolved_storage : default_storage; + } + + ObjectStoragePtr tryGetResolvedStorage() 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); std::shared_ptr clone() const override { return std::make_shared(*this); } Iceberg::IcebergObjectSerializableInfo info; + +private: + 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 8250cf799d985..5a5cc1ed340d1 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp @@ -16,7 +16,6 @@ #include #include - #include #include #include @@ -48,6 +47,8 @@ #include #include #include +#include +#include #include @@ -59,7 +60,6 @@ #include #include - namespace ProfileEvents { extern const Event IcebergIteratorInitializationMicroseconds; @@ -69,7 +69,6 @@ extern const Event IcebergMinMaxNonPrunedDeleteFiles; extern const Event IcebergMinMaxPrunedDeleteFiles; }; - namespace DB { @@ -84,7 +83,6 @@ extern const SettingsBool use_iceberg_partition_pruning; extern const SettingsNonZeroUInt64 iceberg_delete_manifest_decode_concurrency; }; - using namespace Iceberg; namespace @@ -253,7 +251,7 @@ 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, *external_storages); }; prefetched_manifest = PrefetchedManifest{index, prefetch_runner(std::move(fetch), Priority{})}; return; @@ -274,13 +272,15 @@ SingleThreadIcebergKeysIterator::SingleThreadIcebergKeysIterator( const ActionsDAG * filter_dag_, Iceberg::TableStateSnapshotPtr table_snapshot_, Iceberg::IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components_) + PersistentTableComponents persistent_components_, + std::shared_ptr external_storages_) : object_storage(object_storage_) , filter_dag(makeManifestFilterDag(filter_dag_, local_context_)) , local_context(local_context_) , table_snapshot(table_snapshot_) , data_snapshot(data_snapshot_) , persistent_components(persistent_components_) + , external_storages(std::move(external_storages_)) , log(getLogger("IcebergIterator")) , manifest_file_content_type(manifest_file_content_type_) , prefetch_runner(threadPoolCallbackRunnerUnsafe( @@ -297,7 +297,8 @@ IcebergIterator::IcebergIterator( IDataLakeMetadata::FileProgressCallback callback_, Iceberg::TableStateSnapshotPtr table_snapshot_, Iceberg::IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components_) + PersistentTableComponents persistent_components_, + std::shared_ptr external_storages_) : logger(getLogger("IcebergIterator")) , object_storage(std::move(object_storage_)) , local_context(local_context_) @@ -312,10 +313,12 @@ IcebergIterator::IcebergIterator( filter_dag_, table_snapshot_, data_snapshot_, - persistent_components_) + persistent_components_, + external_storages_) , blocking_queue(100) , callback(std::move(callback_)) , table_schema_id(table_snapshot_->schema_id) + , external_storages(external_storages_) { /// Decoding any manifest reads settings from the context, so a missing one is fatal either way. if (!local_context) @@ -422,7 +425,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, + *external_storages); auto manifest_file_iterator = Iceberg::ManifestFileIterator::create( manifest_file_cacheable_part.deserializer, @@ -480,12 +484,21 @@ 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, *external_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); const auto & data_file_path = object_info->info.data_object_file_path_key; bool has_deletion_vector = false; @@ -521,9 +534,14 @@ ObjectInfoPtr IcebergIterator::next(size_t) /// I/O / CRC / roaring materialization happen here, then excluded_rows is sent on /// the wire per task. Workers apply the bitmap and do not re-read the deletion-vector /// object (Puffin or Delta `.bin`). + auto [deletion_vector_storage, deletion_vector_key] = resolveObjectStorageForPath( + persistent_components.table_location, parsed_entry->file_path_key.serialize(), + object_storage, *external_storages, local_context, + persistent_components.path_resolver); + auto excluded_rows = Iceberg::loadDeletionVector( - object_storage, - persistent_components.path_resolver.resolve(parsed_entry->file_path_key), + deletion_vector_storage, + deletion_vector_key, parsed_entry->content_offset.value(), parsed_entry->content_size_in_bytes.value(), data_file_path, @@ -579,8 +597,7 @@ ObjectInfoPtr IcebergIterator::next(size_t) data_file_path, 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)); + object_info->addPositionDeleteObject(position_delete, position_delete->parsed_entry->file_path_key.serialize()); } } } @@ -598,7 +615,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()) @@ -617,6 +634,30 @@ ObjectInfoPtr IcebergIterator::next(size_t) manifest_file_entry->parsed_entry->columns_infos, manifest_file_entry->parsed_entry->value_bounds, data_file_path.serialize())); + + /// Only distributed tasks need compatibility checks, which parse every delete-file URI. + if (tasks_go_to_other_replicas && !object_info->info.requires_external_storage) + { + auto needs_absolute_path_protocol = [&](const String & file_path) + { + return requiresAbsolutePathForCluster( + persistent_components.table_location, file_path, object_storage, *external_storages, local_context, + persistent_components.path_resolver); + }; + 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; + }; + + /// Data paths already travel as resolved keys at every protocol version; only delete paths need this check. + 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 6b27c8a37c811..d68edd90f2da2 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.h @@ -28,6 +28,8 @@ #include #include #include +#include +#include namespace DB { @@ -45,7 +47,8 @@ class SingleThreadIcebergKeysIterator const ActionsDAG * filter_dag_, TableStateSnapshotPtr table_snapshot_, IcebergDataSnapshotPtr data_snapshot_, - PersistentTableComponents persistent_components); + PersistentTableComponents persistent_components, + std::shared_ptr external_storages_); ~SingleThreadIcebergKeysIterator(); @@ -60,6 +63,7 @@ class SingleThreadIcebergKeysIterator Iceberg::TableStateSnapshotPtr table_snapshot; Iceberg::IcebergDataSnapshotPtr data_snapshot; PersistentTableComponents persistent_components; + std::shared_ptr external_storages; LoggerPtr log; size_t manifest_file_index = 0; @@ -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 external_storages_); ObjectInfoPtr next(size_t) override; @@ -121,8 +126,8 @@ class IcebergIterator : public IObjectIterator std::exception_ptr exception; std::mutex exception_mutex; Int32 table_schema_id; + std::shared_ptr external_storages; }; } - #endif diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp index 9beb40dcc89a4..4d6355d9908ca 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -84,6 +85,7 @@ #include #include #include +#include #include #include #include @@ -120,9 +122,11 @@ extern const DataLakeStorageSettingsNonZeroUInt64 iceberg_format_version; namespace ErrorCodes { +extern const int ACCESS_DENIED; extern const int BAD_ARGUMENTS; extern const int LOGICAL_ERROR; extern const int NOT_IMPLEMENTED; +extern const int PATH_ACCESS_DENIED; extern const int ICEBERG_SPECIFICATION_VIOLATION; extern const int S3_ERROR; extern const int TABLE_ALREADY_EXISTS; @@ -131,6 +135,7 @@ extern const int FILE_ALREADY_EXISTS; extern const int METADATA_MISMATCH; extern const int UNFINISHED; extern const int INCORRECT_DATA; +extern const int FILE_DOESNT_EXIST; } namespace Setting @@ -160,6 +165,7 @@ extern const SettingsUInt64 iceberg_insert_max_bytes_in_data_file; extern const SettingsUInt64 iceberg_max_number_datafiles_to_compact; extern const SettingsUInt64 iceberg_data_file_size_lower_threshold_compaction; extern const SettingsUInt64 iceberg_data_file_size_upper_threshold_compaction; +extern const SettingsBool data_lake_delete_data_on_drop; } static constexpr size_t MAX_TRANSACTION_RETRIES = 100; @@ -217,7 +223,6 @@ bool isExportPartitionTransactionAlreadyCommitted(const Poco::JSON::Object::Ptr } - using namespace Iceberg; namespace @@ -302,6 +307,7 @@ IcebergMetadata::IcebergMetadata( ContextPtr context_) : log(getLogger("IcebergMetadata")) , object_storage(std::move(object_storage_)) + , external_storages(std::make_shared()) , persistent_components(std::move(persistent_components_)) , data_lake_settings(configuration_->getDataLakeSettings()) , write_format(configuration_->getFormat()) @@ -362,7 +368,8 @@ void IcebergMetadata::backgroundMetadataPrefetcherThread() auto manifest_file_ptr = Iceberg::getManifestFile( object_storage, persistent_components, ctx, log, entry.manifest_file_path, - entry.manifest_file_byte_size); + entry.manifest_file_byte_size, + *external_storages); } } @@ -498,9 +505,8 @@ IcebergDataSnapshotPtr IcebergMetadata::createIcebergDataSnapshotFromSnapshotJSO throw Exception(ErrorCodes::ICEBERG_SPECIFICATION_VIOLATION, "No schema id found for snapshot id `{}`", snapshot_id); Int32 schema_id = snapshot_object->getValue(f_schema_id); - 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, *external_storages), snapshot_id, schema_id, total_rows, @@ -550,6 +556,7 @@ bool IcebergMetadata::optimize( snapshots_info, persistent_components, object_storage, + external_storages, data_lake_settings, format_settings, sample_block, @@ -593,7 +600,8 @@ bool IcebergMetadata::optimizeManifestFiles( context, write_format, catalog, - storage_id); + storage_id, + *external_storages); return true; } @@ -753,6 +761,7 @@ void IcebergMetadata::mutate( metadata_snapshot, storage_id, object_storage, + *external_storages, data_lake_settings, persistent_components, write_format, @@ -833,7 +842,7 @@ void IcebergMetadata::truncate(ContextPtr context, std::shared_ptrfinalize(); @@ -960,7 +969,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(), *external_storages); } else if (command_name == "remove_orphan_files") { @@ -974,7 +983,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, *external_storages); } else { @@ -1197,7 +1206,6 @@ DataLakeMetadataPtr IcebergMetadata::create( return std::make_unique(object_storage, configuration_ptr, std::move(persistent_components), local_context); } - IcebergMetadata::IcebergHistory IcebergMetadata::getHistory(ContextPtr local_context) const { const auto [metadata_version, metadata_file_path, compression_method] = getLatestOrExplicitMetadataFileAndVersion( @@ -1335,17 +1343,14 @@ String formatPartitionKeyValue(const DB::Row & partition_key_value) return result; } -IcebergFileRecord buildIcebergFileRecord( - const Iceberg::ProcessedManifestFileEntryPtr & processed, - Int64 inherited_snapshot_id, - const Iceberg::IcebergPathResolver & path_resolver) +IcebergFileRecord buildIcebergFileRecord(const Iceberg::ProcessedManifestFileEntryPtr & processed, Int64 inherited_snapshot_id) { const auto & parsed = *processed->parsed_entry; IcebergFileRecord record; record.snapshot_id = parsed.parsed_snapshot_id.value_or(inherited_snapshot_id); record.content = parsed.content_type; - record.file_path = path_resolver.resolve(parsed.file_path_key); + record.file_path = parsed.file_path_key.serialize(); record.file_format = parsed.file_format; record.record_count = parsed.record_count; record.file_size_in_bytes = parsed.file_size_in_bytes; @@ -1382,13 +1387,22 @@ 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, *external_storages); IcebergFiles result; for (auto content_type : {FileContentType::DATA, FileContentType::POSITION_DELETE, FileContentType::EQUALITY_DELETE}) { for (const auto & processed : handle.getFilesWithoutDeleted(content_type)) - result.push_back(buildIcebergFileRecord(processed, manifest_list_entry.added_snapshot_id, persistent_components.path_resolver)); + { + if (!isPathReadGranted( + persistent_components.table_location, + processed->parsed_entry->file_path_key.serialize(), + object_storage, + local_context)) + continue; + + result.push_back(buildIcebergFileRecord(processed, manifest_list_entry.added_snapshot_id)); + } } return result; } @@ -1419,7 +1433,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, *external_storages); if (!files_handle.areAllDataFilesSortedBySortOrderID(sorting_key.sort_order_id.value())) return false; @@ -1450,7 +1464,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, *external_storages); if (!files_handle.areAllDataFilesEligibleForLazyMaterialization(table_state_snapshot->schema_id)) return false; @@ -1494,7 +1508,16 @@ 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, *external_storages); + + /// Metadata-only counts must not bypass access or path restrictions enforced by a scan. + for (const auto & processed : manifest_file_ptr.getFilesWithoutDeleted(FileContentType::DATA)) + if (!isPathReadable( + persistent_components.table_location, + processed->parsed_entry->file_path_key.serialize(), + object_storage, + local_context)) + return {}; /// Live delete files make an exact metadata-only count impossible: /// - the record count of an equality delete file is the number of delete predicates, @@ -1552,7 +1575,16 @@ 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, *external_storages); + + for (const auto & processed : manifest_file_ptr.getFilesWithoutDeleted(FileContentType::DATA)) + if (!isPathReadable( + persistent_components.table_location, + processed->parsed_entry->file_path_key.serialize(), + object_storage, + local_context)) + return {}; + auto count = manifest_file_ptr.getBytesCountInAllDataFilesExcludingDeleted(); if (!count.has_value()) return {}; @@ -1622,7 +1654,8 @@ ObjectIterator IcebergMetadata::iterate( callback, iceberg_table_state, getRelevantDataSnapshotFromTableStateSnapshot(*iceberg_table_state, local_context), - persistent_components); + persistent_components, + external_storages); } NamesAndTypesList IcebergMetadata::getTableSchema(ContextPtr local_context) const @@ -1680,7 +1713,7 @@ void IcebergMetadata::addDeleteTransformers( LOG_DEBUG(log, "Constructing filter transform for position delete, there are {} delete objects", iceberg_object_info->info.position_deletes_objects.size()); 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, external_storages); }); } const auto & delete_files = iceberg_object_info->info.equality_deletes_objects; if (!delete_files.empty()) @@ -1691,9 +1724,13 @@ 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, *external_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; @@ -1720,7 +1757,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, @@ -1807,7 +1844,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, external_storages); } else { @@ -1818,27 +1855,100 @@ SinkToStoragePtr IcebergMetadata::write( } } +std::vector IcebergMetadata::collectExternalFilesToDeleteOnDrop(ContextPtr context) const +{ + /// `drop` runs after the catalog entry is removed, so use the configured metadata head, not an uncommitted higher version. + try + { + return Iceberg::collectReachableFiles( + object_storage, persistent_components, data_lake_settings, context, log, *external_storages, + /* scan_metadata_log_history */ false, /* ignore_explicit_metadata_file_path */ false).external_files; + } + catch (const Exception & e) + { + /// Permanent path rejections and missing metadata cannot succeed on retry; rethrow only transient failures. + if (e.code() != ErrorCodes::BAD_ARGUMENTS && e.code() != ErrorCodes::NOT_IMPLEMENTED + && e.code() != ErrorCodes::SUPPORT_IS_DISABLED && e.code() != ErrorCodes::PATH_ACCESS_DENIED + && e.code() != ErrorCodes::FILE_DOESNT_EXIST) + throw; + + LOG_ERROR( + log, + "Cannot enumerate the files of the Iceberg table at '{}' that live outside its directory: {}. " + "The table directory is deleted, but those files are left behind and have to be removed manually.", + persistent_components.table_path, + e.displayText()); + return {}; + } +} + +void IcebergMetadata::checkCanBeDropped(ContextPtr query_context) const +{ + /// `drop` runs in the background without the query context, so the dropping user's grants are checked here. + if (!query_context->getSettingsRef()[Setting::data_lake_delete_data_on_drop].value + || persistent_components.table_root_was_derived) + return; + + for (const auto & file : collectExternalFilesToDeleteOnDrop(Context::getGlobalContextInstance())) + { + if (!isPathWriteGranted(persistent_components.table_location, file.path.serialize(), object_storage, query_context)) + throw Exception( + ErrorCodes::ACCESS_DENIED, + "Not enough privileges to drop the Iceberg table: `data_lake_delete_data_on_drop` deletes the file '{}' " + "outside the table directory, which requires the `WRITE` grant on it", + file.path); + } + external_files_drop_authorized = true; +} + void IcebergMetadata::drop(bool delete_data) { - if (delete_data) + if (!delete_data) + 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; - } + 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; + } - auto files = listFiles(*object_storage, persistent_components.table_path, "", ""); - for (const auto & file : files) - object_storage->removeObjectIfExists(StoredObject(file)); + /// Enumerate external files before deleting the metadata needed to rediscover them on retry. + auto external_files = collectExternalFilesToDeleteOnDrop(Context::getGlobalContextInstance()); + if (!external_files.empty() && !external_files_drop_authorized) + { + LOG_WARNING( + log, + "Keeping {} file(s) of the Iceberg table at '{}' that live outside its directory: deleting them was not " + "authorized by a `DROP` query in this server process. They have to be removed manually.", + external_files.size(), + persistent_components.table_path); + external_files.clear(); + } + + /// Delete leaves first, one per request: a partial batch failure could delete the manifest needed for retry. + std::reverse(external_files.begin(), external_files.end()); + for (const auto & file : external_files) + { + LOG_DEBUG(log, "Removing external file during drop: storage={}, key={}", file.storage->getDescription(), file.key); + file.storage->removeObjectIfExists(StoredObject(file.key)); } + + 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 @@ -2266,7 +2376,7 @@ std::optional IcebergMetadata::commitImport try { generateManifestList( - resolver, metadata, object_storage, context, {manifest_entry_path}, new_snapshot, {manifest_lengths}, *buffer_manifest_list, Iceberg::FileContentType::DATA, true); + resolver, metadata, object_storage, *external_storages, context, {manifest_entry_path}, new_snapshot, {manifest_lengths}, *buffer_manifest_list, Iceberg::FileContentType::DATA, true); buffer_manifest_list->finalize(); } catch (...) diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h index be4e3433ca62b..96fc947a72359 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -32,6 +33,9 @@ #include #include #include +#include +#include +#include namespace DB @@ -178,6 +182,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, @@ -221,6 +227,7 @@ class IcebergMetadata : public IDataLakeMetadata StorageMetadataPtr storage_metadata, ContextPtr local_context) const override; + void checkCanBeDropped(ContextPtr query_context) const override; void drop(bool delete_data) override; static DataLakeMetadataPtr createWithDeserialization( @@ -295,9 +302,15 @@ class IcebergMetadata : public IDataLakeMetadata std::optional getPartitionKey(ContextPtr local_context, Iceberg::TableStateSnapshot actual_table_state_snapshot) const; KeyDescription getSortingKey(ContextPtr local_context, Iceberg::TableStateSnapshot actual_table_state_snapshot) const; + /// Files outside the table directory that `iceberg_delete_data_on_drop` deletes. + std::vector collectExternalFilesToDeleteOnDrop(ContextPtr context) const; + LoggerPtr log; const ObjectStoragePtr object_storage; - const DB::Iceberg::PersistentTableComponents persistent_components; + mutable std::shared_ptr external_storages; + /// Set by `checkCanBeDropped` once the dropping user is allowed to delete the external files. + mutable std::atomic external_files_drop_authorized = false; + 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 7461e40de7ddf..7182b830f5d6e 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 @@ -69,6 +70,11 @@ IcebergPathResolver::TableRootDerivation IcebergPathResolver::deriveTableRoot( return {String(candidate), RootRelation::AdoptedDescendant}; } +IcebergPathFromMetadata IcebergPathFromMetadata::makeStorageIdentity(const ObjectStoragePtr & storage, const String & key) +{ + return IcebergPathFromMetadata(storage->getDescription() + '\0' + storage->getObjectsNamespace() + '\0' + key); +} + // This function is used to get the file path inside the directory which corresponds to Iceberg table from the full blob path which is written in manifest and metadata files. // For example, if the full blob path is s3://bucket/table_name/data/00000-1-1234567890.avro, the function will return table_name/data/00000-1-1234567890.avro // Common path should end with "" or "/". diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h index 6a698c19cbeb6..3c5a35c9faccd 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h @@ -3,6 +3,11 @@ #include #include +#include + +#include +#include + namespace DB { class FileNamesGenerator; @@ -33,6 +38,9 @@ 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_)); } + /// Use resolved identity so URI aliases for 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; } @@ -42,7 +50,6 @@ class IcebergPathFromMetadata auto operator<=>(const IcebergPathFromMetadata & other) const { return raw_path <=> other.raw_path; } auto operator==(const IcebergPathFromMetadata & other) const { return raw_path == other.raw_path; } - private: friend class DB::FileNamesGenerator; @@ -145,8 +152,9 @@ struct std::hash template <> struct fmt::formatter : fmt::formatter { + /// This formatter is only for diagnostics; I/O and hashing use the unmasked `serialize` result. auto format(const DB::Iceberg::IcebergPathFromMetadata & p, fmt::format_context & ctx) const { - return fmt::formatter::format(p.serialize(), ctx); + return fmt::formatter::format(DB::maskCredentialsInURI(p.serialize()), ctx); } }; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp index d7b2448be3ed7..8f45b9bfa07fb 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -1046,6 +1048,7 @@ void generateManifestList( const Iceberg::IcebergPathResolver & path_resolver, Poco::JSON::Object::Ptr metadata, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, ContextPtr context, const std::vector & manifest_entry_names, Poco::JSON::Object::Ptr new_snapshot, @@ -1278,8 +1281,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, external_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(); @@ -1355,7 +1359,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 external_storages_) : SinkToStorage(sample_block_) , sample_block(sample_block_) , object_storage(object_storage_) @@ -1366,6 +1371,7 @@ IcebergStorageSink::IcebergStorageSink( , persistent_table_components(persistent_table_components_) , data_lake_settings(configuration_->getDataLakeSettings()) , write_format(configuration_->getFormat()) + , external_storages(std::move(external_storages_)) { auto [last_version, metadata_path, compression_method] = getLatestMetadataFileAndVersionWithCatalog( object_storage, @@ -1806,7 +1812,7 @@ bool IcebergStorageSink::initializeMetadata() { generateManifestList( persistent_table_components.path_resolver, - metadata, object_storage, context, + metadata, object_storage, *external_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 f63bcb623dc3e..acdff10acf6d0 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -44,6 +45,8 @@ namespace DB { +struct ExternalStorageCache; + String removeEscapedSlashes(const String & json_str); String stringifyJSON(const Poco::Dynamic::Var & json, unsigned indent = 0); @@ -159,6 +162,7 @@ void generateManifestList( const Iceberg::IcebergPathResolver & path_resolver, Poco::JSON::Object::Ptr metadata, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, ContextPtr context, const std::vector & manifest_entry_names, Poco::JSON::Object::Ptr new_snapshot, @@ -185,7 +189,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 external_storages_); ~IcebergStorageSink() override; @@ -230,6 +235,7 @@ class IcebergStorageSink final : public SinkToStorage Iceberg::PersistentTableComponents persistent_table_components; const DataLakeStorageSettings & data_lake_settings; const String write_format; + std::shared_ptr external_storages; }; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFileIterator.cpp index cecfc51c3c4fd..570735e6a1baa 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 @@ -421,7 +422,7 @@ ProcessedManifestFileEntryPtr ManifestFileIterator::processRow(size_t row_index) "for column id {} of data file '{}'; skipping min/max pruning for this column", path_to_manifest_file, column_id, - parsed_entry->file_path_key.serialize()); + parsed_entry->file_path_key); continue; } @@ -449,7 +450,7 @@ ProcessedManifestFileEntryPtr ManifestFileIterator::processRow(size_t row_index) "{} of data file '{}'; skipping min/max pruning for this column", path_to_manifest_file, column_id, - parsed_entry->file_path_key.serialize()); + parsed_entry->file_path_key); continue; } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp index 80a79c9a9f6c9..1d8a7bf0236fa 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -433,6 +434,7 @@ static bool writeMetadataFiles( const DataFileWriteResultWithStats & delete_files, const DataFileWriteResultWithStats & data_files, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, ContextPtr context, FileNamesGenerator & filename_generator, const Iceberg::IcebergPathResolver & path_resolver, @@ -598,6 +600,7 @@ static bool writeMetadataFiles( path_resolver, metadata, object_storage, + external_storages, context, manifest_entries, new_snapshot, @@ -685,7 +688,8 @@ void validateSnapshotDataFileFormatsForMutation( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + ExternalStorageCache & external_storages) { if (!metadata->has(f_current_snapshot_id) || metadata->isNull(f_current_snapshot_id)) return; @@ -711,11 +715,11 @@ 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, external_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, external_storages); for (const auto & file_entry : files_handle.getFilesWithoutDeleted(FileContentType::DATA)) { @@ -725,7 +729,7 @@ void validateSnapshotDataFileFormatsForMutation( ErrorCodes::NOT_IMPLEMENTED, "Iceberg DELETE and UPDATE require all data files in the current snapshot to be Parquet, " "but data file `{}` has format `{}`", - file_entry->parsed_entry->file_path_key.serialize(), + file_entry->parsed_entry->file_path_key, file_format); } } @@ -739,6 +743,7 @@ void mutate( StorageMetadataPtr storage_metadata, StorageID storage_id, ObjectStoragePtr object_storage, + ExternalStorageCache & external_storages, const DataLakeStorageSettings & data_lake_settings, const PersistentTableComponents & persistent_table_components, const String & write_format, @@ -817,7 +822,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), external_storages); TableStateSnapshot current_iceberg_snapshot; current_iceberg_snapshot.metadata_file_path = metadata_path; @@ -856,6 +861,7 @@ void mutate( mutation_files->delete_file, mutation_files->data_file, object_storage, + external_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 2cd2d33187d53..e41611a1954fb 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.h @@ -16,6 +16,8 @@ #include #include #include +#include +#include namespace DB::Iceberg { @@ -35,6 +37,7 @@ void mutate( StorageMetadataPtr storage_metadata, StorageID storage_id, ObjectStoragePtr object_storage, + ExternalStorageCache & external_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 ddb913f9464b1..0a9326eecbb75 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include "config.h" @@ -75,11 +76,12 @@ void IcebergPositionDeleteTransform::initializeDeleteSources() continue; } + auto [delete_storage_to_use, resolved_key] = resolveObjectStorageForPath( + path_resolver.getTableLocation(), position_deletes_object.file_path, object_storage, *external_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 +89,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 +100,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 f1738b3828b59..b6cb967126b01 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/PositionDeleteTransform.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include "config.h" @@ -8,6 +9,7 @@ #include #include #include +#include namespace DB::Iceberg { @@ -28,7 +30,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 external_storages_) : ISimpleTransform(header_, header_, false) , header(header_) , iceberg_object_info(iceberg_object_info_) @@ -36,6 +40,8 @@ class IcebergPositionDeleteTransform : public ISimpleTransform , format_settings(format_settings_) , context(context_) , parser_shared_resources(parser_shared_resources_) + , path_resolver(path_resolver_) + , external_storages(std::move(external_storages_)) { initializeDeleteSources(); } @@ -62,6 +68,9 @@ class IcebergPositionDeleteTransform : public ISimpleTransform ContextPtr context; FormatParserSharedResourcesPtr parser_shared_resources; + const IcebergPathResolver path_resolver; + std::shared_ptr external_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 +87,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 external_storages_) + : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_, path_resolver_, std::move(external_storages_)) { initialize(); } @@ -104,8 +115,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 external_storages_) + : IcebergPositionDeleteTransform(header_, iceberg_object_info_, object_storage_, format_settings_, parser_shared_resources_, context_, path_resolver_, std::move(external_storages_)) { initialize(); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp index b16516209dd35..715a7e98bcf36 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.cpp @@ -1,4 +1,5 @@ #include "config.h" +#include #if USE_AVRO #include @@ -256,12 +257,25 @@ RemoveOrphanFilesResult removeOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_table_components) + const PersistentTableComponents & persistent_table_components, + ExternalStorageCache & external_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, external_storages, + /* scan_metadata_log_history */ true, /* ignore_explicit_metadata_file_path */ true); + + 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, including historical " + "versions from metadata-log): 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,9 @@ 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, external_storages, + /* scan_metadata_log_history */ false, /* ignore_explicit_metadata_file_path */ true); if (recheck_version != metadata_version) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Metadata version changed during orphan scan (v{} -> v{}); " @@ -296,7 +311,6 @@ RemoveOrphanFilesResult removeOrphanFiles( } - // --------------------------------------------------------------------------- // Public entry point // --------------------------------------------------------------------------- @@ -306,7 +320,8 @@ Pipe executeRemoveOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_components) + const PersistentTableComponents & persistent_components, + ExternalStorageCache & external_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, external_storages); return resultToPipe(result); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h index 1809d3880badd..9bfe77dd07668 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/RemoveOrphanFilesExecute.h @@ -8,6 +8,8 @@ #include #include #include +#include +#include namespace DB::Iceberg { @@ -17,7 +19,8 @@ Pipe executeRemoveOrphanFiles( ContextPtr context, ObjectStoragePtr object_storage, const DataLakeStorageSettings & data_lake_settings, - const PersistentTableComponents & persistent_components); + const PersistentTableComponents & persistent_components, + ExternalStorageCache & external_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp index ade8c7053f395..8c18725497fda 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/SnapshotFilesTraversal.cpp @@ -4,16 +4,26 @@ #include +#include +#include +#include +#include +#include + #include #include #include +#include + #include #include #include #include +#include +#include namespace DB::Iceberg { @@ -24,7 +34,8 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id) + Int32 current_schema_id, + ExternalStorageCache & external_storages) { SnapshotReferencedFiles files; @@ -38,14 +49,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, external_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, external_storages); for (const auto & entry : entries_handle.getFilesWithoutDeleted(FileContentType::DATA)) files.data_file_paths.insert(entry->parsed_entry->file_path_key); @@ -62,11 +73,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 +91,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 +103,10 @@ void collectMetadataRootFiles( const String & metadata_path, const Poco::JSON::Object::Ptr & metadata, const IcebergPathResolver & resolver, + const VisitPathFn & visit, std::unordered_set & out) { + /// `metadata_path` is already a storage key; passing it through URI resolution 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,25 +123,163 @@ 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); } +/// Ignore deleted historical metadata: `remove_orphan_files` may have removed it on a previous run. +/// Other errors propagate because they leave the referenced paths unknown. +void collectHistoricalReferences( + const Poco::JSON::Object::Ptr & current_metadata, + ObjectStoragePtr object_storage, + const PersistentTableComponents & persistent_table_components, + ContextPtr context, + LoggerPtr log, + ExternalStorageCache & external_storages, + Int32 current_schema_id, + const VisitPathFn & visit_history) +{ + const auto & resolver = persistent_table_components.path_resolver; + + std::vector metadata_worklist; + std::unordered_set seen_metadata_paths; + + auto enqueue_log_entries = [&](const Poco::JSON::Object::Ptr & metadata) + { + if (!metadata->has(f_metadata_log)) + return; + auto metadata_log = metadata->get(f_metadata_log).extract(); + if (!metadata_log) + return; + for (UInt32 i = 0; i < metadata_log->size(); ++i) + { + auto entry = metadata_log->getObject(i); + if (!entry->has(f_metadata_file)) + continue; + String mf_path = entry->getValue(f_metadata_file); + if (seen_metadata_paths.emplace(mf_path).second) + metadata_worklist.push_back(IcebergPathFromMetadata::deserialize(mf_path)); + } + }; + + enqueue_log_entries(current_metadata); + + std::unordered_set traversed_manifest_lists; + + using ResolvedPath = std::pair; + auto resolve_if_present = [&](const IcebergPathFromMetadata & path) -> std::optional + { + auto resolved = resolveObjectStorageForPath( + persistent_table_components.table_location, path.serialize(), + object_storage, external_storages, context, resolver); + if (!resolved.first->exists(StoredObject(resolved.second))) + return std::nullopt; + return resolved; + }; + + while (!metadata_worklist.empty()) + { + auto historical_metadata_path = metadata_worklist.back(); + metadata_worklist.pop_back(); + visit_history(historical_metadata_path); + + auto resolved_metadata = resolve_if_present(historical_metadata_path); + if (!resolved_metadata) + { + LOG_WARNING( + log, + "Historical metadata file {} is already deleted from storage, so the paths it referenced " + "cannot be checked for locations outside the table's base directory", + historical_metadata_path); + continue; + } + + auto & [historical_metadata_storage, historical_metadata_key] = *resolved_metadata; + /// The metadata cache is keyed by path alone, which is not unique across storages. + auto historical_metadata = getMetadataJSONObject( + historical_metadata_key, + historical_metadata_storage, + /* metadata_cache */ nullptr, + context, + log, + getCompressionMethodFromMetadataFile(historical_metadata_key), + persistent_table_components.table_uuid); + + enqueue_log_entries(historical_metadata); + collectStatisticsPaths(historical_metadata, f_statistics, visit_history); + collectStatisticsPaths(historical_metadata, f_partition_statistics, visit_history); + + if (!historical_metadata->has(f_snapshots)) + continue; + auto snapshots = historical_metadata->get(f_snapshots).extract(); + if (!snapshots) + continue; + + for (UInt32 i = 0; i < snapshots->size(); ++i) + { + auto snapshot = snapshots->getObject(i); + if (!snapshot->has(f_manifest_list)) + continue; + + auto manifest_list_path = IcebergPathFromMetadata::deserialize(snapshot->getValue(f_manifest_list)); + if (!traversed_manifest_lists.emplace(manifest_list_path).second) + continue; + visit_history(manifest_list_path); + + if (!resolve_if_present(manifest_list_path)) + { + LOG_WARNING( + log, + "Manifest list {} of historical metadata file {} is already deleted from storage, so the " + "files it referenced cannot be checked for locations outside the table's base directory", + manifest_list_path, + historical_metadata_path); + continue; + } + + auto manifest_keys = getManifestList( + object_storage, persistent_table_components, context, manifest_list_path, log, external_storages); + for (const auto & manifest_entry : manifest_keys) + { + visit_history(manifest_entry.manifest_file_path); + if (!resolve_if_present(manifest_entry.manifest_file_path)) + { + LOG_WARNING( + log, + "Manifest file {} of historical metadata file {} is already deleted from storage, so the " + "files it referenced cannot be checked for locations outside the table's base directory", + manifest_entry.manifest_file_path, + historical_metadata_path); + continue; + } + + auto entries_handle = getManifestFileEntriesHandle( + object_storage, persistent_table_components, context, log, manifest_entry, current_schema_id, external_storages); + for (auto content_type : {FileContentType::DATA, FileContentType::POSITION_DELETE, FileContentType::EQUALITY_DELETE}) + for (const auto & entry : entries_handle.getFilesWithoutDeleted(content_type)) + visit_history(entry->parsed_entry->file_path_key); + } + } + } } +} ReachableFilesResult collectReachableFiles( ObjectStoragePtr object_storage, const PersistentTableComponents & persistent_table_components, const DataLakeStorageSettings & data_lake_settings, ContextPtr context, - LoggerPtr log) + LoggerPtr log, + ExternalStorageCache & external_storages, + bool scan_metadata_log_history, + bool ignore_explicit_metadata_file_path) { auto [version, metadata_path, compression_method] = getLatestOrExplicitMetadataFileAndVersion( object_storage, @@ -139,7 +291,7 @@ ReachableFilesResult collectReachableFiles( persistent_table_components.table_uuid, persistent_table_components.metadata_compression_method, /* force_fetch_latest_metadata */ true, - /* ignore_explicit_metadata_file_path */ true); + ignore_explicit_metadata_file_path); auto metadata = getMetadataJSONObject( metadata_path, @@ -151,40 +303,86 @@ 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; + chassert(isObjectInTableDirectory( + object_storage, metadata_path, object_storage, persistent_table_components.table_path)); + + auto visit = [&](const IcebergPathFromMetadata & path) + { + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.table_location, path.serialize(), object_storage, external_storages, context, resolver); + if (isObjectInTableDirectory(storage, key, object_storage, persistent_table_components.table_path)) + reachable.insert(std::move(key)); + else if (seen_external.emplace(storage.get(), key).second) + external_files.push_back({std::move(storage), std::move(key), path}); + }; + collectMetadataRootFiles( metadata_path, metadata, resolver, + visit, reachable); + if (scan_metadata_log_history) + { + /// History contributes only existing external files; expired in-table files must remain eligible as orphans. + std::set> missing_external; + auto visit_history = [&](const IcebergPathFromMetadata & path) + { + auto [storage, key] = resolveObjectStorageForPath( + persistent_table_components.table_location, path.serialize(), object_storage, external_storages, context, resolver); + if (isObjectInTableDirectory(storage, key, object_storage, persistent_table_components.table_path)) + return; + + const std::pair external_id{storage.get(), key}; + if (seen_external.contains(external_id) || missing_external.contains(external_id)) + return; + + if (!storage->exists(StoredObject(key))) + { + missing_external.insert(external_id); + return; + } + + seen_external.insert(external_id); + external_files.push_back({std::move(storage), std::move(key), path}); + }; + collectHistoricalReferences( + metadata, object_storage, persistent_table_components, context, log, external_storages, + metadata->getValue(f_current_schema_id), visit_history); + } + 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, external_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 a9961d8e469b3..aac30de55fad8 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,8 @@ #include #include #include +#include +#include namespace DB::Iceberg { @@ -35,26 +39,41 @@ SnapshotReferencedFiles collectSnapshotReferencedFiles( const PersistentTableComponents & persistent_table_components, ContextPtr context, LoggerPtr log, - Int32 current_schema_id); + Int32 current_schema_id, + ExternalStorageCache & external_storages); + +struct ExternalFile +{ + ObjectStoragePtr storage; + String key; + IcebergPathFromMetadata path; +}; struct ReachableFilesResult { std::unordered_set files; Int32 metadata_version; + std::vector external_files; }; /// Collect all files reachable through the 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. +/// and statistics files. Paths inside the table directory are returned as resolved storage +/// paths in `files`; files outside it (on any storage) are returned in `external_files`. /// Also returns the metadata version used, for TOCTOU detection. +/// `scan_metadata_log_history` adds surviving external references without making expired in-table files reachable. +/// `ignore_explicit_metadata_file_path` selects the latest head (for `remove_orphan_files`) instead of the configured one (for `drop`). ReachableFilesResult collectReachableFiles( ObjectStoragePtr object_storage, const PersistentTableComponents & persistent_table_components, const DataLakeStorageSettings & data_lake_settings, ContextPtr context, - LoggerPtr log); + LoggerPtr log, + ExternalStorageCache & external_storages, + bool scan_metadata_log_history, + bool ignore_explicit_metadata_file_path); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp index 5d9055f020d2d..5b3cb0a50fea1 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include @@ -76,7 +77,8 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( ContextPtr local_context, LoggerPtr log, const IcebergPathFromMetadata & filename, - size_t bytes_size) + size_t bytes_size, + ExternalStorageCache & external_storages) { auto log_level = local_context->getSettingsRef()[Setting::iceberg_metadata_log_level].value; @@ -85,7 +87,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, external_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 +104,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 +126,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH ContextPtr local_context, LoggerPtr log, const ManifestFileCacheKey & cache_key, - Int32 table_snapshot_schema_id) + Int32 table_snapshot_schema_id, + ExternalStorageCache & external_storages) { auto cacheable_info = getManifestFile( object_storage, @@ -128,7 +135,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH local_context, log, cache_key.manifest_file_path, - static_cast(cache_key.manifest_file_byte_size)); + static_cast(cache_key.manifest_file_byte_size), + external_storages); auto iterator = Iceberg::ManifestFileIterator::create( cacheable_info.deserializer, @@ -153,7 +161,8 @@ ManifestFileCacheKeys getManifestList( const PersistentTableComponents & persistent_table_components, ContextPtr local_context, const IcebergPathFromMetadata & filename, - LoggerPtr log) + LoggerPtr log, + ExternalStorageCache & external_storages) { IcebergMetadataLogLevel log_level = local_context->getSettingsRef()[Setting::iceberg_metadata_log_level].value; @@ -162,14 +171,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, external_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 @@ -185,7 +198,7 @@ ManifestFileCacheKeys getManifestList( local_context, dump_metadata, DB::IcebergMetadataLogLevel::ManifestListMetadata, - persistent_table_components.path_resolver.getTableRoot(), + persistent_table_components.table_path, filename, std::nullopt, std::nullopt); @@ -251,7 +264,7 @@ ManifestFileCacheKeys getManifestList( local_context, dump_row_metadata, 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 2bb2edfdff13c..6cbbdc5088497 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/StatelessMetadataFileGetter.h @@ -16,6 +16,8 @@ #include #include +#include +#include namespace DB::Iceberg { @@ -26,7 +28,8 @@ Iceberg::ManifestFileCacheableInfo getManifestFile( ContextPtr local_context, LoggerPtr log, const IcebergPathFromMetadata & filename, - size_t bytes_size); + size_t bytes_size, + ExternalStorageCache & external_storages); /// Creates a fully initialized ManifestFileIterator from a cache key. /// All entries are drained so that aggregate methods (e.g. getRowsCountInAllFilesExcludingDeleted) @@ -37,7 +40,8 @@ Iceberg::ManifestFileIterator::ManifestFileEntriesHandle getManifestFileEntriesH ContextPtr local_context, LoggerPtr log, const ManifestFileCacheKey & cache_key, - Int32 table_snapshot_schema_id); + Int32 table_snapshot_schema_id, + ExternalStorageCache & external_storages); ManifestFileCacheKeys getManifestList( @@ -45,7 +49,8 @@ ManifestFileCacheKeys getManifestList( const PersistentTableComponents & persistent_table_components, ContextPtr local_context, const IcebergPathFromMetadata & filename, - LoggerPtr log); + LoggerPtr log, + ExternalStorageCache & external_storages); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp index 78465258f09bb..f151e6a8067e1 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp @@ -48,13 +48,14 @@ #include #include #include +#include #include #if USE_AVRO #include -#include #include +#include #include #include #include @@ -68,6 +69,7 @@ #include #include #include +#include #include @@ -122,7 +124,6 @@ static constexpr size_t MAX_LIST_RETRIES = 5; namespace DB::Iceberg { - using namespace DB; /// Best-effort heuristic based on ClickHouse naming conventions. diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h index 2d359bc7ae32c..434ff0e8618e4 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 @@ -18,6 +16,9 @@ #include #include + +#if USE_AVRO + #include #include #include diff --git a/src/Storages/ObjectStorage/IObjectIterator.cpp b/src/Storages/ObjectStorage/IObjectIterator.cpp index 2a9545913bf16..d59b9e8aa463a 100644 --- a/src/Storages/ObjectStorage/IObjectIterator.cpp +++ b/src/Storages/ObjectStorage/IObjectIterator.cpp @@ -81,6 +81,9 @@ ObjectInfoPtr ObjectIteratorWithPathAndFileFilter::next(size_t id) path = path.substr(1); path = std::filesystem::path(object_namespace) / path; + if (auto metadata_path = object->getPathInDataLakeMetadata()) + path = *metadata_path; + VirtualColumnUtils::filterByPathOrFile( keys, std::vector{path}, filter_actions, virtual_columns, hive_partition_columns, getContext()); @@ -114,7 +117,6 @@ std::string ObjectInfo::getPathOrPathToArchiveIfArchive() const return getPath(); } - ObjectIteratorSplitByBuckets::ObjectIteratorSplitByBuckets( ObjectIterator iterator_, const String & format_, @@ -149,7 +151,9 @@ ObjectInfoPtr ObjectIteratorSplitByBuckets::next(size_t id) bool has_cache_entry = false; if (query_condition_cache) { - const auto query_condition_cache_key = last_object_info->getIdentifier(/*include_file_bucket_info=*/ false); + auto query_condition_cache_key = last_object_info->getIdentifier(/*include_file_bucket_info=*/ false); + if (auto metadata_path = last_object_info->getPathInDataLakeMetadata()) + query_condition_cache_key = last_object_info->getIdentifierForPath(*metadata_path, /*include_file_bucket_info=*/ false); auto matching_marks = query_condition_cache->read( storage_id.uuid, query_condition_cache_key, @@ -166,24 +170,25 @@ ObjectInfoPtr ObjectIteratorSplitByBuckets::next(size_t id) } } - auto buffer = createReadBuffer(last_object_info->relative_path_with_metadata, object_storage, getContext(), log); + auto storage_to_use = last_object_info->getResolvedStorage(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; + 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 467474e9d8e3d..ee017e44c2d99 100644 --- a/src/Storages/ObjectStorage/IObjectIterator.h +++ b/src/Storages/ObjectStorage/IObjectIterator.h @@ -50,6 +50,14 @@ struct ObjectInfo virtual std::optional getFileSizeHint() const { return std::nullopt; } + virtual ObjectStoragePtr getResolvedStorage(const ObjectStoragePtr & default_storage) const { return default_storage; } + + /// Manifest spelling used by `_path` and cross-storage cache keys; storage keys alone are not unique. + virtual std::optional getPathInDataLakeMetadata() const { return std::nullopt; } + + /// Returns a local dependency that cannot be handed to another replica, including attached delete files. + virtual std::optional getExternalLocalPath() const { return std::nullopt; } + std::optional getObjectMetadata() const { return relative_path_with_metadata.metadata; } void setObjectMetadata(const ObjectMetadata & metadata) { relative_path_with_metadata.metadata = metadata; } @@ -85,6 +93,11 @@ struct IObjectIterator /// Set `emit_profile_events` flag, propagating to nested iterators if any. virtual void setEmitProfileEvents(bool value) { emit_profile_events = value; } + + bool tasks_go_to_other_replicas = false; + + /// Call before the first `next`; propagates to nested iterators. + virtual void setTasksGoToOtherReplicas(bool value) { tasks_go_to_other_replicas = value; } }; using ObjectIterator = std::shared_ptr; @@ -111,6 +124,12 @@ class ObjectIteratorWithPathAndFileFilter : public IObjectIterator, private With iterator->setEmitProfileEvents(value); } + void setTasksGoToOtherReplicas(bool value) override + { + tasks_go_to_other_replicas = value; + iterator->setTasksGoToOtherReplicas(value); + } + private: const ObjectIterator iterator; const std::string object_namespace; @@ -136,6 +155,12 @@ class ObjectIteratorSplitByBuckets : public IObjectIterator, private WithContext size_t estimatedKeysCount() override { return iterator->estimatedKeysCount(); } std::optional getSnapshotVersion() const override { return iterator->getSnapshotVersion(); } + void setTasksGoToOtherReplicas(bool value) override + { + tasks_go_to_other_replicas = value; + iterator->setTasksGoToOtherReplicas(value); + } + private: const ObjectIterator iterator; String format; @@ -149,5 +174,4 @@ class ObjectIteratorSplitByBuckets : public IObjectIterator, private WithContext const LoggerPtr log = getLogger("GlobIterator"); }; - } diff --git a/src/Storages/ObjectStorage/ReadBufferIterator.cpp b/src/Storages/ObjectStorage/ReadBufferIterator.cpp index 264a6fe304c5e..8874a4be95837 100644 --- a/src/Storages/ObjectStorage/ReadBufferIterator.cpp +++ b/src/Storages/ObjectStorage/ReadBufferIterator.cpp @@ -44,9 +44,9 @@ ReadBufferIterator::ReadBufferIterator( format = configuration->getFormat(); } -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 +58,7 @@ SchemaCache::Keys ReadBufferIterator::getKeysForSchemaCache() const read_keys.begin(), read_keys.end(), std::back_inserter(sources), - [&](const auto & elem) { return StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, *elem); }); + [&](const auto & elem) { return StorageObjectStorageSource::getUniqueStoragePathIdentifier(*configuration, elem, object_storage); }); return DB::getKeysForSchemaCache(sources, *format, format_settings, getContext()); } @@ -83,7 +83,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 = object_info->getResolvedStorage(object_storage); + auto meta = storage_to_use->tryGetObjectMetadata(metadata_object, /*with_tags=*/ false); if (meta) object_info->setObjectMetadata(*meta); } @@ -100,7 +101,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 +112,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 +128,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 +154,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, + current_object_info->getResolvedStorage(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 +286,10 @@ ReadBufferIterator::Data ReadBufferIterator::next() { compression_method = chooseCompressionMethod(filename, configuration->getCompressionMethod()); read_buf = createReadBuffer( - current_object_info->relative_path_with_metadata, object_storage, getContext(), getLogger("ReadBufferIterator")); + current_object_info->relative_path_with_metadata, + current_object_info->getResolvedStorage(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 af1c28f437605..a14d609752b7c 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/StorageObjectStorage.h b/src/Storages/ObjectStorage/StorageObjectStorage.h index 5b80172074fed..53305d7c3ceed 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorage.h +++ b/src/Storages/ObjectStorage/StorageObjectStorage.h @@ -132,6 +132,7 @@ class StorageObjectStorage : public IStorage, public IBackgroundOperation ContextPtr local_context, TableExclusiveLockHolder &) override; + void checkTableCanBeDropped(ContextPtr query_context) const override { configuration->checkCanBeDropped(query_context); } void drop() override; void prepareForDrop(ContextPtr query_context) override; diff --git a/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp b/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp index 555d7dd3f159b..c85036edd08e8 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp @@ -711,6 +711,8 @@ RemoteQueryExecutor::Extension StorageObjectStorageCluster::getTaskIteratorExten ); } + iterator->setTasksGoToOtherReplicas(true); + std::vector ids_of_hosts; for (const auto & shard : cluster->getShardsInfo()) { diff --git a/src/Storages/ObjectStorage/StorageObjectStorageCluster.h b/src/Storages/ObjectStorage/StorageObjectStorageCluster.h index 4c077a1fbea23..60a7154d33ac4 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageCluster.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageCluster.h @@ -51,6 +51,7 @@ class StorageObjectStorageCluster : public IStorageCluster const IcebergCommitExportPartitionArguments & iceberg_commit_export_partition_arguments, ContextPtr local_context) override; + void checkTableCanBeDropped(ContextPtr query_context) const override { configuration->checkCanBeDropped(query_context); } void prepareForDrop(ContextPtr query_context) override; RemoteQueryExecutor::Extension getTaskIteratorExtension( diff --git a/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h b/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h index d4e4dfd21c8f7..e51af4fbad73c 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h @@ -357,6 +357,7 @@ class StorageObjectStorageConfiguration return false; } + virtual void checkCanBeDropped(ContextPtr /*query_context*/) const {} /// `delete_data` is what `StorageObjectStorage::drop` resolved from `data_lake_delete_data_on_drop`. virtual void drop(bool /* delete_data */) {} diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp index c2f36fbd96f67..426f4c97169f7 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageSource.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -77,7 +78,6 @@ #include #include - namespace fs = std::filesystem; namespace ProfileEvents { @@ -380,6 +380,27 @@ 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) +{ + auto resolved_storage = object_info->getResolvedStorage(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, @@ -404,11 +425,16 @@ std::shared_ptr StorageObjectStorageSource::createFileIterator( { const bool expect_whole_archive = !local_context->getSettingsRef()[Setting::cluster_function_process_archive_on_multiple_nodes]; + 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) @@ -688,6 +714,8 @@ Chunk StorageObjectStorageSource::generate() read_context); } + std::string path_for_virtual_column = object_info->getPathInDataLakeMetadata().value_or(path); + const String * iceberg_metadata_file_path = nullptr; #if USE_AVRO if (const auto * iceberg_info = dynamic_cast(object_info.get())) @@ -704,7 +732,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, @@ -930,7 +958,7 @@ Chunk StorageObjectStorageSource::generate() && format_filter_info && !format_filter_info->hasFilter() && canUseCountFromFilesCache(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; chassert(reader_future.valid()); @@ -951,11 +979,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->getFormat()), + getUniqueStoragePathIdentifier(*configuration, object_info, object_storage), + object_info->getFileFormat().value_or(configuration->getFormat()), format_settings, read_context); schema_cache.addNumRows(cache_key, num_rows); @@ -1035,9 +1063,11 @@ StorageObjectStorageSource::ReaderHolder StorageObjectStorageSource::createReade auto metadata_object = object_info->relative_path_with_metadata; metadata_object.relative_path = path; + ObjectStoragePtr storage_to_use = object_info->getResolvedStorage(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 {}; @@ -1045,7 +1075,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)); } } @@ -1114,7 +1144,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->getFormat()), format_settings, context_); @@ -1341,7 +1371,7 @@ 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, object_info->getResolvedStorage(object_storage), context_, log, std::nullopt, !headers_requested); } Block initial_header = read_from_format_info.format_header; @@ -1825,7 +1855,6 @@ std::unique_ptr createReadBuffer( bool use_page_cache = allow_page_cache && !use_distributed_cache && !use_filesystem_cache && effective_read_settings.page_cache_settings.cache && effective_read_settings.use_page_cache_for_object_storage; - /// We need object metadata for a few use cases: /// 1. object size suggests whether we need to use prefetch /// 2. object etag suggests a cache key in case we use filesystem cache @@ -2100,7 +2129,6 @@ ObjectInfoPtr StorageObjectStorageSource::GlobIterator::nextUnlocked(size_t /* p { ObjectInfos new_batch; - while (new_batch.empty()) { auto result = object_storage_iterator->getCurrentBatchAndScheduleNext(); @@ -2325,11 +2353,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_) { if (!getContext()->isSwarmModeEnabled()) { @@ -2361,10 +2391,20 @@ StorageObjectStorageSource::ReadTaskIterator::ReadTaskIterator( { auto object = object_future.get(); if (object) + { + resolveObjectStorageIfNeeded(object); buffer.push_back(object); + } } } +void StorageObjectStorageSource::ReadTaskIterator::resolveObjectStorageIfNeeded([[maybe_unused]] const ObjectInfoPtr & object) +{ +#if USE_AVRO + resolveObjectStorageFromDataLakeMetadata(object, table_location, object_storage, external_storages, getContext()); +#endif +} + static size_t getKnownArchiveSize(const ObjectInfoPtr & object_info) { const auto object_metadata = object_info->getObjectMetadata(); @@ -2397,7 +2437,9 @@ ObjectInfoPtr StorageObjectStorageSource::ReadTaskIterator::next(size_t) if (!task || task->isEmpty()) return nullptr; + object_info = task->getObjectInfo(); + resolveObjectStorageIfNeeded(object_info); } else { @@ -2497,7 +2539,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 = object_info->getResolvedStorage(object_storage); + return createReadBuffer(object_info->relative_path_with_metadata, storage, getContext(), log); + }, /* archive_size */ size); } @@ -2519,7 +2564,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 = archive_object->getResolvedStorage(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(); @@ -2545,7 +2593,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 = archive_object->getResolvedStorage(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 3cf2b9788b23d..f0b2c082892ee 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSource.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageSource.h @@ -11,6 +11,9 @@ #include #include #include +#include +#include +#include #include #include @@ -76,6 +79,12 @@ class StorageObjectStorageSource final : public ISource static std::string getUniqueStoragePathIdentifier( const StorageObjectStorageConfiguration & configuration, const ObjectInfo & object_info, bool include_connection_info = true); + 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; @@ -171,7 +180,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(); }; @@ -183,6 +192,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; @@ -195,11 +205,17 @@ class StorageObjectStorageSource::ReadTaskIterator : public IObjectIterator, pri const std::string & path_in_archive, std::optional read_source_index); + void resolveObjectStorageIfNeeded(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 + ExternalStorageCache external_storages; +#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 abf8e1746e9be..b4f26536966cc 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageStableTaskDistributor.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -16,6 +17,7 @@ namespace ErrorCodes { extern const int LOGICAL_ERROR; extern const int CANNOT_READ_ALL_DATA; + extern const int NOT_IMPLEMENTED; }; namespace @@ -26,6 +28,20 @@ String getSchedulingIdentifier(const ObjectInfoPtr & object_info, bool send_over if (send_over_whole_archive && object_info->isArchive()) return object_info->getIdentifierForPath(object_info->getPathToArchive()); + if (auto metadata_path = object_info->getPathInDataLakeMetadata()) + { + /// The coordinator may not be a replica, and replicas need not share its filesystem, so local files cannot be dispatched. + if (auto local_path = object_info->getExternalLocalPath()) + throw Exception( + ErrorCodes::NOT_IMPLEMENTED, + "Iceberg metadata references the file '{}' on the local filesystem, outside of the table location. " + "Such a file cannot be read by a cluster function, which distributes the work across replicas that " + "do not share this filesystem. Read the table without the `*Cluster` function instead", + *local_path); + + return object_info->getIdentifierForPath(*metadata_path); + } + return object_info->getIdentifier(); } @@ -161,7 +177,7 @@ ObjectInfoPtr StorageObjectStorageStableTaskDistributor::getPreQueuedFile(size_t LOG_TRACE( log, "Assigning pre-queued file {} to replica {}", - file_identifier, + maskCredentialsInURI(file_identifier), number_of_current_replica ); @@ -202,11 +218,11 @@ ObjectInfoPtr StorageObjectStorageStableTaskDistributor::getMatchingFileFromIter file_identifier = getSchedulingIdentifier(object_info, send_over_whole_archive); LOG_TEST(log, "Will send over the whole archive {} to replicas. " "This will be suboptimal, consider turning on " - "cluster_function_process_archive_on_multiple_nodes setting", file_identifier); + "cluster_function_process_archive_on_multiple_nodes setting", maskCredentialsInURI(file_identifier)); } else { - file_identifier = object_info->getIdentifier(); + file_identifier = getSchedulingIdentifier(object_info, send_over_whole_archive); } if (iceberg_read_optimization_enabled) @@ -231,7 +247,7 @@ ObjectInfoPtr StorageObjectStorageStableTaskDistributor::getMatchingFileFromIter { LOG_TRACE( log, "Found file {} for replica {}", - file_identifier, number_of_current_replica + maskCredentialsInURI(file_identifier), number_of_current_replica ); ProfileEvents::increment(ProfileEvents::ObjectStorageClusterSentToMatchedReplica); @@ -240,7 +256,7 @@ ObjectInfoPtr StorageObjectStorageStableTaskDistributor::getMatchingFileFromIter LOG_TEST( log, "Found file {} for replica {} (number of current replica: {})", - file_identifier, + maskCredentialsInURI(file_identifier), file_replica_idx, number_of_current_replica ); diff --git a/src/Storages/ObjectStorage/Utils.cpp b/src/Storages/ObjectStorage/Utils.cpp index 958d2adc3be7d..26a0e1067bd4d 100644 --- a/src/Storages/ObjectStorage/Utils.cpp +++ b/src/Storages/ObjectStorage/Utils.cpp @@ -18,6 +18,8 @@ #include #include #include +#include + namespace DB { @@ -365,4 +367,5 @@ extern const SettingsBool use_cache_for_count_from_files; extern const SettingsString filesystem_cache_name; extern const SettingsUInt64 filesystem_cache_boundary_alignment; } + } diff --git a/src/Storages/ObjectStorage/tests/gtest_datalake_table_state_serde.cpp b/src/Storages/ObjectStorage/tests/gtest_datalake_table_state_serde.cpp index cf5b8befdaf2f..d2438a374e183 100644 --- a/src/Storages/ObjectStorage/tests/gtest_datalake_table_state_serde.cpp +++ b/src/Storages/ObjectStorage/tests/gtest_datalake_table_state_serde.cpp @@ -4,7 +4,17 @@ #include #include #include +#include #include +#if USE_AVRO && USE_AWS_S3 +#include +#include +#endif + +namespace DB::ErrorCodes +{ +extern const int PROTOCOL_VERSION_MISMATCH; +} using namespace DB; @@ -24,6 +34,17 @@ std::vector example_iceberg_states = { .snapshot_id = std::nullopt, }, }; + +Iceberg::IcebergObjectSerializableInfo makeObjectInfo() +{ + Iceberg::IcebergObjectSerializableInfo info; + info.data_object_file_path_key = DB::Iceberg::IcebergPathFromMetadata::deserialize("s3://bucket/path/to/file.parquet"); + info.underlying_format_read_schema_id = 1; + info.schema_id_relevant_to_iterator = 1; + info.sequence_number = 0; + info.file_format = "PARQUET"; + return info; +} } TEST(DatalakeStateSerde, IcebergStateSerde) @@ -79,7 +100,7 @@ TEST(DatalakeStateSerde, IcebergObjectSerializableInfoRoundTrip) String str; { WriteBufferFromString write_buffer{str}; - info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION); + info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION, DBMS_ANTALYA_PROTOCOL_VERSION); } ReadBufferFromMemory read_buffer(str.data(), str.size()); Iceberg::IcebergObjectSerializableInfo deserialized; @@ -115,7 +136,7 @@ TEST(DatalakeStateSerde, IcebergObjectSerializableInfoWithoutFileStats) String str; { WriteBufferFromString write_buffer{str}; - info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_EXCLUDED_ROWS); + info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_EXCLUDED_ROWS, DBMS_ANTALYA_PROTOCOL_VERSION); } /// Deserialize at protocol version 5 — the new fields should be absent @@ -145,7 +166,7 @@ TEST(DatalakeStateSerde, IcebergObjectSerializableInfoNulloptFileStats) String str; { WriteBufferFromString write_buffer{str}; - info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION); + info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION, DBMS_ANTALYA_PROTOCOL_VERSION); } ReadBufferFromMemory read_buffer(str.data(), str.size()); Iceberg::IcebergObjectSerializableInfo deserialized; @@ -156,3 +177,75 @@ TEST(DatalakeStateSerde, IcebergObjectSerializableInfoNulloptFileStats) ASSERT_FALSE(deserialized.file_size_in_bytes.has_value()); } + +TEST(DatalakeStateSerde, IcebergObjectSerializableInfoExternalPathRejectedBeforeAbsolutePathProtocol) +{ + auto info = makeObjectInfo(); + info.requires_external_storage = true; + for (UInt64 antalya_version : {0, 1}) + { + SCOPED_TRACE(antalya_version); + String str; + WriteBufferFromString write_buffer{str}; + try + { + info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION, antalya_version); + FAIL() << "External storage accepted by the old protocol"; + } + catch (const DB::Exception & e) + { + ASSERT_EQ(e.code(), ErrorCodes::PROTOCOL_VERSION_MISMATCH); + } + } +} + + +TEST(DatalakeStateSerde, IcebergObjectSerializableInfoDeleteFilePathsByAntalyaVersion) +{ + auto info = makeObjectInfo(); + info.position_deletes_objects = {{"s3://bucket/deletes/pos1.parquet", "PARQUET", "s3://bucket/path/to/file.parquet", 0}}; + info.equality_deletes_objects = {{"s3://bucket/deletes/eq1.parquet", "PARQUET", std::vector{1}, 1}}; + + for (UInt64 antalya_version : {0, 1, 2}) + { + SCOPED_TRACE(antalya_version); + info.requires_external_storage = (antalya_version == 2); + String str; + { + WriteBufferFromString write_buffer{str}; + info.serializeForClusterFunctionProtocol(write_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION, antalya_version); + } + ReadBufferFromMemory read_buffer(str.data(), str.size()); + Iceberg::IcebergObjectSerializableInfo deserialized; + deserialized.deserializeForClusterFunctionProtocol(read_buffer, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION); + + ASSERT_TRUE(read_buffer.eof()); + ASSERT_EQ(deserialized.data_object_file_path_key, info.data_object_file_path_key); + ASSERT_EQ(deserialized.position_deletes_objects.size(), 1); + ASSERT_EQ(deserialized.equality_deletes_objects.size(), 1); + const String prefix = antalya_version == 2 ? "s3://bucket/" : ""; + EXPECT_EQ(deserialized.position_deletes_objects[0].file_path, prefix + "deletes/pos1.parquet"); + EXPECT_EQ(deserialized.position_deletes_objects[0].reference_data_file_path, prefix + "path/to/file.parquet"); + EXPECT_EQ(deserialized.equality_deletes_objects[0].file_path, prefix + "deletes/eq1.parquet"); + } +} + +#if USE_AVRO && USE_AWS_S3 +TEST(DatalakeStateSerde, CopiedTableDeletePathsRequireAbsolutePathProtocol) +{ + const auto storage = std::make_shared(LocalObjectStorageSettings("test", "./", true)); + ExternalStorageCache external_storages; + const String location = "s3://old-bucket/prefix/table"; + const String path = location + "/data/delete.parquet"; + const Iceberg::IcebergPathResolver resolver(location, "table"); + + const auto [resolved_storage, key] = resolveObjectStorageForPath(location, path, storage, external_storages, {}, resolver); + ASSERT_EQ(resolved_storage, storage); + ASSERT_EQ(key, "table/data/delete.parquet"); + EXPECT_TRUE(requiresAbsolutePathForCluster(location, path, storage, external_storages, {}, resolver)); + EXPECT_FALSE(requiresAbsolutePathForCluster(location, key, storage, external_storages, {}, resolver)); + + const Iceberg::IcebergPathResolver original_resolver(location, "prefix/table"); + EXPECT_FALSE(requiresAbsolutePathForCluster(location, path, storage, external_storages, {}, original_resolver)); +} +#endif diff --git a/src/Storages/ObjectStorage/tests/gtest_rendezvous_hashing.cpp b/src/Storages/ObjectStorage/tests/gtest_rendezvous_hashing.cpp index 1c4216c61329b..50ce5453c4c97 100644 --- a/src/Storages/ObjectStorage/tests/gtest_rendezvous_hashing.cpp +++ b/src/Storages/ObjectStorage/tests/gtest_rendezvous_hashing.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -305,7 +306,7 @@ TEST(ClusterFunctionReadTaskResponse, PreservesReadSourceIndex) String serialized; WriteBufferFromString out(serialized); - response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION); + response.serialize(out, DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION, DBMS_ANTALYA_PROTOCOL_VERSION); out.finalize(); ReadBufferFromString in(serialized); 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 295dd645286a0..03db0d0519d41 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/src/Storages/System/StorageSystemIcebergFiles.cpp b/src/Storages/System/StorageSystemIcebergFiles.cpp index 92de0c6eb2c97..6541127e2a924 100644 --- a/src/Storages/System/StorageSystemIcebergFiles.cpp +++ b/src/Storages/System/StorageSystemIcebergFiles.cpp @@ -467,7 +467,7 @@ StorageSystemIcebergFiles::StorageSystemIcebergFiles(const StorageID & table_id_ {"table", std::make_shared(), "Table name."}, {"snapshot_id", std::make_shared(), "Snapshot ID at which the file was added."}, {"content", content_enum, "File content kind."}, - {"file_path", std::make_shared(), "Resolved storage path of the file."}, + {"file_path", std::make_shared(), "Path of the file as written in the manifest, which may be an absolute URI outside the table location."}, {"file_format", std::make_shared(), "File format, e.g. 'PARQUET'."}, {"record_count", std::make_shared(), "Number of records in the file."}, {"file_size_in_bytes", std::make_shared(), "Size of the file in bytes."}, diff --git a/src/TableFunctions/ITableFunction.cpp b/src/TableFunctions/ITableFunction.cpp index f55ab64d7ce64..81b446e09d87f 100644 --- a/src/TableFunctions/ITableFunction.cpp +++ b/src/TableFunctions/ITableFunction.cpp @@ -41,7 +41,7 @@ void ITableFunction::checkSourceAccess(ContextPtr context, bool is_insert_query) if (is_insert_query) type_to_check = AccessType::WRITE; - context->getAccess()->checkAccessWithFilter(type_to_check, toStringSource(*access_object), getFunctionURINormalized()); + context->getAccess()->checkAccessWithFilter(type_to_check, toStringSource(*access_object), getFunctionURI()); } } @@ -97,18 +97,4 @@ StoragePtr ITableFunction::execute(const ASTPtr & ast_function, ContextPtr conte std::move(cached_columns), needStructureConversion()); } -String ITableFunction::getFunctionURINormalized() const -{ - try - { - Poco::URI uri(getFunctionURI()); - uri.normalize(); - return uri.toString(); - } - catch (const Poco::Exception &) - { - return ""; - } -} - } diff --git a/src/TableFunctions/ITableFunction.h b/src/TableFunctions/ITableFunction.h index 1d28c050b2b5b..d38861df6c6f0 100644 --- a/src/TableFunctions/ITableFunction.h +++ b/src/TableFunctions/ITableFunction.h @@ -152,8 +152,6 @@ class ITableFunction : public std::enable_shared_from_this /// privileges as if it was S3. virtual const char * getNonClusteredStorageEngineName() const; -protected: - String getFunctionURINormalized() const; }; /// Properties of table function that are independent of argument types and parameters. 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 5cb1c02a0c07a..9a60da2b23015 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/integration/test_storage_iceberg_with_spark/configs/config.d/remote_host_filter.xml b/tests/integration/test_storage_iceberg_with_spark/configs/config.d/remote_host_filter.xml new file mode 100644 index 0000000000000..2f9488f79fb38 --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/configs/config.d/remote_host_filter.xml @@ -0,0 +1,5 @@ + + + minio1 + + diff --git a/tests/integration/test_storage_iceberg_with_spark/configs/users.d/iceberg_delete_data_on_drop.xml b/tests/integration/test_storage_iceberg_with_spark/configs/users.d/iceberg_delete_data_on_drop.xml new file mode 100644 index 0000000000000..b2eac80ba1f96 --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/configs/users.d/iceberg_delete_data_on_drop.xml @@ -0,0 +1,9 @@ + + + + + 1 + + + diff --git a/tests/integration/test_storage_iceberg_with_spark/external_paths_utils.py b/tests/integration/test_storage_iceberg_with_spark/external_paths_utils.py new file mode 100644 index 0000000000000..96520146568ab --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/external_paths_utils.py @@ -0,0 +1,441 @@ +import os +import json +import shutil +import tempfile +import avro.datafile +import avro.io + +from helpers.s3_tools import AzureUploader, S3Uploader +from helpers.iceberg_utils import default_download_directory, default_upload_directory + + +def modify_avro_file(avro_path: str, field_path: list, modifier_func) -> None: + with open(avro_path, 'rb') as f: + reader = avro.datafile.DataFileReader(f, avro.io.DatumReader()) + schema = reader.datum_reader.writers_schema + 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: + 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: + relative_path = relative_path.lstrip("/") + + if storage_type == "s3": + return f"s3a://{cluster.minio_bucket}/{relative_path}" + elif storage_type.startswith("s3:"): + bucket = storage_type.split(":")[1] + return f"s3a://{bucket}/{relative_path}" + elif storage_type.startswith("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:"): + 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_query_args(metadata_storage: str, cluster, table_path: str): + 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: + 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 external_bucket(started_cluster, index: int = 1) -> str: + name = f"{started_cluster.minio_bucket}-storage{index}" + if not started_cluster.minio_client.bucket_exists(name): + started_cluster.minio_client.make_bucket(name) + return name + + +def path_modifier(old_path: str, new_storage: str, cluster, base_path: str): + if "://" in old_path: + parts = old_path.split("/") + 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) + + +def create_and_upload_table(cluster, table_name): + spark = cluster.spark_session + 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(cluster, "s3", f"/iceberg_data/default/{table_name}/", f"/iceberg_data/default/{table_name}/") + + +def _download_table_for_relocation(started_cluster, 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}" + return temp_dir, host_path, base_path + + +def _distribute_table_components(started_cluster, table_name, metadata_storage, manifest_list_storage, + manifest_storage, data_storage): + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, 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) + 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) + + def upload(files, storage): + uploader = get_uploader(storage, started_cluster) + for f in files: + uploader.upload_file(f, f"{base_path}/{os.path.relpath(f, host_path)}") + + upload(find_files(metadata_dir, ".metadata.json") + find_files(metadata_dir, "version-hint.text"), metadata_storage) + upload(manifest_list_files, manifest_list_storage) + upload(manifest_files, manifest_storage) + upload(find_files(data_dir, ".parquet") if os.path.exists(data_dir) else [], data_storage) + + shutil.rmtree(temp_dir) + return base_path + + +def _move_files_to_bucket(started_cluster, files, bucket, host_path, base_path, destination_prefix=None): + uploader = S3Uploader(started_cluster.minio_client, bucket) + for f in files: + rel = os.path.relpath(f, host_path) + destination = f"{destination_prefix}/{os.path.basename(f)}" if destination_prefix else f"{base_path}/{rel}" + uploader.upload_file(f, destination) + 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): + 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 _delete_file_names(metadata_dir: str) -> set: + names = set() + for mf in find_files(metadata_dir, ".avro"): + if os.path.basename(mf).startswith("snap-"): + continue + with open(mf, 'rb') as f: + reader = avro.datafile.DataFileReader(f, avro.io.DatumReader()) + try: + for record in reader: + data_file = record.get("data_file", {}) + if data_file.get("content", 0) != 0: + names.add(os.path.basename(data_file["file_path"])) + finally: + reader.close() + return names + + +def _rewrite_manifests_and_reupload(started_cluster, host_path, base_path, file_path_modifier): + metadata_dir = os.path.join(host_path, "metadata") + + # Relocating data also requires rewriting position-delete rows and bounds; move delete files instead. + assert not _delete_file_names(metadata_dir), \ + f"{host_path} has delete files; relocating its data files would silently disable them" + + 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, prefix=None, endpoint=None): + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + prefix = prefix if prefix is not None else f"{base_path}/data" + uri = f"{endpoint}/{data_bucket}" if endpoint else f"s3a://{data_bucket}" + _rewrite_manifests_and_reupload( + started_cluster, host_path, base_path, + lambda p: f"{uri}/{prefix}/{os.path.basename(p)}") + _move_files_to_bucket( + started_cluster, find_files(os.path.join(host_path, "data"), ".parquet"), + data_bucket, host_path, base_path, prefix) + shutil.rmtree(temp_dir) + return base_path + + +def relocate_delete_files_to_bucket(started_cluster, table_name, delete_bucket): + delete_storage = f"s3:{delete_bucket}" + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + metadata_dir = os.path.join(host_path, "metadata") + delete_names = _delete_file_names(metadata_dir) + assert delete_names, f"{table_name} has no delete files to relocate" + + def to_delete_bucket(old_path): + if os.path.basename(old_path) not in delete_names: + return old_path + return path_modifier(old_path, delete_storage, started_cluster, base_path) + + for mf in find_files(metadata_dir, ".avro"): + if os.path.basename(mf).startswith("snap-"): + continue + modify_avro_file(mf, ["data_file", "file_path"], to_delete_bucket) + rel = os.path.relpath(mf, host_path) + started_cluster.default_s3_uploader.upload_file(mf, f"{base_path}/{rel}") + + delete_files = [f for f in find_files(os.path.join(host_path, "data"), ".parquet") + if os.path.basename(f) in delete_names] + _move_files_to_bucket(started_cluster, delete_files, delete_bucket, host_path, base_path) + + shutil.rmtree(temp_dir) + return base_path + + +def _external_delete_files_in_another_bucket(started_cluster, table_name): + spark = started_cluster.spark_session + bucket = external_bucket(started_cluster) + + spark.sql( + f"CREATE TABLE {table_name} (id INT, value STRING) USING iceberg " + f"TBLPROPERTIES ('format-version'='2', 'write.delete.mode'='merge-on-read')") + spark.sql(f"INSERT INTO {table_name} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + spark.sql(f"DELETE FROM {table_name} WHERE id = 2") + + default_upload_directory(started_cluster, "s3", f"/iceberg_data/default/{table_name}/", f"/iceberg_data/default/{table_name}/") + base_path = relocate_delete_files_to_bucket(started_cluster, table_name, bucket) + return base_path, bucket, f"{base_path}/" + + +def _external_manifest_lists_in_another_bucket(started_cluster, table_name): + spark = started_cluster.spark_session + bucket = external_bucket(started_cluster) + + 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')") + spark.sql(f"INSERT INTO {table_name} VALUES (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, bucket) + return base_path, bucket, f"{base_path}/" + + +def _external_data_files_in_another_bucket(started_cluster, table_name): + bucket = external_bucket(started_cluster) + + create_and_upload_table(started_cluster, table_name) + base_path = relocate_data_files_to_bucket(started_cluster, table_name, bucket) + return base_path, bucket, f"{base_path}/data/" + + +def _external_data_files_in_the_same_bucket(started_cluster, table_name): + external_prefix = f"external_data/{table_name}" + + create_and_upload_table(started_cluster, table_name) + base_path = relocate_data_files_to_bucket(started_cluster, table_name, started_cluster.minio_bucket, prefix=external_prefix) + return base_path, started_cluster.minio_bucket, f"{external_prefix}/" + + +REMOVE_ORPHAN_FILES = "ALTER TABLE {table} EXECUTE remove_orphan_files(older_than = '2020-01-01 00:00:00', dry_run = 1)" +REMOVE_ORPHAN_FILES_SETTINGS = {"allow_insert_into_iceberg": 1, "allow_iceberg_remove_orphan_files": 1} +ALL_ROWS = "1\talpha\n2\tbeta\n3\tgamma\n" + + +def _manifest_lists_delete_files(avro_path: str) -> bool: + with open(avro_path, 'rb') as f: + reader = avro.datafile.DataFileReader(f, avro.io.DatumReader()) + try: + return any(record.get("data_file", {}).get("content", 0) != 0 for record in reader) + finally: + reader.close() + + +def _rewrite_paths_to_local_uri(started_cluster, table_name, authority, deletes_only=False, target_dir=None): + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, table_name) + metadata_dir = os.path.join(host_path, "metadata") + data_dir = os.path.join(host_path, "data") + + relocated = set() + + def to_local(old_path): + name = os.path.basename(old_path) + relocated.add(name) + if target_dir: + return f"file://{authority}{target_dir}/{name}" + return f"file://{authority}/{base_path}/data/{name}" + + for mf in find_files(metadata_dir, ".avro"): + if os.path.basename(mf).startswith("snap-"): + continue + if deletes_only and not _manifest_lists_delete_files(mf): + continue + modify_avro_file(mf, ["data_file", "file_path"], to_local) + rel = os.path.relpath(mf, host_path) + started_cluster.default_s3_uploader.upload_file(mf, f"{base_path}/{rel}") + + assert relocated, f"Nothing to relocate for {table_name}" + + if target_dir: + started_cluster.instances["node1"].exec_in_container(["mkdir", "-p", target_dir]) + + for f in find_files(data_dir, ".parquet"): + if os.path.basename(f) not in relocated: + continue + rel = os.path.relpath(f, host_path) + local_path = f"{target_dir}/{os.path.basename(f)}" if target_dir else f"/{base_path}/{rel}" + upload_path = f"/{base_path}/{rel}" if target_dir else local_path + started_cluster.default_local_uploader.upload_file(f, upload_path) + if target_dir: + started_cluster.instances["node1"].exec_in_container( + ["mv", upload_path, local_path]) + started_cluster.minio_client.remove_object(started_cluster.minio_bucket, f"{base_path}/{rel}") + + shutil.rmtree(temp_dir) + return base_path + + +def _create_iceberg_s3_table(started_cluster, table_name, base_path): + instance = started_cluster.instances["node1"] + args = get_query_args("s3", started_cluster, base_path) + instance.query(f"DROP TABLE IF EXISTS {table_name}") + instance.query(f"CREATE TABLE {table_name} ENGINE=IcebergS3({args})") + + +def _check_cluster_function_rejects_table(started_cluster, table_name, base_path, expected_rows): + instance = started_cluster.instances["node1"] + args = get_query_args("s3", started_cluster, base_path) + error = instance.query_and_get_error( + f"SELECT * FROM icebergS3Cluster('cluster_simple', {args}) ORDER BY id") + assert "cannot be read by a cluster function" in error + + _create_iceberg_s3_table(started_cluster, table_name, base_path) + assert instance.query(f"SELECT * FROM {table_name} ORDER BY id") == expected_rows + instance.query(f"DROP TABLE {table_name} SYNC") + + +def _relocate_local_data_files_outside_table_directory(started_cluster, table_name, external_dir, scheme): + instance = started_cluster.instances["node1"] + + host_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") + + for metadata_json in find_files(metadata_dir, ".metadata.json"): + with open(metadata_json) as f: + metadata = json.load(f) + metadata["location"] = f"{scheme}{host_path}" + with open(metadata_json, "w") as f: + json.dump(metadata, f) + + for manifest in [f for f in find_files(metadata_dir, ".avro") if not os.path.basename(f).startswith("snap-")]: + modify_avro_file(manifest, ["data_file", "file_path"], lambda p: f"{scheme}{external_dir}/{os.path.basename(p)}") + + default_upload_directory(started_cluster, "local", f"/iceberg_data/default/{table_name}/", f"/iceberg_data/default/{table_name}/") + for f in find_files(data_dir, ".parquet"): + started_cluster.default_local_uploader.upload_file(f, f"{external_dir}/{os.path.basename(f)}") + instance.exec_in_container(["bash", "-c", f"rm -f {host_path}/data/*.parquet"]) + + return host_path diff --git a/tests/integration/test_storage_iceberg_with_spark/test_expire_snapshots.py b/tests/integration/test_storage_iceberg_with_spark/test_expire_snapshots.py index 0868185addbf1..dae337cd8ef22 100644 --- a/tests/integration/test_storage_iceberg_with_spark/test_expire_snapshots.py +++ b/tests/integration/test_storage_iceberg_with_spark/test_expire_snapshots.py @@ -189,7 +189,8 @@ def test_expire_snapshots_basic(started_cluster_iceberg_with_spark, storage_type result = expire_snapshots(instance, TABLE_NAME, expire_timestamp) counts = parse_expire_result(result) - assert len(counts) == 7, f"Expected 7 metrics, got {counts}" + assert len(counts) == 8, f"Expected 8 metrics, got {counts}" + assert counts["failed_deletions_count"] == 0, f"Nothing should have failed to delete, got {counts}" assert all(v >= 0 for v in counts.values()), f"All counts should be non-negative, got {counts}" assert_data_intact(instance, TABLE_NAME, 4) @@ -216,7 +217,8 @@ def test_expire_snapshots_positional_timestamp(started_cluster_iceberg_with_spar settings=ICEBERG_SETTINGS, ) counts = parse_expire_result(result) - assert len(counts) == 7, f"Expected 7 metrics, got {counts}" + assert len(counts) == 8, f"Expected 8 metrics, got {counts}" + assert counts["failed_deletions_count"] == 0, f"Nothing should have failed to delete, got {counts}" assert all(v >= 0 for v in counts.values()), f"All counts should be non-negative, got {counts}" assert_data_intact(instance, TABLE_NAME, 4) @@ -939,6 +941,129 @@ def share_s1_manifest_list(m): "Expected at least one manifest list deleted (ML1 shared by S1 and S2)" +def test_expire_snapshots_keeps_anchors_of_failed_deletion(started_cluster_iceberg_with_spark): + storage_type = "local" + instance = started_cluster_iceberg_with_spark.instances["node1"] + TABLE_NAME = make_table_name("test_expire_keeps_anchors", storage_type) + + create_and_populate( + started_cluster_iceberg_with_spark, instance, storage_type, TABLE_NAME, 3 + ) + + snap_ids = get_snapshot_ids(instance, TABLE_NAME) + assert len(snap_ids) == 3, f"Expected 3 snapshots, got {snap_ids}" + s1_id = snap_ids[0] + + # Rewind to make newer snapshots non-ancestors, so expiration removes their data and manifests. + def rewind_to_first_snapshot(m): + m["current-snapshot-id"] = s1_id + for ref in m.get("refs", {}).values(): + ref["snapshot-id"] = s1_id + m.setdefault("properties", {}).update(AGGRESSIVE_RETENTION) + + update_iceberg_metadata(instance, TABLE_NAME, rewind_to_first_snapshot) + + meta_before = read_iceberg_metadata(instance, TABLE_NAME) + expired_manifest_lists = [ + os.path.basename(s["manifest-list"]) for s in meta_before["snapshots"] if s["snapshot-id"] != s1_id + ] + assert len(expired_manifest_lists) == 2, f"Expected 2 expired manifest lists, got {expired_manifest_lists}" + + metadata_dir = f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}/metadata" + + def list_avro(): + return set(instance.exec_in_container( + ["bash", "-c", f"ls {metadata_dir}/*.avro 2>/dev/null || true"] + ).split()) + + avro_before = list_avro() + + instance.query("SYSTEM ENABLE FAILPOINT local_object_storage_network_error_during_remove") + try: + counts = parse_expire_result(expire_snapshots(instance, TABLE_NAME, FAR_FUTURE)) + finally: + instance.query("SYSTEM DISABLE FAILPOINT local_object_storage_network_error_during_remove") + + assert counts["deleted_data_files_count"] >= 1, f"Expected expired data files, got {counts}" + assert counts["deleted_manifest_files_count"] >= 1, f"Expected expired manifests, got {counts}" + + # One failed data deletion must retain both its manifest and manifest list. + assert counts["failed_deletions_count"] == 3, f"Expected the leaf and its two anchors, got {counts}" + + reported_avro = counts["deleted_manifest_files_count"] + counts["deleted_manifest_lists_count"] + deleted_avro = avro_before - list_avro() + assert len(deleted_avro) == reported_avro, ( + f"Reported {reported_avro} deleted .avro files, but {len(deleted_avro)} disappeared" + ) + + kept = [ml for ml in expired_manifest_lists if any(ml in f for f in list_avro())] + assert len(kept) == 1, f"Exactly one expired manifest list should have survived, kept {kept}" + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY x") == "1\n" + + +@pytest.mark.parametrize("missing_anchor", ["manifest_list", "manifest"]) +def test_expire_snapshots_missing_anchor_does_not_commit( + started_cluster_iceberg_with_spark, missing_anchor +): + instance = started_cluster_iceberg_with_spark.instances["node1"] + TABLE_NAME = make_table_name("test_expire_missing_anchor", "local") + + create_and_populate( + started_cluster_iceberg_with_spark, instance, "local", TABLE_NAME, 3 + ) + + snapshot_ids = get_snapshot_ids(instance, TABLE_NAME) + retained_snapshot_id = snapshot_ids[0] + + def rewind_to_first_snapshot(m): + m["current-snapshot-id"] = retained_snapshot_id + for ref in m.get("refs", {}).values(): + ref["snapshot-id"] = retained_snapshot_id + m.setdefault("properties", {}).update(AGGRESSIVE_RETENTION) + + update_iceberg_metadata(instance, TABLE_NAME, rewind_to_first_snapshot) + + metadata = read_iceberg_metadata(instance, TABLE_NAME) + retained_manifest_list = next( + snapshot["manifest-list"] + for snapshot in metadata["snapshots"] + if snapshot["snapshot-id"] == retained_snapshot_id + ) + expired_manifest_list = next( + snapshot["manifest-list"] + for snapshot in metadata["snapshots"] + if snapshot["snapshot-id"] != retained_snapshot_id + ) + + def manifest_paths(manifest_list): + return set( + instance.query( + f"SELECT manifest_path FROM file('{manifest_list}', Avro) FORMAT TSV" + ) + .strip() + .splitlines() + ) + + if missing_anchor == "manifest_list": + anchor_to_remove = expired_manifest_list + else: + retained_manifests = manifest_paths(retained_manifest_list) + expired_only_manifests = manifest_paths(expired_manifest_list) - retained_manifests + assert expired_only_manifests, "Expected an expired-only manifest" + anchor_to_remove = next(iter(expired_only_manifests)) + + instance.exec_in_container(["rm", anchor_to_remove]) + + error = instance.query_and_get_error( + f"ALTER TABLE {TABLE_NAME} EXECUTE expire_snapshots(expire_before = '{FAR_FUTURE}');", + settings=ICEBERG_SETTINGS, + ) + assert error + assert os.path.basename(anchor_to_remove) in error + assert get_snapshot_ids(instance, TABLE_NAME) == snapshot_ids + + # --------------------------------------------------------------------------- # Coverage gap tests (PR #99130) # --------------------------------------------------------------------------- diff --git a/tests/integration/test_storage_iceberg_with_spark/test_external_paths_access.py b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_access.py new file mode 100644 index 0000000000000..8c1be2f7c3aa6 --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_access.py @@ -0,0 +1,186 @@ + +from helpers.iceberg_utils import get_uuid_str +from .external_paths_utils import ( + ALL_ROWS, + _create_iceberg_s3_table, + _rewrite_paths_to_local_uri, + create_and_upload_table, + external_bucket, + relocate_data_files_to_bucket, +) + + +def test_propagate_credentials_to_other_endpoint(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_propagate_creds_{get_uuid_str()}" + data_bucket = external_bucket(started_cluster_iceberg_with_spark) + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + base_path = relocate_data_files_to_bucket( + started_cluster_iceberg_with_spark, TABLE_NAME, data_bucket, + endpoint=f"http://{started_cluster_iceberg_with_spark.minio_ip}:{started_cluster_iceberg_with_spark.minio_port}") + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + table_function = f"icebergS3(s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{started_cluster_iceberg_with_spark.minio_bucket}/')" + + error = instance.query_and_get_error(f"SELECT * FROM {table_function} ORDER BY id") + assert "not allowed to use the server's own credentials" in error, error + + error = instance.query_and_get_error( + f"SELECT * FROM {table_function} ORDER BY id " + f"SETTINGS object_storage_propagate_credentials_to_other_storages = 1") + assert "not allowed to use the server's own credentials" in error, error + + result = instance.query( + f"SELECT * FROM {table_function} ORDER BY id " + f"SETTINGS object_storage_propagate_credentials_to_other_storages = 1, " + f"s3_allow_server_credentials_in_user_queries = 1") + assert result == ALL_ROWS + + +def test_same_bucket_external_path_requires_source_grant(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_same_bucket_grant_{get_uuid_str()}" + base_bucket = started_cluster_iceberg_with_spark.minio_bucket + external_prefix = f"external_data/{TABLE_NAME}" + user = f"user_{TABLE_NAME}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + base_path = relocate_data_files_to_bucket(started_cluster_iceberg_with_spark, TABLE_NAME, base_bucket, prefix=external_prefix) + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + table_function = (f"icebergS3('{minio_url}/{base_bucket}/{base_path}/', " + f"'minio', 'ClickHouse_Minio_P@ssw0rd')") + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + iceberg_files_query = ( + f"SELECT file_path FROM system.iceberg_files " + f"WHERE database = 'default' AND table = '{TABLE_NAME}' AND content = 'DATA' ORDER BY file_path") + expected_paths = instance.query(iceberg_files_query) + assert expected_paths + + instance.query(f"DROP USER IF EXISTS {user}") + instance.query(f"CREATE USER {user}") + instance.query(f"GRANT CREATE TEMPORARY TABLE ON *.* TO {user}") + instance.query(f"GRANT SHOW TABLES ON {TABLE_NAME} TO {user}") + instance.query(f"GRANT SELECT ON system.iceberg_files TO {user}") + instance.query(f"GRANT READ ON S3('{minio_url}/{base_bucket}/{base_path}/.*') TO {user}") + + error = instance.query_and_get_error(f"SELECT * FROM {table_function} ORDER BY id", user=user) + assert "ACCESS_DENIED" in error, error + + error = instance.query_and_get_error(f"SELECT count() FROM {table_function}", user=user) + assert "ACCESS_DENIED" in error, error + + assert instance.query(iceberg_files_query, user=user) == "" + + instance.query(f"GRANT READ ON S3('s3a://{base_bucket}/{external_prefix}/.*') TO {user}") + assert instance.query(f"SELECT * FROM {table_function} ORDER BY id", user=user) == ALL_ROWS + assert instance.query(f"SELECT count() FROM {table_function}", user=user) == "3\n" + assert instance.query(iceberg_files_query, user=user) == expected_paths + + instance.query(f"DROP USER {user}") + instance.query(f"DROP TABLE {TABLE_NAME}") + + +def test_external_local_file_requires_file_grant(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_local_file_grant_{get_uuid_str()}" + base_bucket = started_cluster_iceberg_with_spark.minio_bucket + target_dir = f"/var/lib/clickhouse/user_files/external data/{TABLE_NAME}" + user = f"user_{TABLE_NAME}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = _rewrite_paths_to_local_uri( + started_cluster_iceberg_with_spark, TABLE_NAME, "", target_dir=target_dir) + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + table_function = (f"icebergS3('{minio_url}/{base_bucket}/{base_path}/', " + f"'minio', 'ClickHouse_Minio_P@ssw0rd')") + + instance.query(f"DROP USER IF EXISTS {user}") + instance.query(f"CREATE USER {user}") + instance.query(f"GRANT CREATE TEMPORARY TABLE ON *.* TO {user}") + instance.query(f"GRANT READ ON S3 TO {user}") + + error = instance.query_and_get_error(f"SELECT * FROM {table_function} ORDER BY id", user=user) + assert "ACCESS_DENIED" in error, error + + instance.query(f"GRANT READ ON FILE('{target_dir}/.*') TO {user}") + assert instance.query(f"SELECT * FROM {table_function} ORDER BY id", user=user) == ALL_ROWS + + instance.query(f"DROP USER {user}") + + +def test_external_local_file_grant_rejects_parent_traversal(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_local_file_grant_escape_{get_uuid_str()}" + base_bucket = started_cluster_iceberg_with_spark.minio_bucket + path_root = f"/var/lib/clickhouse/user_files/external data/{TABLE_NAME}" + granted_dir = f"{path_root}/granted" + target_dir = f"{granted_dir}/../outside" + user = f"user_{TABLE_NAME}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = _rewrite_paths_to_local_uri( + started_cluster_iceberg_with_spark, TABLE_NAME, "", target_dir=target_dir) + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + table_function = (f"icebergS3('{minio_url}/{base_bucket}/{base_path}/', " + f"'minio', 'ClickHouse_Minio_P@ssw0rd')") + + instance.query(f"DROP USER IF EXISTS {user}") + instance.query(f"CREATE USER {user}") + instance.query(f"GRANT CREATE TEMPORARY TABLE ON *.* TO {user}") + instance.query(f"GRANT READ ON S3 TO {user}") + instance.query(f"GRANT READ ON FILE('{granted_dir}/.*') TO {user}") + + error = instance.query_and_get_error(f"SELECT * FROM {table_function} ORDER BY id", user=user) + assert "ACCESS_DENIED" in error, error + + instance.query(f"GRANT READ ON FILE('{path_root}/outside/.*') TO {user}") + assert instance.query(f"SELECT * FROM {table_function} ORDER BY id", user=user) == ALL_ROWS + + instance.query(f"DROP USER {user}") + + +def test_external_local_file_grant_rejects_symlink_alias(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_local_file_grant_symlink_{get_uuid_str()}" + base_bucket = started_cluster_iceberg_with_spark.minio_bucket + path_root = f"/var/lib/clickhouse/user_files/external data/{TABLE_NAME}" + granted_dir = f"{path_root}/granted" + user = f"user_{TABLE_NAME}" + + instance.exec_in_container(["mkdir", "-p", f"{path_root}/outside", granted_dir]) + instance.exec_in_container(["ln", "-s", "../outside", f"{granted_dir}/link"]) + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = _rewrite_paths_to_local_uri( + started_cluster_iceberg_with_spark, TABLE_NAME, "", target_dir=f"{granted_dir}/link") + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + table_function = (f"icebergS3('{minio_url}/{base_bucket}/{base_path}/', " + f"'minio', 'ClickHouse_Minio_P@ssw0rd')") + + instance.query(f"DROP USER IF EXISTS {user}") + instance.query(f"CREATE USER {user}") + instance.query(f"GRANT CREATE TEMPORARY TABLE ON *.* TO {user}") + instance.query(f"GRANT READ ON S3 TO {user}") + instance.query(f"GRANT READ ON FILE('{granted_dir}/.*') TO {user}") + + error = instance.query_and_get_error(f"SELECT * FROM {table_function} ORDER BY id", user=user) + assert "ACCESS_DENIED" in error, error + + instance.query(f"GRANT READ ON FILE('{path_root}/outside/.*') TO {user}") + assert instance.query(f"SELECT * FROM {table_function} ORDER BY id", user=user) == ALL_ROWS + + instance.query(f"DROP USER {user}") diff --git a/tests/integration/test_storage_iceberg_with_spark/test_external_paths_cross_bucket.py b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_cross_bucket.py new file mode 100644 index 0000000000000..b660822b4ce1f --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_cross_bucket.py @@ -0,0 +1,260 @@ +import os +import shutil +import time +import pytest + +from helpers.s3_tools import S3Uploader +from helpers.iceberg_utils import default_upload_directory, get_uuid_str +from .external_paths_utils import ( + ALL_ROWS, + create_and_upload_table, + _create_iceberg_s3_table, + _distribute_table_components, + _download_table_for_relocation, + _external_delete_files_in_another_bucket, + _rewrite_manifests_and_reupload, + external_bucket, + find_files, + get_query_args, + modify_avro_file, + read_avro_records, + relocate_data_files_to_bucket, +) + + +@pytest.mark.parametrize("spelling", ["s3", "url"]) +def test_four_different_s3_buckets(started_cluster_iceberg_with_spark, spelling): + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + + TABLE_NAME = f"test_four_buckets_{spelling}_{get_uuid_str()}" + buckets = [ + started_cluster_iceberg_with_spark.minio_bucket, + external_bucket(started_cluster_iceberg_with_spark, 1), + external_bucket(started_cluster_iceberg_with_spark, 2), + external_bucket(started_cluster_iceberg_with_spark, 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_iceberg_with_spark, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + base_path = _distribute_table_components(started_cluster_iceberg_with_spark, TABLE_NAME, f"s3:{buckets[0]}", + *(f"{spelling}:{b}" for b in buckets[1:])) + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.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" + + +def test_num_rows_cache_no_collision_across_buckets(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + base_bucket = started_cluster_iceberg_with_spark.minio_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 referencing only that file. Live position deletes make + # `count()` scan the data files instead of answering from the manifests, so the first data + # file -- relocated below, with no deletes applied -- goes through the num-rows cache. The + # second file keeps a surviving row: deleting every row of a file drops it from the manifest + # without writing a delete file. + 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_iceberg_with_spark, "s3", f"/iceberg_data/default/{table_name}/", f"/iceberg_data/default/{table_name}/") + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster_iceberg_with_spark, 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. + 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, + ) + # Without `value_counts` the per-file row count is not answered from the manifest, which + # would bypass the num-rows cache under test. + modify_avro_file(mf, ["data_file", "value_counts"], lambda _: None) + + for f in manifest_files: + rel = os.path.relpath(f, host_path) + started_cluster_iceberg_with_spark.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_iceberg_with_spark.minio_client, data_bucket).upload_file(local_relocated[0], shared_key) + + shutil.rmtree(temp_dir) + return base_path + + # Upload both files before populating the cache: entries are reused only for older files. + base_path_a = prepare_table( + f"test_count_cache_a_{get_uuid_str()}", "(1, 'a'), (2, 'b'), (3, 'c')", 3, external_bucket(started_cluster_iceberg_with_spark, 1) + ) + base_path_b = prepare_table( + f"test_count_cache_b_{get_uuid_str()}", "(1, 'a'), (2, 'b'), (3, 'c'), (4, 'd'), (5, 'e')", 5, external_bucket(started_cluster_iceberg_with_spark, 2) + ) + # Allow 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 + + # Each table holds its relocated file plus the surviving row of the second file, 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" + assert cache_lookups_a >= 1 + assert cache_lookups_b >= 1 + + +def test_external_path_virtual_column_filter(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_path_filter_{get_uuid_str()}" + data_bucket = external_bucket(started_cluster_iceberg_with_spark) + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = relocate_data_files_to_bucket(started_cluster_iceberg_with_spark, TABLE_NAME, data_bucket) + + minio_url = f"http://{started_cluster_iceberg_with_spark.minio_host}:{started_cluster_iceberg_with_spark.minio_port}" + args = f"s3, filename='{base_path}/', format=Parquet, url='{minio_url}/{started_cluster_iceberg_with_spark.minio_bucket}/'" + + paths = instance.query(f"SELECT DISTINCT _path FROM icebergS3({args})").strip().splitlines() + assert len(paths) == 1 + external_path = paths[0] + assert data_bucket in external_path + + assert instance.query( + f"SELECT count() FROM icebergS3Cluster('cluster_simple', {args}) WHERE _path = '{external_path}' " + "SETTINGS cluster_table_function_split_granularity = 'bucket'" + ).strip() == "3" + + +def test_gs_path_is_not_served_by_same_named_s3_bucket(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_gs_same_bucket_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster_iceberg_with_spark, TABLE_NAME) + _rewrite_manifests_and_reupload( + started_cluster_iceberg_with_spark, host_path, base_path, + lambda p: f"gs://{started_cluster_iceberg_with_spark.minio_bucket}/{base_path}/data/{os.path.basename(p)}") + shutil.rmtree(temp_dir) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + # GCS is unreachable here; a successful read would mean the MinIO bucket was used incorrectly. + error = instance.query_and_get_error( + f"SELECT * FROM {TABLE_NAME} ORDER BY id " + f"SETTINGS s3_request_timeout_ms = 3000, s3_connect_timeout_ms = 3000") + assert "alpha" not in error + + instance.query(f"DETACH TABLE {TABLE_NAME}") + + +def test_unsupported_scheme_is_still_reported_by_system_iceberg_files(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_unsupported_scheme_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster_iceberg_with_spark, TABLE_NAME) + _rewrite_manifests_and_reupload( + started_cluster_iceberg_with_spark, host_path, base_path, + lambda p: f"viewfs://cluster/{base_path}/data/{os.path.basename(p)}") + shutil.rmtree(temp_dir) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + assert "Unsupported storage scheme" in instance.query_and_get_error(f"SELECT * FROM {TABLE_NAME} ORDER BY id") + + paths = instance.query( + f"SELECT file_path FROM system.iceberg_files " + f"WHERE database = 'default' AND table = '{TABLE_NAME}' AND content = 'DATA' ORDER BY file_path").split() + assert paths, "The unreadable table was dropped from system.iceberg_files" + assert all(p.startswith("viewfs://cluster/") for p in paths), paths + + assert "Unsupported storage scheme" in instance.query_and_get_error(f"SELECT count() FROM {TABLE_NAME}") + + instance.query(f"DETACH TABLE {TABLE_NAME}") + + +def test_cluster_function_reads_external_delete_file(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_cluster_external_delete_{get_uuid_str()}" + base_path, _, _ = _external_delete_files_in_another_bucket(started_cluster_iceberg_with_spark, TABLE_NAME) + + args = get_query_args("s3", started_cluster_iceberg_with_spark, base_path) + assert instance.query(f"SELECT * FROM icebergS3Cluster('cluster_simple', {args}) ORDER BY id") == "1\talpha\n3\tgamma\n" + + +@pytest.mark.parametrize("metadata_storage,manifest_list_storage,manifest_storage,data_storage", [ + ("s3", "local", "s3", "s3"), + ("s3", "s3", "local", "s3"), + ("azure", "azure", "azure", "local"), +]) +def test_multi_storage_combinations(started_cluster_iceberg_with_spark, metadata_storage, manifest_list_storage, manifest_storage, data_storage): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_combo_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = _distribute_table_components(started_cluster_iceberg_with_spark, TABLE_NAME, metadata_storage, + manifest_list_storage, manifest_storage, data_storage) + + func = {"s3": "icebergS3", "azure": "icebergAzure", "local": "icebergLocal"}[metadata_storage.split(":")[0]] + args = get_query_args(metadata_storage, started_cluster_iceberg_with_spark, base_path) + + assert instance.query(f"SELECT * FROM {func}({args}) ORDER BY id") == ALL_ROWS diff --git a/tests/integration/test_storage_iceberg_with_spark/test_external_paths_local_files.py b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_local_files.py new file mode 100644 index 0000000000000..a41142590f357 --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_local_files.py @@ -0,0 +1,82 @@ +import pytest + +from helpers.iceberg_utils import additional_upload_directory, default_upload_directory, get_uuid_str +from .external_paths_utils import ( + ALL_ROWS, + create_and_upload_table, + _check_cluster_function_rejects_table, + _create_iceberg_s3_table, + _relocate_local_data_files_outside_table_directory, + _rewrite_paths_to_local_uri, +) + + +def test_cluster_function_rejects_external_local_file(started_cluster_iceberg_with_spark): + TABLE_NAME = f"test_cluster_external_local_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + base_path = _rewrite_paths_to_local_uri(started_cluster_iceberg_with_spark, TABLE_NAME, "localhost") + + _check_cluster_function_rejects_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path, ALL_ROWS) + + +def test_cluster_function_rejects_external_local_delete_file(started_cluster_iceberg_with_spark): + spark = started_cluster_iceberg_with_spark.spark_session + + TABLE_NAME = f"test_cluster_external_local_delete_{get_uuid_str()}" + + spark.sql( + f"CREATE TABLE {TABLE_NAME} (id INT, value STRING) USING iceberg " + f"TBLPROPERTIES ('format-version'='2', 'write.delete.mode'='merge-on-read')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (1, 'alpha'), (2, 'beta'), (3, 'gamma')") + spark.sql(f"DELETE FROM {TABLE_NAME} WHERE id = 2") + + default_upload_directory(started_cluster_iceberg_with_spark, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + base_path = _rewrite_paths_to_local_uri(started_cluster_iceberg_with_spark, TABLE_NAME, "localhost", deletes_only=True) + + _check_cluster_function_rejects_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path, "1\talpha\n3\tgamma\n") + + +@pytest.mark.parametrize("scheme", ["", "file://"]) +def test_local_table_with_external_data_files(started_cluster_iceberg_with_spark, scheme): + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + + TABLE_NAME = f"test_cluster_bare_external_local_{get_uuid_str()}" + external_dir = f"/var/lib/clickhouse/user_files/iceberg_external_{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')") + + host_path = _relocate_local_data_files_outside_table_directory(started_cluster_iceberg_with_spark, TABLE_NAME, external_dir, scheme) + for node in ("node2", "node3"): + additional_upload_directory( + started_cluster_iceberg_with_spark, node, "local", f"{host_path}/metadata", f"{host_path}/metadata") + + error = instance.query_and_get_error( + f"SELECT * FROM icebergLocalCluster('cluster_simple', local, path = '{host_path}', format=Parquet) ORDER BY id") + assert "cannot be read by a cluster function" in error + + instance.query(f"DROP TABLE IF EXISTS {TABLE_NAME}") + instance.query(f"CREATE TABLE {TABLE_NAME} ENGINE=IcebergLocal(local, path = '{host_path}', format=Parquet)") + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + +def test_trivial_count_rejects_path_outside_user_files(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_count_outside_user_files_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + # Keep the local files present so failure proves path rejection, not a missing file. + outside_dir = f"/var/lib/clickhouse/iceberg_outside_user_files_{get_uuid_str()}" + base_path = _rewrite_paths_to_local_uri(started_cluster_iceberg_with_spark, TABLE_NAME, "", target_dir=outside_dir) + expected_error = "outside of allowed `user_files` path" + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + assert expected_error in instance.query_and_get_error(f"SELECT count() FROM {TABLE_NAME}") + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") diff --git a/tests/integration/test_storage_iceberg_with_spark/test_external_paths_maintenance.py b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_maintenance.py new file mode 100644 index 0000000000000..cc12d54e7662a --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_maintenance.py @@ -0,0 +1,234 @@ +import json +import os +import shutil + +import pytest + +from helpers.s3_tools import S3Uploader +from helpers.iceberg_utils import default_upload_directory, get_uuid_str +from .external_paths_utils import ( + create_and_upload_table, + _download_table_for_relocation, + find_files, + ALL_ROWS, + REMOVE_ORPHAN_FILES, + REMOVE_ORPHAN_FILES_SETTINGS, + _create_iceberg_s3_table, + _external_data_files_in_another_bucket, + _external_data_files_in_the_same_bucket, + _external_delete_files_in_another_bucket, + _external_manifest_lists_in_another_bucket, + external_bucket, + relocate_manifest_lists_to_bucket, +) + + +def test_optimize_manifest_with_external_manifest_list(started_cluster_iceberg_with_spark): + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + + TABLE_NAME = f"test_optimize_external_ml_{get_uuid_str()}" + base_bucket = started_cluster_iceberg_with_spark.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')") + 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_iceberg_with_spark, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + base_path = relocate_manifest_lists_to_bucket(started_cluster_iceberg_with_spark, TABLE_NAME, external_bucket(started_cluster_iceberg_with_spark)) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + + def count_metadata_files(): + return sum( + 1 for obj in started_cluster_iceberg_with_spark.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, + }, + ) + + assert count_metadata_files() > metadata_files_before + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + instance.query(f"DROP TABLE {TABLE_NAME}") + + +@pytest.mark.parametrize( + "make_external,command,settings,rows", + [ + pytest.param( + _external_delete_files_in_another_bucket, + "OPTIMIZE TABLE {table}", + {"allow_experimental_iceberg_compaction": 1}, + "1\talpha\n3\tgamma\n", + id="optimize", + ), + pytest.param( + _external_manifest_lists_in_another_bucket, + "ALTER TABLE {table} EXECUTE expire_snapshots(" + "expire_before = '2099-12-31 23:59:59', retain_last = 1, retention_period = '1ms')", + {"allow_insert_into_iceberg": 1, "allow_experimental_expire_snapshots": 1}, + "1\talpha\n2\tbeta\n", + id="expire_snapshots", + ), + pytest.param( + _external_data_files_in_another_bucket, + REMOVE_ORPHAN_FILES, + REMOVE_ORPHAN_FILES_SETTINGS, + ALL_ROWS, + id="remove_orphan_files", + ), + pytest.param( + _external_data_files_in_the_same_bucket, + REMOVE_ORPHAN_FILES, + REMOVE_ORPHAN_FILES_SETTINGS, + ALL_ROWS, + id="remove_orphan_files_same_bucket", + ), + ], +) + + +def test_maintenance_commands_reject_external_paths(started_cluster_iceberg_with_spark, make_external, command, settings, rows): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_rejects_external_{get_uuid_str()}" + base_path, external_bucket, external_prefix = make_external(started_cluster_iceberg_with_spark, TABLE_NAME) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + def external_objects(): + return sorted( + obj.object_name for obj in + started_cluster_iceberg_with_spark.minio_client.list_objects(external_bucket, prefix=external_prefix, recursive=True)) + + objects_before = external_objects() + assert objects_before, "The files should have been relocated outside the table directory" + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == rows + + error = instance.query_and_get_error(command.format(table=TABLE_NAME), settings=settings) + assert "outside the table's base directory" in error + + assert external_objects() == objects_before + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == rows + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + +@pytest.mark.parametrize( + "write_query,rows_after", + [ + pytest.param("INSERT INTO {table} VALUES (4, 'delta')", ALL_ROWS + "4\tdelta\n", id="insert"), + pytest.param("ALTER TABLE {table} DELETE WHERE id = 2", "1\talpha\n3\tgamma\n", id="mutation"), + ], +) + + +def test_write_with_external_manifest_list(started_cluster_iceberg_with_spark, write_query, rows_after): + instance = started_cluster_iceberg_with_spark.instances["node1"] + + TABLE_NAME = f"test_write_external_ml_{get_uuid_str()}" + + create_and_upload_table(started_cluster_iceberg_with_spark, TABLE_NAME) + + base_path = relocate_manifest_lists_to_bucket(started_cluster_iceberg_with_spark, TABLE_NAME, external_bucket(started_cluster_iceberg_with_spark)) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + + instance.query(write_query.format(table=TABLE_NAME), settings={"allow_insert_into_iceberg": 1}) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == rows_after + instance.query(f"DROP TABLE {TABLE_NAME}") + + +@pytest.mark.parametrize("external_history_ref", ["manifest_list", "statistics"]) +def test_remove_orphan_files_ignores_deleted_external_history(started_cluster_iceberg_with_spark, external_history_ref): + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + + TABLE_NAME = f"test_orphan_history_{external_history_ref}_{get_uuid_str()}" + history_bucket = external_bucket(started_cluster_iceberg_with_spark) + + # Keep an expired snapshot reachable only through historical metadata. + 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')") + spark.sql(f"INSERT INTO {TABLE_NAME} VALUES (2, 'beta')") + + default_upload_directory(started_cluster_iceberg_with_spark, "s3", f"/iceberg_data/default/{TABLE_NAME}/", f"/iceberg_data/default/{TABLE_NAME}/") + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster_iceberg_with_spark, TABLE_NAME) + metadata_dir = os.path.join(host_path, "metadata") + current_metadata_file = sorted(find_files(metadata_dir, ".metadata.json"))[-1] + with open(current_metadata_file) as f: + current = json.load(f) + + external_key = f"{base_path}/metadata/history_external" + external_path = f"s3a://{history_bucket}/{external_key}" + + history = json.loads(json.dumps(current)) + assert len(history["snapshots"]) >= 2, history["snapshots"] + older_snapshot = history["snapshots"][0] + if external_history_ref == "manifest_list": + external_source = next( + f for f in find_files(metadata_dir, ".avro") + if os.path.basename(f).startswith("snap-") and str(older_snapshot["snapshot-id"]) in os.path.basename(f)) + older_snapshot["manifest-list"] = external_path + else: + # Statistics contents are never read during traversal. + external_source = current_metadata_file + history["statistics"] = [{ + "snapshot-id": older_snapshot["snapshot-id"], + "statistics-path": external_path, + "file-size-in-bytes": 1, + "file-footer-size-in-bytes": 1, + "blob-metadata": [], + }] + + # Use a lower version in the existing naming scheme so this history cannot become the table head. + assert current.get("metadata-log"), "expected a metadata-log entry to copy the path spelling from" + log_entry = dict(current["metadata-log"][-1]) + history_name = "v0.metadata.json" + history_key = f"{base_path}/metadata/{history_name}" + log_entry["metadata-file"] = log_entry["metadata-file"].rsplit("/", 1)[0] + "/" + history_name + current["metadata-log"] = [log_entry] + current["snapshots"] = [s for s in current["snapshots"] if s["snapshot-id"] == current["current-snapshot-id"]] + + history_file = os.path.join(host_path, history_name) + with open(history_file, "w") as f: + json.dump(history, f, indent=2) + with open(current_metadata_file, "w") as f: + json.dump(current, f, indent=2) + + uploader = started_cluster_iceberg_with_spark.default_s3_uploader + uploader.upload_file(history_file, history_key) + uploader.upload_file(current_metadata_file, f"{base_path}/metadata/{os.path.basename(current_metadata_file)}") + S3Uploader(started_cluster_iceberg_with_spark.minio_client, history_bucket).upload_file(external_source, external_key) + + shutil.rmtree(temp_dir) + + _create_iceberg_s3_table(started_cluster_iceberg_with_spark, TABLE_NAME, base_path) + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n" + + error = instance.query_and_get_error( + REMOVE_ORPHAN_FILES.format(table=TABLE_NAME), settings=REMOVE_ORPHAN_FILES_SETTINGS) + assert "outside the table's base directory" in error + + started_cluster_iceberg_with_spark.minio_client.remove_object(history_bucket, external_key) + + instance.query(REMOVE_ORPHAN_FILES.format(table=TABLE_NAME), settings=REMOVE_ORPHAN_FILES_SETTINGS) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == "1\talpha\n2\tbeta\n" + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") diff --git a/tests/integration/test_storage_iceberg_with_spark/test_external_paths_restricted.py b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_restricted.py new file mode 100644 index 0000000000000..9135a612031c6 --- /dev/null +++ b/tests/integration/test_storage_iceberg_with_spark/test_external_paths_restricted.py @@ -0,0 +1,203 @@ +import os +import shutil + +import pytest + +from helpers.cluster import ClickHouseCluster +from helpers.s3_tools import LocalUploader, S3Downloader, S3Uploader, prepare_s3_bucket +from helpers.spark_tools import ResilientSparkSession +from helpers.iceberg_utils import get_uuid_str +from .conftest import get_spark +from .external_paths_utils import ( + create_and_upload_table, + ALL_ROWS, + _create_iceberg_s3_table, + _download_table_for_relocation, + _external_data_files_in_another_bucket, + _external_data_files_in_the_same_bucket, + _rewrite_manifests_and_reupload, + _rewrite_paths_to_local_uri, + find_files, + relocate_data_files_to_bucket, +) + + +# `iceberg_delete_data_on_drop` and `remote_url_allow_hosts` require a separate cluster. +@pytest.fixture(scope="module") +def started_cluster(): + cluster = ClickHouseCluster(__file__, with_spark=True) + try: + cluster.add_instance( + "node1", + main_configs=[ + "configs/config.d/named_collections.xml", + "configs/config.d/remote_host_filter.xml", + ], + user_configs=[ + "configs/users.d/users.xml", + "configs/users.d/iceberg_delete_data_on_drop.xml", + ], + with_minio=True, + stay_alive=True, + ) + + cluster.start() + + prepare_s3_bucket(cluster) + + cluster.spark_session = ResilientSparkSession(lambda: get_spark(cluster.instances_dir)) + cluster.default_s3_uploader = S3Uploader(cluster.minio_client, cluster.minio_bucket) + cluster.default_s3_downloader = S3Downloader(cluster.minio_client, cluster.minio_bucket) + cluster.default_local_uploader = LocalUploader(cluster.instances["node1"]) + + yield cluster + + finally: + cluster.shutdown() + + +@pytest.mark.parametrize( + "make_external", + [ + pytest.param(_external_data_files_in_another_bucket, id="another_bucket"), + pytest.param(_external_data_files_in_the_same_bucket, id="same_bucket"), + ], +) + + +def test_delete_data_on_drop_removes_external_files(started_cluster, make_external): + instance = started_cluster.instances["node1"] + + TABLE_NAME = f"test_drop_external_{get_uuid_str()}" + base_path, external_bucket, external_prefix = make_external(started_cluster, TABLE_NAME) + + def count_objects(bucket, prefix): + return sum(1 for _ in started_cluster.minio_client.list_objects(bucket, prefix=prefix, recursive=True)) + + _create_iceberg_s3_table(started_cluster, TABLE_NAME, base_path) + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + + assert count_objects(external_bucket, external_prefix) > 0 + assert count_objects(started_cluster.minio_bucket, f"{base_path}/") > 0 + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + assert count_objects(started_cluster.minio_bucket, f"{base_path}/") == 0 + assert count_objects(external_bucket, external_prefix) == 0 + + +def test_delete_data_on_drop_uses_configured_metadata_head(started_cluster): + instance = started_cluster.instances["node1"] + + TABLE_NAME = f"test_drop_stale_head_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + external_prefix = f"external_data/{TABLE_NAME}" + + create_and_upload_table(started_cluster, TABLE_NAME) + base_path = relocate_data_files_to_bucket(started_cluster, TABLE_NAME, base_bucket, prefix=external_prefix) + + temp_dir, host_path, _ = _download_table_for_relocation(started_cluster, TABLE_NAME) + metadata_files = sorted(find_files(os.path.join(host_path, "metadata"), ".metadata.json")) + head = os.path.basename(metadata_files[-1]) + # An uncommitted higher version must not hide the configured head during deletion. + started_cluster.default_s3_uploader.upload_file( + metadata_files[0], f"{base_path}/metadata/99999-{get_uuid_str()}.metadata.json") + shutil.rmtree(temp_dir) + + def count_objects(prefix): + return sum(1 for _ in started_cluster.minio_client.list_objects(base_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}) " + f"SETTINGS iceberg_metadata_file_path = 'metadata/{head}'") + + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + assert count_objects(f"{external_prefix}/") > 0 + + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + assert count_objects(f"{base_path}/") == 0 + assert count_objects(f"{external_prefix}/") == 0 + + +def test_delete_data_on_drop_requires_write_grant_on_external_files(started_cluster): + instance = started_cluster.instances["node1"] + + TABLE_NAME = f"test_drop_external_grant_{get_uuid_str()}" + user = f"user_{TABLE_NAME}" + base_path, bucket, external_prefix = _external_data_files_in_the_same_bucket(started_cluster, TABLE_NAME) + external_uri_regex = f"s3a://{bucket}/{external_prefix}.*" + + def count_objects(prefix): + return sum(1 for _ in started_cluster.minio_client.list_objects(bucket, prefix=prefix, recursive=True)) + + _create_iceberg_s3_table(started_cluster, TABLE_NAME, base_path) + + instance.query(f"DROP USER IF EXISTS {user}") + instance.query(f"CREATE USER {user}") + instance.query(f"GRANT DROP TABLE ON {TABLE_NAME} TO {user}") + instance.query(f"GRANT READ ON S3('{external_uri_regex}') TO {user}") + + error = instance.query_and_get_error(f"DROP TABLE {TABLE_NAME} SYNC", user=user) + assert "ACCESS_DENIED" in error, error + assert instance.query(f"SELECT * FROM {TABLE_NAME} ORDER BY id") == ALL_ROWS + + instance.query(f"REVOKE READ ON S3('{external_uri_regex}') FROM {user}") + instance.query(f"GRANT WRITE ON S3('{external_uri_regex}') TO {user}") + instance.query(f"DROP TABLE {TABLE_NAME} SYNC", user=user) + + assert count_objects(f"{base_path}/") == 0 + assert count_objects(external_prefix) == 0 + + instance.query(f"DROP USER {user}") + + +def test_file_uri_with_remote_authority_is_rejected(started_cluster): + instance = started_cluster.instances["node1"] + + TABLE_NAME = f"test_file_remote_authority_{get_uuid_str()}" + + create_and_upload_table(started_cluster, TABLE_NAME) + # Keep the local files present so failure proves path rejection, not a missing file. + base_path = _rewrite_paths_to_local_uri(started_cluster, TABLE_NAME, "other-host") + + _create_iceberg_s3_table(started_cluster, TABLE_NAME, base_path) + + error = instance.query_and_get_error(f"SELECT * FROM {TABLE_NAME} ORDER BY id") + assert "refers to host 'other-host'" in error + assert "refers to host 'other-host'" in instance.query_and_get_error(f"SELECT count() FROM {TABLE_NAME}") + + paths = instance.query( + f"SELECT file_path FROM system.iceberg_files " + f"WHERE database = 'default' AND table = '{TABLE_NAME}' AND content = 'DATA' ORDER BY file_path").split() + assert paths, "The unreadable table was dropped from system.iceberg_files" + assert all(p.startswith("file://other-host/") for p in paths), paths + + # Permanent path rejection must not make `DatabaseCatalog` retry this drop forever. + instance.query(f"DROP TABLE {TABLE_NAME} SYNC") + + +def test_external_endpoint_is_rejected_by_remote_host_filter(started_cluster): + instance = started_cluster.instances["node1"] + + TABLE_NAME = f"test_host_filter_{get_uuid_str()}" + base_bucket = started_cluster.minio_bucket + + create_and_upload_table(started_cluster, TABLE_NAME) + + temp_dir, host_path, base_path = _download_table_for_relocation(started_cluster, TABLE_NAME) + _rewrite_manifests_and_reupload( + started_cluster, host_path, base_path, + lambda p: f"http://not-allowed-host:{started_cluster.minio_port}/{base_bucket}/{base_path}/data/{os.path.basename(p)}") + 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}/'" + + error = instance.query_and_get_error(f"SELECT * FROM icebergS3({args})") + assert "not allowed in configuration file" in error, error + assert "not-allowed-host" in error, error diff --git a/tests/integration/test_storage_iceberg_with_spark/test_optimize.py b/tests/integration/test_storage_iceberg_with_spark/test_optimize.py index b6f2fee5389fd..d72c8184e4a69 100644 --- a/tests/integration/test_storage_iceberg_with_spark/test_optimize.py +++ b/tests/integration/test_storage_iceberg_with_spark/test_optimize.py @@ -185,4 +185,60 @@ def test_optimize_manifest_per_file_stats(started_cluster_iceberg_with_spark): assert int(record_count) == actual_rows data_entries_checked += 1 - assert data_entries_checked > 0 \ No newline at end of file + assert data_entries_checked > 0 + + +def test_optimize_keeps_data_when_it_cannot_remove_metadata(started_cluster_iceberg_with_spark): + storage_type = "local" + instance = started_cluster_iceberg_with_spark.instances["node1"] + spark = started_cluster_iceberg_with_spark.spark_session + TABLE_NAME = "test_optimize_failed_cleanup_" + storage_type + "_" + get_uuid_str() + + spark.sql( + f""" + CREATE TABLE {TABLE_NAME} (id long, data string) USING iceberg + TBLPROPERTIES ('format-version' = '2', 'write.delete.mode'='merge-on-read') + """ + ) + spark.sql(f"INSERT INTO {TABLE_NAME} select id, char(id + ascii('a')) from range(10, 100)") + default_upload_directory( + started_cluster_iceberg_with_spark, + storage_type, + f"/iceberg_data/default/{TABLE_NAME}/", + f"/iceberg_data/default/{TABLE_NAME}/", + ) + spark.sql(f"DELETE FROM {TABLE_NAME} WHERE id < 20") + default_upload_directory( + started_cluster_iceberg_with_spark, + storage_type, + f"/iceberg_data/default/{TABLE_NAME}/", + f"/iceberg_data/default/{TABLE_NAME}/", + ) + + create_iceberg_table(storage_type, instance, TABLE_NAME, started_cluster_iceberg_with_spark) + assert int(instance.query(f"SELECT count() FROM {TABLE_NAME}")) == 80 + + table_dir = f"/var/lib/clickhouse/user_files/iceberg_data/default/{TABLE_NAME}" + + def list_dir(subdir): + return set(instance.exec_in_container(["bash", "-c", f"ls {table_dir}/{subdir}"]).split()) + + metadata_before = list_dir("metadata") + data_before = list_dir("data") + + # The failpoint throws after removal; the head must be removed last to remain readable. + instance.query("SYSTEM ENABLE FAILPOINT local_object_storage_network_error_during_remove") + try: + error = instance.query_and_get_error( + f"OPTIMIZE TABLE {TABLE_NAME};", settings={"allow_experimental_iceberg_compaction": 1} + ) + finally: + instance.query("SYSTEM DISABLE FAILPOINT local_object_storage_network_error_during_remove") + + assert "the metadata files the compaction replaced" in error, error + + assert list_dir("metadata") & metadata_before, \ + "The whole old `metadata` prefix was removed even though one of its removals failed" + assert data_before <= list_dir("data"), \ + f"Data files of the still-current snapshot were deleted: {sorted(data_before - list_dir('data'))}" + assert int(instance.query(f"SELECT count() FROM {TABLE_NAME}")) == 80 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 97c4555c6858d..0c4b00e0ab40e 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,6 @@ 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. WORK_DIR="${CLICKHOUSE_TMP}/iceberg_alter_drop_column_104295_${CLICKHOUSE_TEST_UNIQUE_NAME}" rm -rf "${WORK_DIR}" mkdir -p "${WORK_DIR}" @@ -49,4 +47,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/05182_iceberg_same_bucket_location_mismatch.reference b/tests/queries/0_stateless/05182_iceberg_same_bucket_location_mismatch.reference new file mode 100644 index 0000000000000..d81cc0710eb6c --- /dev/null +++ b/tests/queries/0_stateless/05182_iceberg_same_bucket_location_mismatch.reference @@ -0,0 +1 @@ +42 diff --git a/tests/queries/0_stateless/05182_iceberg_same_bucket_location_mismatch.sh b/tests/queries/0_stateless/05182_iceberg_same_bucket_location_mismatch.sh new file mode 100755 index 0000000000000..a451ae23d8c37 --- /dev/null +++ b/tests/queries/0_stateless/05182_iceberg_same_bucket_location_mismatch.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Tags: no-fasttest + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +TABLE_PATH="05182_iceberg_same_bucket/${CLICKHOUSE_TEST_UNIQUE_NAME}" + +${CLICKHOUSE_CLIENT} --allow_experimental_insert_into_iceberg 1 -q " + DROP TABLE IF EXISTS t_iceberg_same_bucket; + CREATE TABLE t_iceberg_same_bucket (c0 Int) ENGINE = IcebergS3(s3_conn, filename = '${TABLE_PATH}'); + INSERT INTO t_iceberg_same_bucket VALUES (42); + DROP TABLE IF EXISTS t_iceberg_same_bucket; +" + +${CLICKHOUSE_CLIENT} --input_format_parallel_parsing 0 --output_format_parallel_formatting 0 -q " + SELECT * FROM s3(s3_conn, filename='${TABLE_PATH}/metadata/v2.metadata.json', structure='line String', format='LineAsString') +" | python3 -c " +import json, sys +m = json.load(sys.stdin) +old_location = m['location'].rstrip('/') +head, _, leaf = old_location.rpartition('/') +new_location = head + '/relocated_' + leaf +m['location'] = new_location +for s in m.get('snapshots', []): + ml = s['manifest-list'] + assert ml.startswith(old_location + '/') + s['manifest-list'] = new_location + ml[len(old_location):] +print(json.dumps(m)) +" | ${CLICKHOUSE_CLIENT} -q " + INSERT INTO FUNCTION s3(s3_conn, filename='${TABLE_PATH}/metadata/v2.metadata.json', structure='line String', format='LineAsString') + SETTINGS s3_truncate_on_insert=1 + SELECT * FROM input('line String') FORMAT LineAsString +" + +${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache 0 -q " + SELECT * FROM icebergS3(s3_conn, filename='${TABLE_PATH}'); +" diff --git a/tests/queries/0_stateless/05183_iceberg_colon_in_path.reference b/tests/queries/0_stateless/05183_iceberg_colon_in_path.reference new file mode 100644 index 0000000000000..d81cc0710eb6c --- /dev/null +++ b/tests/queries/0_stateless/05183_iceberg_colon_in_path.reference @@ -0,0 +1 @@ +42 diff --git a/tests/queries/0_stateless/05183_iceberg_colon_in_path.sh b/tests/queries/0_stateless/05183_iceberg_colon_in_path.sh new file mode 100755 index 0000000000000..00d4b7e692fc4 --- /dev/null +++ b/tests/queries/0_stateless/05183_iceberg_colon_in_path.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Tags: no-fasttest + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +WORK_DIR="${CLICKHOUSE_TMP}/iceberg_colon_${CLICKHOUSE_TEST_UNIQUE_NAME}" +rm -rf "${WORK_DIR}" +TABLE_DIR="${WORK_DIR}/t0:" +mkdir -p "${TABLE_DIR}" +trap 'rm -rf "${WORK_DIR}"' EXIT + +${CLICKHOUSE_LOCAL} \ + --allow_insert_into_iceberg=1 \ + --multiquery -q " +CREATE TABLE t0 (c0 Int) ENGINE = IcebergLocal('${TABLE_DIR}/'); +INSERT INTO t0 VALUES (42); +SELECT c0 FROM t0; +"