diff --git a/example/demo_example.cc b/example/demo_example.cc index 3c8745be2..22ecc0c90 100644 --- a/example/demo_example.cc +++ b/example/demo_example.cc @@ -18,6 +18,7 @@ */ #include +#include #include "iceberg/arrow/arrow_io_util.h" #include "iceberg/avro/avro_register.h" @@ -42,8 +43,14 @@ int main(int argc, char** argv) { iceberg::avro::RegisterAll(); iceberg::parquet::RegisterAll(); - auto catalog = iceberg::InMemoryCatalog::Make("test", iceberg::arrow::MakeLocalFileIO(), - warehouse_location, properties); + auto catalog_result = iceberg::InMemoryCatalog::Make( + "test", iceberg::arrow::MakeLocalFileIO(), warehouse_location, properties); + if (!catalog_result.has_value()) { + std::cerr << "Failed to create catalog: " << catalog_result.error().message + << std::endl; + return 1; + } + auto catalog = std::move(catalog_result.value()); auto register_result = catalog->RegisterTable({.name = table_name}, table_location); if (!register_result.has_value()) { diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index f605e57ad..e48209d62 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -21,6 +21,7 @@ set(ICEBERG_SOURCES arrow_c_data_guard_internal.cc arrow_c_data_util.cc arrow_row_builder.cc + catalog/catalog_util.cc catalog/memory/in_memory_catalog.cc catalog/session_catalog.cc catalog/session_context.cc @@ -39,6 +40,7 @@ set(ICEBERG_SOURCES expression/projections.cc expression/residual_evaluator.cc expression/rewrite_not.cc + expression/sanitize_expression.cc expression/strict_metrics_evaluator.cc expression/term.cc file_io.cc diff --git a/src/iceberg/catalog/catalog_util.cc b/src/iceberg/catalog/catalog_util.cc new file mode 100644 index 000000000..d0dc5d4a0 --- /dev/null +++ b/src/iceberg/catalog/catalog_util.cc @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/catalog_util.h" + +#include "iceberg/table_identifier.h" + +namespace iceberg { + +std::string CatalogUtil::FullTableName(std::string_view catalog_name, + const TableIdentifier& identifier) { + if (catalog_name.empty()) { + return identifier.ToString(); + } + + std::string result; + if (catalog_name.contains('/') || catalog_name.contains(':')) { + result = catalog_name; + if (!catalog_name.ends_with('/')) { + result += '/'; + } + } else { + result = std::string(catalog_name) + '.'; + } + + for (const auto& level : identifier.ns.levels) { + result += level + '.'; + } + result += identifier.name; + return result; +} + +} // namespace iceberg diff --git a/src/iceberg/catalog/catalog_util.h b/src/iceberg/catalog/catalog_util.h new file mode 100644 index 000000000..f11f0b919 --- /dev/null +++ b/src/iceberg/catalog/catalog_util.h @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Utilities for working with catalogs. +class ICEBERG_EXPORT CatalogUtil { + public: + /// \brief Return the fully-qualified name for a table in a catalog. + static std::string FullTableName(std::string_view catalog_name, + const TableIdentifier& identifier); +}; + +} // namespace iceberg diff --git a/src/iceberg/catalog/memory/in_memory_catalog.cc b/src/iceberg/catalog/memory/in_memory_catalog.cc index 6148ef4e6..d0797e728 100644 --- a/src/iceberg/catalog/memory/in_memory_catalog.cc +++ b/src/iceberg/catalog/memory/in_memory_catalog.cc @@ -22,7 +22,9 @@ #include #include +#include "iceberg/catalog/catalog_util.h" #include "iceberg/file_io.h" +#include "iceberg/metrics/metrics_reporters.h" #include "iceberg/table.h" #include "iceberg/table_identifier.h" #include "iceberg/table_metadata.h" @@ -337,22 +339,30 @@ Status InMemoryNamespace::UpdateTableMetadataLocation( return {}; } -std::shared_ptr InMemoryCatalog::Make( +Result> InMemoryCatalog::Make( const std::string& name, const std::shared_ptr& file_io, const std::string& warehouse_location, const std::unordered_map& properties) { - return std::make_shared(name, file_io, warehouse_location, properties); + std::shared_ptr reporter; + auto it = properties.find(std::string(kMetricsReporterImpl)); + if (it != properties.end() && !it->second.empty() && + it->second != kMetricsReporterTypeNoop) { + ICEBERG_ASSIGN_OR_RAISE(reporter, MetricsReporters::Load(properties)); + } + return std::make_shared(name, file_io, warehouse_location, properties, + std::move(reporter)); } -InMemoryCatalog::InMemoryCatalog( - const std::string& name, const std::shared_ptr& file_io, - const std::string& warehouse_location, - const std::unordered_map& properties) +InMemoryCatalog::InMemoryCatalog(std::string name, std::shared_ptr file_io, + std::string warehouse_location, + std::unordered_map properties, + std::shared_ptr reporter) : catalog_name_(std::move(name)), properties_(std::move(properties)), file_io_(std::move(file_io)), warehouse_location_(std::move(warehouse_location)), - root_namespace_(std::make_unique()) {} + root_namespace_(std::make_unique()), + reporter_(std::move(reporter)) {} InMemoryCatalog::~InMemoryCatalog() = default; @@ -429,7 +439,8 @@ Result> InMemoryCatalog::CreateTable( ICEBERG_RETURN_UNEXPECTED( root_namespace_->UpdateTableMetadataLocation(identifier, metadata_file_location)); return Table::Make(identifier, std::move(table_metadata), - std::move(metadata_file_location), file_io_, shared_from_this()); + std::move(metadata_file_location), file_io_, shared_from_this(), + CatalogUtil::FullTableName(name(), identifier), reporter_); } Result> InMemoryCatalog::UpdateTable( @@ -480,7 +491,8 @@ Result> InMemoryCatalog::UpdateTable( TableMetadataUtil::DeleteRemovedMetadataFiles(*file_io_, base.get(), *updated); return Table::Make(identifier, std::move(updated), std::move(new_metadata_location), - file_io_, shared_from_this()); + file_io_, shared_from_this(), + CatalogUtil::FullTableName(name(), identifier), reporter_); } Result> InMemoryCatalog::StageCreateTable( @@ -500,8 +512,10 @@ Result> InMemoryCatalog::StageCreateTable( auto table_metadata, TableMetadata::Make(*schema, *spec, *order, base_location, properties)); ICEBERG_ASSIGN_OR_RAISE( - auto table, StagedTable::Make(identifier, std::move(table_metadata), "", file_io_, - shared_from_this())); + auto table, + StagedTable::Make(identifier, std::move(table_metadata), "", file_io_, + shared_from_this(), + CatalogUtil::FullTableName(name(), identifier), reporter_)); return Transaction::Make(std::move(table), TransactionKind::kCreate); } @@ -581,7 +595,8 @@ Result> InMemoryCatalog::LoadTable( ICEBERG_ASSIGN_OR_RAISE(auto metadata, TableMetadataUtil::Read(*file_io_, metadata_location)); return Table::Make(identifier, std::move(metadata), std::move(metadata_location), - file_io_, shared_from_this()); + file_io_, shared_from_this(), + CatalogUtil::FullTableName(name(), identifier), reporter_); } Result> InMemoryCatalog::RegisterTable( @@ -601,7 +616,8 @@ Result> InMemoryCatalog::RegisterTable( return UnknownError("The registry failed."); } return Table::Make(identifier, std::move(metadata), metadata_file_location, file_io_, - shared_from_this()); + shared_from_this(), CatalogUtil::FullTableName(name(), identifier), + reporter_); } } // namespace iceberg diff --git a/src/iceberg/catalog/memory/in_memory_catalog.h b/src/iceberg/catalog/memory/in_memory_catalog.h index 548fd7afc..ea874d68d 100644 --- a/src/iceberg/catalog/memory/in_memory_catalog.h +++ b/src/iceberg/catalog/memory/in_memory_catalog.h @@ -22,6 +22,7 @@ /// \file iceberg/catalog/memory/in_memory_catalog.h /// \brief Provide an in-memory catalog implementation. +#include #include #include "iceberg/catalog.h" @@ -42,12 +43,13 @@ class ICEBERG_EXPORT InMemoryCatalog : public Catalog, public std::enable_shared_from_this { public: - InMemoryCatalog(std::string const& name, std::shared_ptr const& file_io, - std::string const& warehouse_location, - std::unordered_map const& properties); + InMemoryCatalog(std::string name, std::shared_ptr file_io, + std::string warehouse_location, + std::unordered_map properties, + std::shared_ptr reporter = nullptr); ~InMemoryCatalog() override; - static std::shared_ptr Make( + static Result> Make( std::string const& name, std::shared_ptr const& file_io, std::string const& warehouse_location, std::unordered_map const& properties); @@ -109,6 +111,7 @@ class ICEBERG_EXPORT InMemoryCatalog std::string warehouse_location_; std::unique_ptr root_namespace_; mutable std::shared_mutex mutex_; + std::shared_ptr reporter_; }; } // namespace iceberg diff --git a/src/iceberg/catalog/meson.build b/src/iceberg/catalog/meson.build index 1da2e533a..57a4cd59a 100644 --- a/src/iceberg/catalog/meson.build +++ b/src/iceberg/catalog/meson.build @@ -18,7 +18,7 @@ subdir('memory') install_headers( - ['session_catalog.h', 'session_context.h'], + ['catalog_util.h', 'session_catalog.h', 'session_context.h'], subdir: 'iceberg/catalog', ) diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index b6438486a..f64860ff4 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -33,6 +33,7 @@ set(ICEBERG_REST_SOURCES resource_paths.cc rest_catalog.cc rest_file_io.cc + rest_metrics_reporter.cc rest_util.cc types.cc) diff --git a/src/iceberg/catalog/rest/catalog_properties.h b/src/iceberg/catalog/rest/catalog_properties.h index 0515926c7..d1ee0e9c4 100644 --- a/src/iceberg/catalog/rest/catalog_properties.h +++ b/src/iceberg/catalog/rest/catalog_properties.h @@ -55,6 +55,9 @@ class ICEBERG_REST_EXPORT RestCatalogProperties inline static Entry kNamespaceSeparator{"namespace-separator", "%1F"}; /// \brief The snapshot loading mode (ALL or REFS). inline static Entry kSnapshotLoadingMode{"snapshot-loading-mode", "ALL"}; + /// \brief Whether to report metrics to the REST catalog server (default: true). + inline static Entry kMetricsReportingEnabled{ + "rest-metrics-reporting-enabled", "true"}; /// \brief The prefix for HTTP headers. inline static constexpr std::string_view kHeaderPrefix = "header."; diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 48254614f..65a67ffb8 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -30,6 +30,7 @@ iceberg_rest_sources = files( 'resource_paths.cc', 'rest_catalog.cc', 'rest_file_io.cc', + 'rest_metrics_reporter.cc', 'rest_util.cc', 'types.cc', ) diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 27de0befa..349071f42 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -38,9 +38,11 @@ #include "iceberg/catalog/rest/json_serde_internal.h" #include "iceberg/catalog/rest/resource_paths.h" #include "iceberg/catalog/rest/rest_file_io.h" +#include "iceberg/catalog/rest/rest_metrics_reporter_internal.h" #include "iceberg/catalog/rest/rest_util.h" #include "iceberg/catalog/rest/types.h" #include "iceberg/json_serde_internal.h" +#include "iceberg/metrics/metrics_reporters.h" #include "iceberg/partition_spec.h" #include "iceberg/result.h" #include "iceberg/schema.h" @@ -52,6 +54,7 @@ #include "iceberg/transaction.h" #include "iceberg/util/formatter_internal.h" #include "iceberg/util/macros.h" +#include "iceberg/util/string_util.h" namespace iceberg::rest { @@ -71,6 +74,14 @@ std::unordered_set GetDefaultEndpoints() { }; } +std::string RestTableName(std::string_view catalog_name, + const TableIdentifier& identifier) { + if (catalog_name.empty()) { + return identifier.ToString(); + } + return std::string(catalog_name) + '.' + identifier.ToString(); +} + /// \brief Fetch server configuration from the REST catalog server. Result FetchServerConfig(const ResourcePaths& paths, const RestCatalogProperties& current_config, @@ -375,7 +386,7 @@ RestCatalog::~RestCatalog() { } Result> RestCatalog::Make( - const RestCatalogProperties& config) { + const RestCatalogProperties& config, Executor* metrics_executor) { ICEBERG_ASSIGN_OR_RAISE(auto uri, config.Uri()); std::string catalog_name = config.Get(RestCatalogProperties::kName); @@ -419,27 +430,37 @@ Result> RestCatalog::Make( // Get snapshot loading mode ICEBERG_ASSIGN_OR_RAISE(auto snapshot_mode, final_config.SnapshotLoadingMode()); - auto client = std::make_unique(final_config.ExtractHeaders()); + auto client = std::make_shared(final_config.ExtractHeaders()); ICEBERG_ASSIGN_OR_RAISE(auto catalog_session, auth_manager->CatalogSession(*client, final_config.configs())); // Create FileIO with the final configuration ICEBERG_ASSIGN_OR_RAISE(auto file_io, MakeCatalogFileIO(final_config)); + std::shared_ptr reporter; + const auto& props = final_config.configs(); + if (auto it = props.find(std::string(kMetricsReporterImpl)); + it != props.end() && !it->second.empty() && + it->second != kMetricsReporterTypeNoop) { + ICEBERG_ASSIGN_OR_RAISE(reporter, MetricsReporters::Load(props)); + } + auto default_context = SessionContext::Empty(); return std::shared_ptr(new RestCatalog( std::move(final_config), std::move(file_io), std::move(client), std::move(paths), std::move(endpoints), std::move(auth_manager), std::move(catalog_session), - snapshot_mode, std::move(default_context))); + snapshot_mode, std::move(default_context), std::move(reporter), metrics_executor)); } RestCatalog::RestCatalog(RestCatalogProperties config, std::shared_ptr file_io, - std::unique_ptr client, + std::shared_ptr client, std::unique_ptr paths, std::unordered_set endpoints, std::unique_ptr auth_manager, std::shared_ptr catalog_session, - SnapshotMode snapshot_mode, SessionContext default_context) + SnapshotMode snapshot_mode, SessionContext default_context, + std::shared_ptr reporter, + Executor* metrics_executor) : config_(std::move(config)), file_io_(std::move(file_io)), client_(std::move(client)), @@ -449,7 +470,9 @@ RestCatalog::RestCatalog(RestCatalogProperties config, std::shared_ptr f auth_manager_(std::move(auth_manager)), catalog_session_(std::move(catalog_session)), snapshot_mode_(snapshot_mode), - default_context_(std::move(default_context)) { + default_context_(std::move(default_context)), + reporter_(std::move(reporter)), + metrics_executor_(metrics_executor) { ICEBERG_DCHECK(catalog_session_ != nullptr, "catalog_session must not be null"); } @@ -496,6 +519,25 @@ Result> RestCatalog::TableFileIO( return file_io_; } +Result> RestCatalog::MakeTableReporter( + const TableIdentifier& identifier, + const std::shared_ptr& table_session) const { + auto metrics_enabled = config_.Get(RestCatalogProperties::kMetricsReportingEnabled); + if (StringUtils::ToLower(metrics_enabled) == "true" && + supported_endpoints_.contains(Endpoint::ReportMetrics())) { + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Metrics(identifier)); + auto post = [client = client_](const std::string& endpoint, const std::string& body, + auth::AuthSession& session) { + std::ignore = client->Post(endpoint, body, /*headers=*/{}, + *DefaultErrorHandler::Instance(), session); + }; + auto rest_reporter = std::make_shared( + std::move(path), table_session, metrics_executor_, std::move(post)); + return MetricsReporters::Combine(reporter_, rest_reporter); + } + return reporter_; +} + Result> RestCatalog::ListNamespaces( const Namespace& ns, auth::AuthSession& session) const { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::ListNamespaces()); @@ -735,6 +777,7 @@ Result> RestCatalog::StageCreateTable( ICEBERG_ASSIGN_OR_RAISE( auto table_session, TableAuthSession(identifier, table_config, std::move(contextual_session))); + ICEBERG_ASSIGN_OR_RAISE(auto reporter, MakeTableReporter(identifier, table_session)); auto table_catalog = std::make_shared( shared_from_this(), context, identifier, table_config, std::move(table_session), table_io); @@ -742,7 +785,8 @@ Result> RestCatalog::StageCreateTable( auto staged_table, StagedTable::Make(identifier, std::move(result.metadata), std::move(result.metadata_location), std::move(table_io), - std::move(table_catalog))); + std::move(table_catalog), RestTableName(name_, identifier), + std::move(reporter))); return Transaction::Make(std::move(staged_table), TransactionKind::kCreate); } @@ -851,11 +895,14 @@ Result> RestCatalog::MakeTableFromLoadResult( ICEBERG_ASSIGN_OR_RAISE( auto table_session, TableAuthSession(identifier, table_config, std::move(contextual_session))); + ICEBERG_ASSIGN_OR_RAISE(auto reporter, MakeTableReporter(identifier, table_session)); auto table_catalog = std::make_shared( shared_from_this(), context, identifier, table_config, table_session, table_io); + return Table::Make(identifier, std::move(result.metadata), std::move(result.metadata_location), std::move(table_io), - std::move(table_catalog)); + std::move(table_catalog), RestTableName(name_, identifier), + std::move(reporter)); } Result> RestCatalog::MakeTableFromCommitResponse( @@ -863,12 +910,14 @@ Result> RestCatalog::MakeTableFromCommitResponse( const SessionContext& context, const std::unordered_map& table_config, std::shared_ptr table_session, std::shared_ptr table_io) { + ICEBERG_ASSIGN_OR_RAISE(auto reporter, MakeTableReporter(identifier, table_session)); // Reuse the bound FileIO because commit responses carry no config or credentials. auto table_catalog = std::make_shared( shared_from_this(), context, identifier, table_config, table_session, table_io); return Table::Make(identifier, std::move(response.metadata), std::move(response.metadata_location), std::move(table_io), - std::move(table_catalog)); + std::move(table_catalog), RestTableName(name_, identifier), + std::move(reporter)); } } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 97cf42151..65b0b5eab 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -32,6 +32,7 @@ #include "iceberg/catalog/session_context.h" #include "iceberg/result.h" #include "iceberg/storage_credential.h" +#include "iceberg/type_fwd.h" /// \file iceberg/catalog/rest/rest_catalog.h /// RestCatalog implementation for Iceberg REST API. @@ -51,7 +52,11 @@ class ICEBERG_REST_EXPORT RestCatalog final RestCatalog& operator=(RestCatalog&&) = delete; /// \brief Create a RestCatalog instance. - static Result> Make(const RestCatalogProperties& config); + /// \param metrics_executor Optional non-owning executor for asynchronous REST metrics. + /// It must outlive the catalog and all tables created by it. When null, REST metrics + /// are reported synchronously. + static Result> Make(const RestCatalogProperties& config, + Executor* metrics_executor = nullptr); std::string_view name() const override; @@ -64,11 +69,12 @@ class ICEBERG_REST_EXPORT RestCatalog final class TableScopedCatalog; RestCatalog(RestCatalogProperties config, std::shared_ptr file_io, - std::unique_ptr client, std::unique_ptr paths, + std::shared_ptr client, std::unique_ptr paths, std::unordered_set endpoints, std::unique_ptr auth_manager, std::shared_ptr catalog_session, - SnapshotMode snapshot_mode, SessionContext default_context); + SnapshotMode snapshot_mode, SessionContext default_context, + std::shared_ptr reporter, Executor* metrics_executor); Result> ContextualAuthSession( const SessionContext& context); @@ -149,6 +155,17 @@ class ICEBERG_REST_EXPORT RestCatalog final Result LoadTableInternal(const TableIdentifier& identifier, auth::AuthSession& session) const; + /// \brief Build the per-table metrics reporter. + /// + /// When rest-metrics-reporting-enabled is true and the server advertises the + /// ReportMetrics endpoint, returns a CompositeMetricsReporter combining configured + /// reporter with a RestMetricsReporter targeting this table, authenticated with the + /// table-scoped session so metrics POSTs use the same credentials as table + /// operations. Otherwise returns the configured reporter. + Result> MakeTableReporter( + const TableIdentifier& identifier, + const std::shared_ptr& table_session) const; + Result CreateTableInternal( const TableIdentifier& identifier, const std::shared_ptr& schema, const std::shared_ptr& spec, const std::shared_ptr& order, @@ -175,7 +192,7 @@ class ICEBERG_REST_EXPORT RestCatalog final RestCatalogProperties config_; std::shared_ptr file_io_; - std::unique_ptr client_; + std::shared_ptr client_; std::unique_ptr paths_; std::string name_; std::unordered_set supported_endpoints_; @@ -184,6 +201,8 @@ class ICEBERG_REST_EXPORT RestCatalog final SnapshotMode snapshot_mode_; SessionContext default_context_; std::weak_ptr default_catalog_; + std::shared_ptr reporter_; + Executor* metrics_executor_ = nullptr; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_metrics_reporter.cc b/src/iceberg/catalog/rest/rest_metrics_reporter.cc new file mode 100644 index 000000000..0873b9378 --- /dev/null +++ b/src/iceberg/catalog/rest/rest_metrics_reporter.cc @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include + +#include + +#include "iceberg/catalog/rest/auth/auth_session.h" +#include "iceberg/catalog/rest/rest_metrics_reporter_internal.h" +#include "iceberg/metrics/json_serde_internal.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/util/executor.h" + +namespace iceberg::rest { + +namespace { + +constexpr std::string_view kReportType = "report-type"; +constexpr std::string_view kScanReportType = "scan-report"; +constexpr std::string_view kCommitReportType = "commit-report"; + +} // namespace + +RestMetricsReporter::RestMetricsReporter(std::string metrics_endpoint, + std::shared_ptr session, + Executor* executor, RestMetricsPost post) + : metrics_endpoint_(std::move(metrics_endpoint)), + session_(std::move(session)), + executor_(executor), + post_(std::move(post)) {} + +Result RestMetricsReporter::BuildRequestBody(const MetricsReport& report) { + ICEBERG_ASSIGN_OR_RAISE( + auto json, + std::visit([](const auto& r) -> Result { return ToJson(r); }, + report)); + + json[kReportType] = + std::holds_alternative(report) ? kScanReportType : kCommitReportType; + return json.dump(); +} + +Status RestMetricsReporter::Report(const MetricsReport& report) { + try { + auto body_result = BuildRequestBody(report); + if (!body_result || !session_ || !post_) { + return {}; + } + + ExecutorTask task([post = post_, endpoint = metrics_endpoint_, + body = std::move(*body_result), session = session_]() mutable { + try { + post(endpoint, body, *session); + } catch (...) { + } + }); + if (executor_) { + std::ignore = executor_->Submit(std::move(task)); + } else { + std::move(task)(); + } + } catch (...) { + } + return {}; +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_metrics_reporter_internal.h b/src/iceberg/catalog/rest/rest_metrics_reporter_internal.h new file mode 100644 index 000000000..bb26c538d --- /dev/null +++ b/src/iceberg/catalog/rest/rest_metrics_reporter_internal.h @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/type_fwd.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" + +namespace iceberg::rest { + +using RestMetricsPost = + std::function; + +class ICEBERG_REST_EXPORT RestMetricsReporter final : public MetricsReporter { + public: + RestMetricsReporter(std::string metrics_endpoint, + std::shared_ptr session, Executor* executor, + RestMetricsPost post); + + Status Report(const MetricsReport& report) override; + + private: + static Result BuildRequestBody(const MetricsReport& report); + + std::string metrics_endpoint_; + std::shared_ptr session_; + Executor* executor_; + RestMetricsPost post_; +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/sql/sql_catalog.cc b/src/iceberg/catalog/sql/sql_catalog.cc index cfe155f76..e69729327 100644 --- a/src/iceberg/catalog/sql/sql_catalog.cc +++ b/src/iceberg/catalog/sql/sql_catalog.cc @@ -23,8 +23,10 @@ #include #include +#include "iceberg/catalog/catalog_util.h" #include "iceberg/catalog/sql/config.h" #include "iceberg/file_io.h" +#include "iceberg/metrics/metrics_reporters.h" #include "iceberg/table.h" #include "iceberg/table_identifier.h" #include "iceberg/table_metadata.h" @@ -128,10 +130,12 @@ Result ResolveTableLocation( } // namespace SqlCatalog::SqlCatalog(SqlCatalogConfig config, std::shared_ptr file_io, - std::shared_ptr store) + std::shared_ptr store, + std::shared_ptr reporter) : config_(std::move(config)), file_io_(std::move(file_io)), - store_(std::move(store)) {} + store_(std::move(store)), + reporter_(std::move(reporter)) {} SqlCatalog::~SqlCatalog() = default; @@ -144,10 +148,18 @@ Result> SqlCatalog::Make( if (file_io == nullptr) { return InvalidArgument("SqlCatalog requires a non-null FileIO"); } - auto catalog = std::shared_ptr( - new SqlCatalog(config, std::move(file_io), std::move(store))); - ICEBERG_RETURN_UNEXPECTED(catalog->store_->Initialize()); - return catalog; + ICEBERG_RETURN_UNEXPECTED(store->Initialize()); + + std::shared_ptr reporter; + const auto& props = config.props; + if (auto it = props.find(std::string(kMetricsReporterImpl)); + it != props.end() && !it->second.empty() && + it->second != kMetricsReporterTypeNoop) { + ICEBERG_ASSIGN_OR_RAISE(reporter, MetricsReporters::Load(props)); + } + + return std::shared_ptr( + new SqlCatalog(config, std::move(file_io), std::move(store), std::move(reporter))); } std::string_view SqlCatalog::name() const { return config_.name; } @@ -372,7 +384,8 @@ Result> SqlCatalog::LoadTableFrom( ICEBERG_ASSIGN_OR_RAISE(auto metadata, TableMetadataUtil::Read(*file_io_, metadata_location)); return Table::Make(identifier, std::move(metadata), metadata_location, file_io_, - shared_from_this()); + shared_from_this(), CatalogUtil::FullTableName(name(), identifier), + reporter_); } Result> SqlCatalog::LoadTable(const TableIdentifier& identifier) { @@ -410,7 +423,8 @@ Result> SqlCatalog::CreateTable( store_->InsertTable(ns_str, identifier.name, metadata_location)); return Table::Make(identifier, std::move(metadata), metadata_location, file_io_, - shared_from_this()); + shared_from_this(), CatalogUtil::FullTableName(name(), identifier), + reporter_); } Result> SqlCatalog::UpdateTable( @@ -475,7 +489,8 @@ Result> SqlCatalog::UpdateTable( } return Table::Make(identifier, std::move(updated), new_metadata_location, file_io_, - shared_from_this()); + shared_from_this(), CatalogUtil::FullTableName(name(), identifier), + reporter_); } Result> SqlCatalog::StageCreateTable( @@ -500,9 +515,10 @@ Result> SqlCatalog::StageCreateTable( ResolveTableLocation(config_, identifier, namespace_properties, location)); ICEBERG_ASSIGN_OR_RAISE(auto metadata, TableMetadata::Make(*schema, *spec, *order, base_location, properties)); - ICEBERG_ASSIGN_OR_RAISE(auto table, - StagedTable::Make(identifier, std::move(metadata), "", file_io_, - shared_from_this())); + ICEBERG_ASSIGN_OR_RAISE( + auto table, + StagedTable::Make(identifier, std::move(metadata), "", file_io_, shared_from_this(), + CatalogUtil::FullTableName(name(), identifier), reporter_)); return Transaction::Make(std::move(table), TransactionKind::kCreate); } @@ -582,7 +598,8 @@ Result> SqlCatalog::RegisterTable( store_->InsertTable(ns_str, identifier.name, metadata_file_location)); return Table::Make(identifier, std::move(metadata), metadata_file_location, file_io_, - shared_from_this()); + shared_from_this(), CatalogUtil::FullTableName(name(), identifier), + reporter_); } // -------------------------------------------------------------------------- diff --git a/src/iceberg/catalog/sql/sql_catalog.h b/src/iceberg/catalog/sql/sql_catalog.h index 35ef107b1..528452c12 100644 --- a/src/iceberg/catalog/sql/sql_catalog.h +++ b/src/iceberg/catalog/sql/sql_catalog.h @@ -172,7 +172,8 @@ class ICEBERG_SQL_CATALOG_EXPORT SqlCatalog private: SqlCatalog(SqlCatalogConfig config, std::shared_ptr file_io, - std::shared_ptr store); + std::shared_ptr store, + std::shared_ptr reporter); /// \brief Resolve the current metadata location for a table, or NoSuchTable. Result GetTableMetadataLocation(const TableIdentifier& identifier) const; @@ -184,6 +185,7 @@ class ICEBERG_SQL_CATALOG_EXPORT SqlCatalog SqlCatalogConfig config_; std::shared_ptr file_io_; std::shared_ptr store_; + std::shared_ptr reporter_; }; } // namespace iceberg::sql diff --git a/src/iceberg/delete_file_index.cc b/src/iceberg/delete_file_index.cc index 5f3f51742..634ab63bc 100644 --- a/src/iceberg/delete_file_index.cc +++ b/src/iceberg/delete_file_index.cc @@ -35,6 +35,7 @@ #include "iceberg/manifest/manifest_list.h" #include "iceberg/manifest/manifest_reader.h" #include "iceberg/metadata_columns.h" +#include "iceberg/metrics/scan_report.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" #include "iceberg/util/checked_cast.h" @@ -535,6 +536,11 @@ DeleteFileIndex::Builder& DeleteFileIndex::Builder::PlanWith(OptionalExecutor ex executor_ = executor; return *this; } +DeleteFileIndex::Builder& DeleteFileIndex::Builder::WithScanMetrics( + std::shared_ptr scan_metrics) { + scan_metrics_ = std::move(scan_metrics); + return *this; +} Result> DeleteFileIndex::Builder::LoadDeleteFiles() { // TODO(zehua): Replace with a thread-safe LRU cache. @@ -616,6 +622,7 @@ Result> DeleteFileIndex::Builder::LoadDeleteFiles() { return manifest_result; } if (!manifest.has_added_files() && !manifest.has_existing_files()) { + if (scan_metrics_) scan_metrics_->skipped_delete_manifests->Increment(1); return manifest_result; } @@ -638,14 +645,20 @@ Result> DeleteFileIndex::Builder::LoadDeleteFiles() { ICEBERG_ASSIGN_OR_RAISE(auto should_match, manifest_evaluator->Evaluate(manifest)); if (!should_match) { + if (scan_metrics_) scan_metrics_->skipped_delete_manifests->Increment(1); return manifest_result; } } + if (scan_metrics_) scan_metrics_->scanned_delete_manifests->Increment(1); // Read manifest entries ICEBERG_ASSIGN_OR_RAISE(auto reader, ManifestReader::Make(manifest, io_, schema_, spec)); + if (scan_metrics_) { + reader->SkipCounter(scan_metrics_->skipped_delete_files); + } + if (delete_partition_filter) { reader->FilterPartitions(std::move(delete_partition_filter)); } @@ -820,12 +833,30 @@ Result> DeleteFileIndex::Builder::Build() { } } - return std::unique_ptr(new DeleteFileIndex( + auto index = std::unique_ptr(new DeleteFileIndex( global_deletes->empty() ? nullptr : std::move(global_deletes), eq_deletes_by_partition->empty() ? nullptr : std::move(eq_deletes_by_partition), pos_deletes_by_partition->empty() ? nullptr : std::move(pos_deletes_by_partition), pos_deletes_by_path->empty() ? nullptr : std::move(pos_deletes_by_path), dv_by_path->empty() ? nullptr : std::move(dv_by_path))); + + if (scan_metrics_) { + for (const auto& delete_file : index->ReferencedDeleteFiles()) { + scan_metrics_->indexed_delete_files->Increment(1); + + if (delete_file->content == DataFile::Content::kPositionDeletes) { + if (ContentFileUtil::IsDV(*delete_file)) { + scan_metrics_->dvs->Increment(1); + } else { + scan_metrics_->positional_delete_files->Increment(1); + } + } else if (delete_file->content == DataFile::Content::kEqualityDeletes) { + scan_metrics_->equality_delete_files->Increment(1); + } + } + } + + return index; } } // namespace iceberg diff --git a/src/iceberg/delete_file_index.h b/src/iceberg/delete_file_index.h index 555114a23..c8e34ad47 100644 --- a/src/iceberg/delete_file_index.h +++ b/src/iceberg/delete_file_index.h @@ -363,6 +363,9 @@ class ICEBERG_EXPORT DeleteFileIndex::Builder : public ErrorCollector { /// \return Reference to this for method chaining. Builder& PlanWith(OptionalExecutor executor); + /// \brief Attach scan metrics for counting scanned/skipped delete manifests. + Builder& WithScanMetrics(std::shared_ptr scan_metrics); + /// \brief Build the DeleteFileIndex. Result> Build(); @@ -398,6 +401,7 @@ class ICEBERG_EXPORT DeleteFileIndex::Builder : public ErrorCollector { OptionalExecutor executor_; bool case_sensitive_ = true; bool ignore_residuals_ = false; + std::shared_ptr scan_metrics_; }; } // namespace iceberg diff --git a/src/iceberg/expression/meson.build b/src/iceberg/expression/meson.build index 9b143ad31..d8abe8d2a 100644 --- a/src/iceberg/expression/meson.build +++ b/src/iceberg/expression/meson.build @@ -30,6 +30,7 @@ install_headers( 'projections.h', 'residual_evaluator.h', 'rewrite_not.h', + 'sanitize_expression.h', 'strict_metrics_evaluator.h', 'term.h', ], diff --git a/src/iceberg/expression/sanitize_expression.cc b/src/iceberg/expression/sanitize_expression.cc new file mode 100644 index 000000000..1fc5694ed --- /dev/null +++ b/src/iceberg/expression/sanitize_expression.cc @@ -0,0 +1,363 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/expression/sanitize_expression.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/expression/binder.h" +#include "iceberg/expression/literal.h" +#include "iceberg/expression/predicate.h" +#include "iceberg/expression/term.h" +#include "iceberg/transform.h" +#include "iceberg/type.h" +#include "iceberg/util/bucket_util.h" +#include "iceberg/util/checked_cast.h" +#include "iceberg/util/temporal_util.h" + +namespace iceberg { + +namespace { + +std::string SanitizeDate(int32_t days, int32_t today) { + std::string is_past = today > days ? "ago" : "from-now"; + uint32_t diff = today > days ? static_cast(static_cast(today) - + static_cast(days)) + : static_cast(static_cast(days) - + static_cast(today)); + if (diff == 0) { + return "(date-today)"; + } else if (diff < 90) { + return "(date-" + std::to_string(diff) + "-days-" + is_past + ")"; + } + + return "(date)"; +} + +std::string SanitizeTimestamp(int64_t micros, int64_t now) { + constexpr int64_t kMicrosPerHour = 60LL * 60LL * 1'000'000LL; + constexpr int64_t kFiveMinutesInMicros = 5LL * 60LL * 1'000'000LL; + constexpr int64_t kThreeDaysInHours = 3LL * 24LL; + constexpr int64_t kNinetyDaysInHours = 90LL * 24LL; + + std::string is_past = now > micros ? "ago" : "from-now"; + uint64_t diff = now > micros ? static_cast(static_cast(now) - + static_cast(micros)) + : static_cast(static_cast(micros) - + static_cast(now)); + if (diff < kFiveMinutesInMicros) { + return "(timestamp-about-now)"; + } + + int64_t hours = diff / kMicrosPerHour; + if (hours <= kThreeDaysInHours) { + return "(timestamp-" + std::to_string(hours) + "-hours-" + is_past + ")"; + } else if (hours < kNinetyDaysInHours) { + int64_t days = hours / 24; + return "(timestamp-" + std::to_string(days) + "-days-" + is_past + ")"; + } + + return "(timestamp)"; +} + +std::string SanitizeNumber(double value, std::string_view type) { + if (!std::isfinite(value)) { + return std::format("({})", type); + } + int32_t num_digits = + value == 0 ? 1 : static_cast(std::log10(std::abs(value))) + 1; + return std::format("({}-digit-{})", num_digits, type); +} + +Result SanitizeSimpleString(std::string_view value) { + ICEBERG_ASSIGN_OR_RAISE(auto hash, + BucketUtils::BucketIndex(Literal::String(std::string(value)), + std::numeric_limits::max())); + return std::format("(hash-{:08x})", hash); +} + +Result SanitizeString(std::string_view value, int64_t now, int32_t today) { + static const std::regex kDate(R"(\d{4}-\d{2}-\d{2})"); + static const std::regex kTime(R"(\d{2}:\d{2}(:\d{2}(.\d{1,9})?)?)"); + static const std::regex kTimestamp( + R"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(.\d{1,9})?)?)"); + static const std::regex kTimestampTz( + R"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(.\d{1,9})?)?([-+]\d{2}:\d{2}|Z))"); + static const std::regex kTimestampNs( + R"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(.\d{7,9})?)?)"); + static const std::regex kTimestampTzNs( + R"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(.\d{7,9})?)?([-+]\d{2}:\d{2}|Z))"); + + try { + if (std::regex_match(value.begin(), value.end(), kDate)) { + auto days = TemporalUtils::ParseDay(value); + return days.has_value() ? SanitizeDate(*days, today) : SanitizeSimpleString(value); + } + if (std::regex_match(value.begin(), value.end(), kTimestampNs)) { + auto nanos = TemporalUtils::ParseTimestampNs(value); + return nanos.has_value() + ? SanitizeTimestamp(TemporalUtils::NanosToMicros(*nanos), now) + : SanitizeSimpleString(value); + } + if (std::regex_match(value.begin(), value.end(), kTimestampTzNs)) { + auto nanos = TemporalUtils::ParseTimestampNsWithZone(value); + return nanos.has_value() + ? SanitizeTimestamp(TemporalUtils::NanosToMicros(*nanos), now) + : SanitizeSimpleString(value); + } + if (std::regex_match(value.begin(), value.end(), kTimestamp)) { + auto micros = TemporalUtils::ParseTimestamp(value); + return micros.has_value() ? SanitizeTimestamp(*micros, now) + : SanitizeSimpleString(value); + } + if (std::regex_match(value.begin(), value.end(), kTimestampTz)) { + auto micros = TemporalUtils::ParseTimestampWithZone(value); + return micros.has_value() ? SanitizeTimestamp(*micros, now) + : SanitizeSimpleString(value); + } + if (std::regex_match(value.begin(), value.end(), kTime)) { + return std::string("(time)"); + } + return SanitizeSimpleString(value); + } catch (const std::exception&) { + return SanitizeSimpleString(value); + } +} + +Result SanitizePlaceholder(const Literal& literal, int64_t now, + int32_t today) { + if (literal.IsNull()) { + return std::string("(null)"); + } + const auto& value = literal.value(); + switch (literal.type()->type_id()) { + case TypeId::kString: + return SanitizeString(std::get(value), now, today); + case TypeId::kDate: + return SanitizeDate(std::get(value), today); + case TypeId::kTimestamp: + case TypeId::kTimestampTz: + return SanitizeTimestamp(std::get(value), now); + case TypeId::kTimestampNs: + case TypeId::kTimestampTzNs: + return SanitizeTimestamp(TemporalUtils::NanosToMicros(std::get(value)), + now); + case TypeId::kTime: + return std::string("(time)"); + case TypeId::kInt: + return SanitizeNumber(std::get(value), "int"); + case TypeId::kLong: + return SanitizeNumber(static_cast(std::get(value)), "int"); + case TypeId::kFloat: + return SanitizeNumber(std::get(value), "float"); + case TypeId::kDouble: + return SanitizeNumber(std::get(value), "float"); + case TypeId::kBinary: + case TypeId::kFixed: + return SanitizeSimpleString(literal.ToString()); + default: + return SanitizeSimpleString(literal.ToString()); + } +} + +Result SanitizeLiteral(const Literal& literal, int64_t now, int32_t today) { + ICEBERG_ASSIGN_OR_RAISE(auto placeholder, SanitizePlaceholder(literal, now, today)); + return Literal::String(std::move(placeholder)); +} + +Result>> MakeSanitizedTransformTerm( + std::string_view name, const std::shared_ptr& transform) { + ICEBERG_ASSIGN_OR_RAISE(std::shared_ptr named_ref, + NamedReference::Make(std::string(name))); + return UnboundTransform::Make(std::move(named_ref), transform); +} + +template +Result> MakeSanitizedPredicateOverTerm( + Expression::Operation op, std::shared_ptr> term, + std::vector values) { + if (values.empty()) { + return UnboundPredicateImpl::Make(op, term); + } + if (values.size() == 1) { + return UnboundPredicateImpl::Make(op, term, std::move(values[0])); + } + return UnboundPredicateImpl::Make(op, term, std::move(values)); +} + +// Rebuilds a sanitized predicate over a bound `term`, preserving whether it was a plain +// column reference or a transform -- only the literal values are replaced with +// placeholders. +Result> MakeSanitizedPredicate( + Expression::Operation op, const std::shared_ptr& term, + std::vector values) { + if (term->kind() == Term::Kind::kTransform) { + const auto& bound_transform = internal::checked_cast(*term); + ICEBERG_ASSIGN_OR_RAISE(auto transform_term, + MakeSanitizedTransformTerm(term->reference()->name(), + bound_transform.transform())); + return MakeSanitizedPredicateOverTerm(op, std::move(transform_term), + std::move(values)); + } + ICEBERG_ASSIGN_OR_RAISE(auto named_ref, + NamedReference::Make(std::string(term->reference()->name()))); + return MakeSanitizedPredicateOverTerm(op, std::move(named_ref), + std::move(values)); +} + +template +Result> MakeSanitizedUnboundPredicate( + const std::shared_ptr& pred, std::vector values) { + auto typed_pred = std::dynamic_pointer_cast>(pred); + if (typed_pred == nullptr) [[unlikely]] { + return InvalidExpression("Unexpected unbound predicate term type"); + } + return MakeSanitizedPredicateOverTerm(pred->op(), typed_pred->term(), + std::move(values)); +} + +} // namespace + +SanitizeExpression::SanitizeExpression() { + auto now = std::chrono::system_clock::now().time_since_epoch(); + auto now_millis = std::chrono::duration_cast(now); + now_ = std::chrono::duration_cast(now_millis).count(); + today_ = static_cast( + std::chrono::duration_cast(now_millis).count()); +} + +Result> SanitizeExpression::Sanitize( + const std::shared_ptr& expr) { + ICEBERG_DCHECK(expr != nullptr, "Expression cannot be null"); + SanitizeExpression visitor; + return Visit, SanitizeExpression>(expr, visitor); +} + +Result> SanitizeExpression::AlwaysTrue() { + return True::Instance(); +} + +Result> SanitizeExpression::AlwaysFalse() { + return False::Instance(); +} + +Result> SanitizeExpression::Not( + const std::shared_ptr& child_result) { + return Not::MakeFolded(child_result); +} + +Result> SanitizeExpression::And( + const std::shared_ptr& left_result, + const std::shared_ptr& right_result) { + return And::MakeFolded(left_result, right_result); +} + +Result> SanitizeExpression::Or( + const std::shared_ptr& left_result, + const std::shared_ptr& right_result) { + return Or::MakeFolded(left_result, right_result); +} + +Result> SanitizeExpression::Predicate( + const std::shared_ptr& pred) { + switch (pred->kind()) { + case BoundPredicate::Kind::kUnary: + return MakeSanitizedPredicate(pred->op(), pred->term(), {}); + case BoundPredicate::Kind::kLiteral: { + const auto& literal_pred = + internal::checked_cast(*pred); + ICEBERG_ASSIGN_OR_RAISE(auto placeholder, + SanitizeLiteral(literal_pred.literal(), now_, today_)); + return MakeSanitizedPredicate(pred->op(), pred->term(), {std::move(placeholder)}); + } + case BoundPredicate::Kind::kSet: { + const auto& set_pred = internal::checked_cast(*pred); + std::vector placeholders; + placeholders.reserve(set_pred.literal_set().size()); + for (const auto& literal : set_pred.literal_set()) { + ICEBERG_ASSIGN_OR_RAISE(auto placeholder, SanitizeLiteral(literal, now_, today_)); + placeholders.push_back(std::move(placeholder)); + } + return MakeSanitizedPredicate(pred->op(), pred->term(), std::move(placeholders)); + } + } + return InvalidExpression("Unsupported bound predicate kind for sanitization"); +} + +Result> SanitizeExpression::Predicate( + const std::shared_ptr& pred) { + switch (pred->op()) { + case Expression::Operation::kIsNull: + case Expression::Operation::kNotNull: + case Expression::Operation::kIsNan: + case Expression::Operation::kNotNan: + return pred; + case Expression::Operation::kLt: + case Expression::Operation::kLtEq: + case Expression::Operation::kGt: + case Expression::Operation::kGtEq: + case Expression::Operation::kEq: + case Expression::Operation::kNotEq: + case Expression::Operation::kStartsWith: + case Expression::Operation::kNotStartsWith: + case Expression::Operation::kIn: + case Expression::Operation::kNotIn: + break; + default: + return InvalidExpression( + "Unsupported unbound predicate operation for sanitization"); + } + + auto literals = pred->literals(); + std::vector placeholders; + placeholders.reserve(literals.size()); + for (const auto& literal : literals) { + ICEBERG_ASSIGN_OR_RAISE(auto placeholder, SanitizeLiteral(literal, now_, today_)); + placeholders.push_back(std::move(placeholder)); + } + switch (pred->unbound_term().kind()) { + case Term::Kind::kReference: + return MakeSanitizedUnboundPredicate(pred, std::move(placeholders)); + case Term::Kind::kTransform: + return MakeSanitizedUnboundPredicate(pred, std::move(placeholders)); + case Term::Kind::kExtract: + return NotSupported("Cannot sanitize an extract predicate"); + } + std::unreachable(); +} + +Result> SanitizeExpression::Sanitize( + const Schema& schema, const std::shared_ptr& expr, bool case_sensitive) { + auto bound = Binder::Bind(schema, expr, case_sensitive); + if (bound.has_value()) { + return Sanitize(*bound); + } + return Sanitize(expr); +} +// TODO(evindj) : add StringSanitizer for logging. +} // namespace iceberg diff --git a/src/iceberg/expression/sanitize_expression.h b/src/iceberg/expression/sanitize_expression.h new file mode 100644 index 000000000..e66387e8c --- /dev/null +++ b/src/iceberg/expression/sanitize_expression.h @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/expression/sanitize_expression.h +/// Replace literal values in an expression with type-aware placeholders. + +#include +#include +#include + +#include "iceberg/expression/expression_visitor.h" +#include "iceberg/iceberg_export.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Rewrites an expression tree as an unbound expression whose literal values +/// are replaced with type-aware placeholders (e.g. "(2-digit-int)", +/// "(hash-3f9a1c02)", "(date-5-days-ago)"). +class ICEBERG_EXPORT SanitizeExpression + : public ExpressionVisitor> { + public: + /// \brief Sanitize an expression tree and return an unbound expression. + static Result> Sanitize( + const std::shared_ptr& expr); + + /// \brief Bind `expr` to `schema` first, falling back to sanitizing the unbound + /// expression if binding fails. + static Result> Sanitize( + const Schema& schema, const std::shared_ptr& expr, bool case_sensitive); + + Result> AlwaysTrue() override; + Result> AlwaysFalse() override; + Result> Not( + const std::shared_ptr& child_result) override; + Result> And( + const std::shared_ptr& left_result, + const std::shared_ptr& right_result) override; + Result> Or( + const std::shared_ptr& left_result, + const std::shared_ptr& right_result) override; + Result> Predicate( + const std::shared_ptr& pred) override; + Result> Predicate( + const std::shared_ptr& pred) override; + + private: + SanitizeExpression(); + + // Microseconds since the Unix epoch, at millisecond precision. + int64_t now_; + // UTC days since the Unix epoch. + int32_t today_; +}; + +} // namespace iceberg diff --git a/src/iceberg/manifest/manifest_group.cc b/src/iceberg/manifest/manifest_group.cc index ec5eb66bc..3db0a1df4 100644 --- a/src/iceberg/manifest/manifest_group.cc +++ b/src/iceberg/manifest/manifest_group.cc @@ -36,6 +36,7 @@ #include "iceberg/expression/residual_evaluator.h" #include "iceberg/file_io.h" #include "iceberg/manifest/manifest_reader.h" +#include "iceberg/metrics/scan_report.h" #include "iceberg/partition_spec.h" #include "iceberg/row/manifest_wrapper.h" #include "iceberg/schema.h" @@ -198,6 +199,11 @@ ManifestGroup& ManifestGroup::PlanWith(OptionalExecutor executor) { return *this; } +ManifestGroup& ManifestGroup::WithScanMetrics(std::shared_ptr scan_metrics) { + scan_metrics_ = std::move(scan_metrics); + return *this; +} + Result>> ManifestGroup::PlanFiles() { auto create_file_scan_tasks = [this](std::vector&& entries, @@ -212,6 +218,20 @@ Result>> ManifestGroup::PlanFiles() { ContentFileUtil::DropUnselectedStats(*entry.data_file, ctx.columns_to_keep_stats); } ICEBERG_ASSIGN_OR_RAISE(auto delete_files, ctx.deletes->ForEntry(entry)); + // Count result metrics once per data file task. A delete file shared by + // multiple data files contributes once to each task, unlike indexed delete files. + if (scan_metrics_) { + scan_metrics_->total_file_size_in_bytes->Increment( + ContentFileUtil::ContentSizeInBytes(*entry.data_file)); + scan_metrics_->result_data_files->Increment(1); + scan_metrics_->result_delete_files->Increment( + static_cast(delete_files.size())); + int64_t deletes_size = 0; + for (const auto& delete_file : delete_files) { + deletes_size += ContentFileUtil::ContentSizeInBytes(*delete_file); + } + scan_metrics_->total_delete_file_size_in_bytes->Increment(deletes_size); + } ICEBERG_ASSIGN_OR_RAISE(auto residual, ctx.residuals->ResidualFor(entry.data_file->partition)); tasks.push_back(std::make_shared( @@ -254,6 +274,7 @@ Result>> ManifestGroup::Plan( return residual_cache[spec_id].get(); }; + delete_index_builder_.WithScanMetrics(scan_metrics_); ICEBERG_ASSIGN_OR_RAISE(auto delete_index, delete_index_builder_.Build()); bool drop_stats = ManifestReader::ShouldDropStats(columns_); @@ -346,6 +367,10 @@ Result> ManifestGroup::MakeReader( .CaseSensitive(case_sensitive_) .Select(std::move(columns)); + if (scan_metrics_) { + reader->SkipCounter(scan_metrics_->skipped_data_files); + } + return reader; } @@ -408,12 +433,15 @@ ManifestGroup::ReadEntries() { manifest_evaluator->Evaluate(manifest)); if (!should_match) { // Skip this manifest because it doesn't match partition filter + if (scan_metrics_) { + scan_metrics_->skipped_data_manifests->Increment(1); + } return {}; } - if (ignore_deleted_) { // only scan manifests that have entries other than deletes if (!manifest.has_added_files() && !manifest.has_existing_files()) { + if (scan_metrics_) scan_metrics_->skipped_data_manifests->Increment(1); return {}; } } @@ -421,10 +449,15 @@ ManifestGroup::ReadEntries() { if (ignore_existing_) { // only scan manifests that have entries other than existing if (!manifest.has_added_files() && !manifest.has_deleted_files()) { + if (scan_metrics_) scan_metrics_->skipped_data_manifests->Increment(1); return {}; } } + if (scan_metrics_) { + scan_metrics_->scanned_data_manifests->Increment(1); + } + // Read manifest entries ICEBERG_ASSIGN_OR_RAISE(auto reader, MakeReader(manifest)); ICEBERG_ASSIGN_OR_RAISE( @@ -434,6 +467,7 @@ ManifestGroup::ReadEntries() { for (auto& entry : entries) { if (ignore_existing_ && entry.status == ManifestStatus::kExisting) { + if (scan_metrics_) scan_metrics_->skipped_data_files->Increment(1); continue; } @@ -442,11 +476,13 @@ ManifestGroup::ReadEntries() { ICEBERG_ASSIGN_OR_RAISE(bool should_match, data_file_evaluator->Evaluate(data_file)); if (!should_match) { + if (scan_metrics_) scan_metrics_->skipped_data_files->Increment(1); continue; } } if (!manifest_entry_predicate_(entry)) { + if (scan_metrics_) scan_metrics_->skipped_data_files->Increment(1); continue; } diff --git a/src/iceberg/manifest/manifest_group.h b/src/iceberg/manifest/manifest_group.h index 09ae4a503..be0ca4b8b 100644 --- a/src/iceberg/manifest/manifest_group.h +++ b/src/iceberg/manifest/manifest_group.h @@ -130,6 +130,9 @@ class ICEBERG_EXPORT ManifestGroup : public ErrorCollector { /// \return Reference to this for method chaining. ManifestGroup& PlanWith(OptionalExecutor executor); + /// \brief Attach scan metrics to receive per-manifest and per-file counters. + ManifestGroup& WithScanMetrics(std::shared_ptr scan_metrics); + /// \brief Plan scan tasks for all matching data files. Result>> PlanFiles(); @@ -173,6 +176,7 @@ class ICEBERG_EXPORT ManifestGroup : public ErrorCollector { bool ignore_deleted_ = false; bool ignore_existing_ = false; bool ignore_residuals_ = false; + std::shared_ptr scan_metrics_; }; } // namespace iceberg diff --git a/src/iceberg/manifest/manifest_reader.cc b/src/iceberg/manifest/manifest_reader.cc index 8757b5d61..68760b440 100644 --- a/src/iceberg/manifest/manifest_reader.cc +++ b/src/iceberg/manifest/manifest_reader.cc @@ -37,6 +37,7 @@ #include "iceberg/manifest/manifest_entry.h" #include "iceberg/manifest/manifest_list.h" #include "iceberg/manifest/manifest_reader_internal.h" +#include "iceberg/metrics/counter.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" #include "iceberg/schema_field.h" @@ -772,6 +773,11 @@ ManifestReader& ManifestReaderImpl::TryDropStats() { return *this; } +ManifestReader& ManifestReaderImpl::SkipCounter(std::shared_ptr counter) { + skip_counter_ = std::move(counter); + return *this; +} + bool ManifestReaderImpl::HasPartitionFilter() const { ICEBERG_DCHECK(part_filter_, "Partition filter is not set"); return part_filter_->op() != Expression::Operation::kTrue; @@ -928,6 +934,7 @@ Result> ManifestReaderImpl::ReadEntries(bool only_liv ICEBERG_ASSIGN_OR_RAISE(bool partition_match, evaluator->Evaluate(entry.data_file->partition)); if (!partition_match) { + if (skip_counter_) skip_counter_->Increment(1); continue; } } @@ -935,11 +942,13 @@ Result> ManifestReaderImpl::ReadEntries(bool only_liv ICEBERG_ASSIGN_OR_RAISE(bool metrics_match, metrics_evaluator->Evaluate(*entry.data_file)); if (!metrics_match) { + if (skip_counter_) skip_counter_->Increment(1); continue; } } ICEBERG_ASSIGN_OR_RAISE(bool in_partition_set, InPartitionSet(*entry.data_file)); if (!in_partition_set) { + if (skip_counter_) skip_counter_->Increment(1); continue; } } diff --git a/src/iceberg/manifest/manifest_reader.h b/src/iceberg/manifest/manifest_reader.h index b2d1c6505..72cb9ae56 100644 --- a/src/iceberg/manifest/manifest_reader.h +++ b/src/iceberg/manifest/manifest_reader.h @@ -30,6 +30,7 @@ #include #include "iceberg/iceberg_export.h" +#include "iceberg/metrics/counter.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -76,6 +77,9 @@ class ICEBERG_EXPORT ManifestReader { /// \brief Try to drop stats from returned DataFile objects. virtual ManifestReader& TryDropStats() = 0; + /// \brief Set a counter to increment for each entry skipped by per-entry filters. + virtual ManifestReader& SkipCounter(std::shared_ptr counter) = 0; + /// \brief Determine whether stats should be dropped based on selected columns. /// /// Returns true if the selected columns do not include any stats columns, or only diff --git a/src/iceberg/manifest/manifest_reader_internal.h b/src/iceberg/manifest/manifest_reader_internal.h index 53ce2fcb5..4ad708e43 100644 --- a/src/iceberg/manifest/manifest_reader_internal.h +++ b/src/iceberg/manifest/manifest_reader_internal.h @@ -33,6 +33,7 @@ #include "iceberg/file_reader.h" #include "iceberg/inheritable_metadata.h" #include "iceberg/manifest/manifest_reader.h" +#include "iceberg/type_fwd.h" #include "iceberg/util/partition_value_util.h" namespace iceberg { @@ -77,6 +78,8 @@ class ManifestReaderImpl : public ManifestReader { ManifestReader& TryDropStats() override; + ManifestReader& SkipCounter(std::shared_ptr counter) override; + private: /// \brief Read entries with optional live-only filtering. Result> ReadEntries(bool only_live); @@ -114,6 +117,7 @@ class ManifestReaderImpl : public ManifestReader { std::shared_ptr part_filter_{True::Instance()}; std::shared_ptr row_filter_{True::Instance()}; std::shared_ptr partition_set_; + std::shared_ptr skip_counter_; bool case_sensitive_{true}; bool drop_stats_{false}; diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index de39e53d7..39e8b2939 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -70,6 +70,7 @@ iceberg_sources = files( 'arrow_c_data_guard_internal.cc', 'arrow_c_data_util.cc', 'arrow_row_builder.cc', + 'catalog/catalog_util.cc', 'catalog/memory/in_memory_catalog.cc', 'catalog/session_catalog.cc', 'catalog/session_context.cc', @@ -87,6 +88,7 @@ iceberg_sources = files( 'expression/projections.cc', 'expression/residual_evaluator.cc', 'expression/rewrite_not.cc', + 'expression/sanitize_expression.cc', 'expression/strict_metrics_evaluator.cc', 'expression/term.cc', 'file_io.cc', diff --git a/src/iceberg/table.cc b/src/iceberg/table.cc index 0a2b54082..f9e7026f9 100644 --- a/src/iceberg/table.cc +++ b/src/iceberg/table.cc @@ -56,7 +56,9 @@ Result> Table::Make(TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, - std::shared_ptr catalog) { + std::shared_ptr catalog, + std::string full_name, + std::shared_ptr reporter) { if (metadata == nullptr) [[unlikely]] { return InvalidArgument("Metadata cannot be null"); } @@ -69,21 +71,24 @@ Result> Table::Make(TableIdentifier identifier, if (catalog == nullptr) [[unlikely]] { return InvalidArgument("Catalog cannot be null"); } - return std::shared_ptr(new Table(std::move(identifier), std::move(metadata), - std::move(metadata_location), std::move(io), - std::move(catalog))); + return std::shared_ptr
(new Table( + std::move(identifier), std::move(metadata), std::move(metadata_location), + std::move(io), std::move(catalog), std::move(full_name), std::move(reporter))); } Table::~Table() = default; Table::Table(TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, - std::shared_ptr catalog) + std::shared_ptr catalog, std::string full_name, + std::shared_ptr reporter) : identifier_(std::move(identifier)), + full_name_(full_name.empty() ? identifier_.ToString() : std::move(full_name)), metadata_(std::move(metadata)), metadata_location_(std::move(metadata_location)), io_(std::move(io)), catalog_(std::move(catalog)), + reporter_(std::move(reporter)), metadata_cache_(std::make_unique(metadata_.get())) {} const std::string& Table::uuid() const { return metadata_->table_uuid; } @@ -156,22 +161,24 @@ const std::shared_ptr& Table::metadata() const { return metadata_ const std::shared_ptr& Table::catalog() const { return catalog_; } +const std::shared_ptr& Table::reporter() const { return reporter_; } + Result> Table::location_provider() const { return LocationProvider::Make(metadata_->location, metadata_->properties); } Result> Table::NewScan() const { - return DataTableScanBuilder::Make(metadata_, io_); + return DataTableScanBuilder::Make(*this); } Result> Table::NewIncrementalAppendScan() const { - return IncrementalAppendScanBuilder::Make(metadata_, io_); + return IncrementalAppendScanBuilder::Make(*this); } Result> Table::NewIncrementalChangelogScan() const { - return IncrementalChangelogScanBuilder::Make(metadata_, io_); + return IncrementalChangelogScanBuilder::Make(*this); } Result> Table::NewTransaction() { @@ -271,7 +278,8 @@ Result> Table::NewSnapshotManager() { Result> StagedTable::Make( TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, - std::shared_ptr catalog) { + std::shared_ptr catalog, std::string full_name, + std::shared_ptr reporter) { if (metadata == nullptr) [[unlikely]] { return InvalidArgument("Metadata cannot be null"); } @@ -281,9 +289,9 @@ Result> StagedTable::Make( if (catalog == nullptr) [[unlikely]] { return InvalidArgument("Catalog cannot be null"); } - return std::shared_ptr( - new StagedTable(std::move(identifier), std::move(metadata), - std::move(metadata_location), std::move(io), std::move(catalog))); + return std::shared_ptr(new StagedTable( + std::move(identifier), std::move(metadata), std::move(metadata_location), + std::move(io), std::move(catalog), std::move(full_name), std::move(reporter))); } StagedTable::~StagedTable() = default; @@ -294,16 +302,16 @@ Result> StagedTable::NewScan() const { Result> StaticTable::Make( TableIdentifier identifier, std::shared_ptr metadata, - std::string metadata_location, std::shared_ptr io) { + std::string metadata_location, std::shared_ptr io, std::string full_name) { if (metadata == nullptr) [[unlikely]] { return InvalidArgument("Metadata cannot be null"); } if (io == nullptr) [[unlikely]] { return InvalidArgument("FileIO cannot be null"); } - return std::shared_ptr( - new StaticTable(std::move(identifier), std::move(metadata), - std::move(metadata_location), std::move(io), /*catalog=*/nullptr)); + return std::shared_ptr(new StaticTable( + std::move(identifier), std::move(metadata), std::move(metadata_location), + std::move(io), /*catalog=*/nullptr, std::move(full_name))); } StaticTable::~StaticTable() = default; diff --git a/src/iceberg/table.h b/src/iceberg/table.h index f9b72302a..eb028c738 100644 --- a/src/iceberg/table.h +++ b/src/iceberg/table.h @@ -46,17 +46,24 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ /// \param[in] metadata_location The location of the table metadata file. /// \param[in] io The FileIO to read and write table data and metadata files. /// \param[in] catalog The catalog that this table belongs to. - static Result> Make(TableIdentifier identifier, - std::shared_ptr metadata, - std::string metadata_location, - std::shared_ptr io, - std::shared_ptr catalog); + /// \param[in] full_name The fully-qualified name of this table. Defaults to the + /// string representation of identifier when empty. + /// \param[in] reporter Optional metrics reporter for this table. Defaults to nullptr + /// (noop). + static Result> Make( + TableIdentifier identifier, std::shared_ptr metadata, + std::string metadata_location, std::shared_ptr io, + std::shared_ptr catalog, std::string full_name = "", + std::shared_ptr reporter = nullptr); virtual ~Table(); /// \brief Returns the identifier of this table const TableIdentifier& name() const { return identifier_; } + /// \brief Returns the fully-qualified name of this table. + const std::string& full_name() const { return full_name_; } + /// \brief Returns the UUID of the table const std::string& uuid() const; @@ -120,6 +127,9 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ /// \brief Returns the catalog that this table belongs to const std::shared_ptr& catalog() const; + /// \brief Returns the metrics reporter for this table. + const std::shared_ptr& reporter() const; + /// \brief Returns a LocationProvider for this table Result> location_provider() const; @@ -201,13 +211,16 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ protected: Table(TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, - std::shared_ptr catalog); + std::shared_ptr catalog, std::string full_name, + std::shared_ptr reporter = nullptr); const TableIdentifier identifier_; + const std::string full_name_; std::shared_ptr metadata_; std::string metadata_location_; std::shared_ptr io_; std::shared_ptr catalog_; + std::shared_ptr reporter_; std::unique_ptr metadata_cache_; }; @@ -217,7 +230,8 @@ class ICEBERG_EXPORT StagedTable final : public Table { static Result> Make( TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, - std::shared_ptr catalog); + std::shared_ptr catalog, std::string full_name = "", + std::shared_ptr reporter = nullptr); ~StagedTable() override; @@ -235,7 +249,8 @@ class ICEBERG_EXPORT StaticTable : public Table { public: static Result> Make( TableIdentifier identifier, std::shared_ptr metadata, - std::string metadata_location, std::shared_ptr io); + std::string metadata_location, std::shared_ptr io, + std::string full_name = ""); ~StaticTable() override; diff --git a/src/iceberg/table_scan.cc b/src/iceberg/table_scan.cc index 5f7d03648..ef4e94c5b 100644 --- a/src/iceberg/table_scan.cc +++ b/src/iceberg/table_scan.cc @@ -19,17 +19,23 @@ #include "iceberg/table_scan.h" +#include #include #include #include "iceberg/expression/binder.h" #include "iceberg/expression/expression.h" #include "iceberg/expression/residual_evaluator.h" +#include "iceberg/expression/sanitize_expression.h" #include "iceberg/manifest/manifest_entry.h" #include "iceberg/manifest/manifest_group.h" +#include "iceberg/metrics/metrics_context.h" +#include "iceberg/metrics/metrics_reporters.h" +#include "iceberg/metrics/scan_report.h" #include "iceberg/result.h" #include "iceberg/schema.h" #include "iceberg/snapshot.h" +#include "iceberg/table.h" #include "iceberg/table_metadata.h" #include "iceberg/util/content_file_util.h" #include "iceberg/util/macros.h" @@ -215,17 +221,23 @@ int64_t ChangelogScanTask::estimated_row_count() const { // Generic template implementation for Make template Result>> TableScanBuilder::Make( - std::shared_ptr metadata, std::shared_ptr io) { - ICEBERG_PRECHECK(metadata != nullptr, "Table metadata cannot be null"); - ICEBERG_PRECHECK(io != nullptr, "FileIO cannot be null"); - return std::unique_ptr>( - new TableScanBuilder(std::move(metadata), std::move(io))); + const Table& table) { + ICEBERG_PRECHECK(table.metadata() != nullptr, "Table metadata cannot be null"); + ICEBERG_PRECHECK(table.io() != nullptr, "FileIO cannot be null"); + auto builder = + std::unique_ptr>(new TableScanBuilder( + table.metadata(), table.io(), table.full_name(), table.reporter())); + return builder; } template TableScanBuilder::TableScanBuilder( - std::shared_ptr table_metadata, std::shared_ptr file_io) - : metadata_(std::move(table_metadata)), io_(std::move(file_io)) {} + std::shared_ptr table_metadata, std::shared_ptr file_io, + std::string table_name, std::shared_ptr metrics_reporter) + : metadata_(std::move(table_metadata)), io_(std::move(file_io)) { + context_.table_name = std::move(table_name); + context_.metrics_reporter = std::move(metrics_reporter); +} template TableScanBuilder& TableScanBuilder::Option(std::string key, @@ -436,6 +448,14 @@ TableScanBuilder::ResolveSnapshotSchema() { return snapshot_schema_; } +template +TableScanBuilder& TableScanBuilder::ReportWith( + std::shared_ptr reporter) { + context_.metrics_reporter = + MetricsReporters::Combine(context_.metrics_reporter, std::move(reporter)); + return *this; +} + template Result> TableScanBuilder::Build() { ICEBERG_RETURN_UNEXPECTED(CheckErrors()); @@ -546,12 +566,62 @@ Result> DataTableScan::Make( std::move(metadata), std::move(schema), std::move(io), std::move(context))); } +Status DataTableScan::ReportScan(const Snapshot& snapshot, + const ScanMetrics& scan_metrics) const { + if (!context_.metrics_reporter) { + return {}; + } + + ICEBERG_ASSIGN_OR_RAISE(auto projected_schema, ResolveProjectedSchema()); + const auto& schema_ptr = projected_schema.get(); + + ICEBERG_ASSIGN_OR_RAISE( + auto projected_id_set, + GetProjectedIdsVisitor::GetProjectedIds(*schema_ptr, /*include_struct_ids=*/true)); + std::vector projected_field_ids(projected_id_set.begin(), + projected_id_set.end()); + std::ranges::sort(projected_field_ids); + + std::vector projected_field_names; + projected_field_names.reserve(projected_field_ids.size()); + for (int32_t field_id : projected_field_ids) { + ICEBERG_ASSIGN_OR_RAISE(auto field_name, schema_ptr->FindColumnNameById(field_id)); + ICEBERG_CHECK(field_name.has_value(), "Projected field {} not found in schema", + field_id); + projected_field_names.emplace_back(*field_name); + } + + ICEBERG_ASSIGN_OR_RAISE( + auto sanitized_filter, + SanitizeExpression::Sanitize(*schema_ptr, filter(), context_.case_sensitive)); + + ScanReport report{ + .table_name = context_.table_name, + .snapshot_id = snapshot.snapshot_id, + .filter = std::move(sanitized_filter), + .schema_id = schema_ptr->schema_id(), + .projected_field_ids = std::move(projected_field_ids), + .projected_field_names = std::move(projected_field_names), + .scan_metrics = scan_metrics.ToResult(), + .metadata = context_.options, + }; + return context_.metrics_reporter->Report(report); +} + Result>> DataTableScan::PlanFiles() const { ICEBERG_ASSIGN_OR_RAISE(auto snapshot, this->snapshot()); if (!snapshot) { return std::vector>{}; } + std::shared_ptr scan_metrics; + std::optional planning_duration; + if (context_.metrics_reporter) { + auto metrics_context = MetricsContext::Default(); + scan_metrics = ScanMetrics::Make(*metrics_context); + planning_duration.emplace(scan_metrics->total_planning_duration->Start()); + } + TableMetadataCache metadata_cache(metadata_.get()); ICEBERG_ASSIGN_OR_RAISE(auto specs_by_id, metadata_cache.GetPartitionSpecsById()); @@ -559,6 +629,13 @@ Result>> DataTableScan::PlanFiles() co ICEBERG_ASSIGN_OR_RAISE(auto data_manifests, snapshot_cache.DataManifests(io_)); ICEBERG_ASSIGN_OR_RAISE(auto delete_manifests, snapshot_cache.DeleteManifests(io_)); + if (scan_metrics) { + scan_metrics->total_data_manifests->Increment( + static_cast(data_manifests.size())); + scan_metrics->total_delete_manifests->Increment( + static_cast(delete_manifests.size())); + } + ICEBERG_ASSIGN_OR_RAISE( auto manifest_group, ManifestGroup::Make(io_, schema_, specs_by_id, @@ -569,11 +646,19 @@ Result>> DataTableScan::PlanFiles() co .FilterData(filter()) .IgnoreDeleted() .ColumnsToKeepStats(context_.columns_to_keep_stats) - .PlanWith(context_.plan_executor); + .PlanWith(context_.plan_executor) + .WithScanMetrics(scan_metrics); if (context_.ignore_residuals) { manifest_group->IgnoreResiduals(); } - return manifest_group->PlanFiles(); + ICEBERG_ASSIGN_OR_RAISE(auto tasks, manifest_group->PlanFiles()); + + if (planning_duration) { + planning_duration->Stop(); + std::ignore = ReportScan(*snapshot, *scan_metrics); + } + + return tasks; } // Friend function template for IncrementalScan that implements the shared PlanFiles diff --git a/src/iceberg/table_scan.h b/src/iceberg/table_scan.h index 5ee61cde0..bee2b7d1d 100644 --- a/src/iceberg/table_scan.h +++ b/src/iceberg/table_scan.h @@ -228,6 +228,8 @@ struct TableScanContext { std::string branch{}; std::optional min_rows_requested; OptionalExecutor plan_executor; + std::string table_name; + std::shared_ptr metrics_reporter; // Validate the context parameters to see if they have conflicts. [[nodiscard]] Status Validate() const; @@ -245,10 +247,8 @@ template class ICEBERG_TEMPLATE_CLASS_EXPORT TableScanBuilder : public ErrorCollector { public: /// \brief Constructs a TableScanBuilder for the given table. - /// \param metadata Current table metadata. - /// \param io FileIO instance for reading manifests files. - static Result>> Make( - std::shared_ptr metadata, std::shared_ptr io); + /// \param table Table whose metadata, FileIO, name, and reporter are captured. + static Result>> Make(const Table& table); /// \brief Update property that will override the table's behavior /// based on the incoming pair. Unknown properties will be ignored. @@ -381,12 +381,20 @@ class ICEBERG_TEMPLATE_CLASS_EXPORT TableScanBuilder : public ErrorCollector { TableScanBuilder& UseBranch(const std::string& branch) requires IsIncrementalScan; + /// \brief Add a metrics reporter for this scan. + /// + /// May be called multiple times; each call combines with the previous reporter + /// via MetricsReporters::Combine(). + TableScanBuilder& ReportWith(std::shared_ptr reporter); + /// \brief Builds and returns a TableScan instance. /// \return A Result containing the TableScan or an error. Result> Build(); protected: - TableScanBuilder(std::shared_ptr metadata, std::shared_ptr io); + TableScanBuilder(std::shared_ptr metadata, std::shared_ptr io, + std::string table_name, + std::shared_ptr metrics_reporter); // Return the schema bound to the specified snapshot. Result>> ResolveSnapshotSchema(); @@ -455,6 +463,9 @@ class ICEBERG_EXPORT DataTableScan : public TableScan { /// \return A Result containing scan tasks or an error. Result>> PlanFiles() const; + private: + Status ReportScan(const Snapshot& snapshot, const ScanMetrics& scan_metrics) const; + protected: using TableScan::TableScan; }; diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 14313b08f..f1e2a3a78 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -77,6 +77,7 @@ add_iceberg_test(schema_test add_iceberg_test(table_test SOURCES + catalog_util_test.cc location_provider_test.cc metrics_config_test.cc metrics_reporter_test.cc @@ -125,6 +126,7 @@ add_iceberg_test(util_test base64_test.cc bucket_util_test.cc config_test.cc + content_file_util_test.cc data_file_set_test.cc decimal_test.cc endian_test.cc @@ -214,6 +216,7 @@ if(ICEBERG_BUILD_BUNDLE) file_scan_task_test.cc incremental_append_scan_test.cc incremental_changelog_scan_test.cc + scan_planning_metrics_test.cc table_scan_test.cc) add_iceberg_test(table_update_test @@ -298,6 +301,7 @@ if(ICEBERG_BUILD_REST) endpoint_test.cc rest_file_io_test.cc rest_json_serde_test.cc + rest_metrics_reporter_test.cc rest_util_test.cc) if(ICEBERG_SIGV4) diff --git a/src/iceberg/test/catalog_util_test.cc b/src/iceberg/test/catalog_util_test.cc new file mode 100644 index 000000000..a99f75bb7 --- /dev/null +++ b/src/iceberg/test/catalog_util_test.cc @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/catalog_util.h" + +#include + +#include "iceberg/table_identifier.h" + +namespace iceberg { + +class CatalogUtilTest : public ::testing::Test { + protected: + TableIdentifier identifier_ = { + .ns = Namespace{.levels = {"db"}}, + .name = "test_table", + }; +}; + +TEST_F(CatalogUtilTest, EmptyCatalogName) { + EXPECT_EQ(CatalogUtil::FullTableName("", identifier_), "db.test_table"); +} + +TEST_F(CatalogUtilTest, DotJoinedCatalogName) { + EXPECT_EQ(CatalogUtil::FullTableName("my_catalog", identifier_), + "my_catalog.db.test_table"); +} + +TEST_F(CatalogUtilTest, UriCatalogNameWithoutTrailingSlash) { + EXPECT_EQ(CatalogUtil::FullTableName("thrift://localhost:9083", identifier_), + "thrift://localhost:9083/db.test_table"); +} + +TEST_F(CatalogUtilTest, UriCatalogNameWithTrailingSlash) { + EXPECT_EQ(CatalogUtil::FullTableName("hdfs://nameservice/warehouse/", identifier_), + "hdfs://nameservice/warehouse/db.test_table"); +} + +TEST_F(CatalogUtilTest, PathStyleCatalogName) { + EXPECT_EQ(CatalogUtil::FullTableName("/test/catalog", identifier_), + "/test/catalog/db.test_table"); +} + +TEST_F(CatalogUtilTest, MultiLevelNamespace) { + TableIdentifier identifier{ + .ns = Namespace{.levels = {"analytics", "prod"}}, + .name = "events", + }; + EXPECT_EQ(CatalogUtil::FullTableName("catalog", identifier), + "catalog.analytics.prod.events"); +} + +} // namespace iceberg diff --git a/src/iceberg/test/content_file_util_test.cc b/src/iceberg/test/content_file_util_test.cc new file mode 100644 index 000000000..cca0eba8e --- /dev/null +++ b/src/iceberg/test/content_file_util_test.cc @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/util/content_file_util.h" + +#include + +#include "iceberg/file_format.h" +#include "iceberg/manifest/manifest_entry.h" + +namespace iceberg { + +TEST(ContentFileUtilTest, ContentSizeInBytesUsesFileSizeForDataFile) { + DataFile file{ + .content = DataFile::Content::kData, + .file_path = "data.parquet", + .file_format = FileFormatType::kParquet, + .record_count = 10, + .file_size_in_bytes = 100, + }; + + EXPECT_FALSE(ContentFileUtil::IsDV(file)); + EXPECT_EQ(ContentFileUtil::ContentSizeInBytes(file), 100); +} + +TEST(ContentFileUtilTest, ContentSizeInBytesUsesFileSizeForNonDVDeleteFiles) { + DataFile positional_delete{ + .content = DataFile::Content::kPositionDeletes, + .file_path = "position-deletes.parquet", + .file_format = FileFormatType::kParquet, + .record_count = 10, + .file_size_in_bytes = 100, + }; + DataFile equality_delete{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "equality-deletes.parquet", + .file_format = FileFormatType::kParquet, + .record_count = 10, + .file_size_in_bytes = 200, + .equality_ids = {1}, + }; + + EXPECT_FALSE(ContentFileUtil::IsDV(positional_delete)); + EXPECT_EQ(ContentFileUtil::ContentSizeInBytes(positional_delete), 100); + EXPECT_FALSE(ContentFileUtil::IsDV(equality_delete)); + EXPECT_EQ(ContentFileUtil::ContentSizeInBytes(equality_delete), 200); +} + +TEST(ContentFileUtilTest, ContentSizeInBytesUsesContentSizeForDVFile) { + DataFile file{ + .content = DataFile::Content::kPositionDeletes, + .file_path = "deletes.puffin", + .file_format = FileFormatType::kPuffin, + .record_count = 10, + .file_size_in_bytes = 100, + .referenced_data_file = "data.parquet", + .content_offset = 4, + .content_size_in_bytes = 42, + }; + + EXPECT_TRUE(ContentFileUtil::IsDV(file)); + EXPECT_EQ(ContentFileUtil::ContentSizeInBytes(file), 42); +} + +} // namespace iceberg diff --git a/src/iceberg/test/delete_file_index_test.cc b/src/iceberg/test/delete_file_index_test.cc index fea9b6a04..75c82bc39 100644 --- a/src/iceberg/test/delete_file_index_test.cc +++ b/src/iceberg/test/delete_file_index_test.cc @@ -36,11 +36,14 @@ #include "iceberg/manifest/manifest_reader.h" #include "iceberg/manifest/manifest_writer.h" #include "iceberg/metadata_columns.h" +#include "iceberg/metrics/metrics_context.h" +#include "iceberg/metrics/scan_report.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" #include "iceberg/test/matchers.h" #include "iceberg/transform.h" #include "iceberg/type.h" +#include "iceberg/util/partition_value_util.h" namespace iceberg { @@ -189,13 +192,17 @@ class DeleteFileIndexTest : public testing::TestWithParam { Result> BuildIndex( std::vector delete_manifests, - std::optional after_sequence_number = std::nullopt) { + std::optional after_sequence_number = std::nullopt, + std::shared_ptr scan_metrics = nullptr) { ICEBERG_ASSIGN_OR_RAISE(auto builder, DeleteFileIndex::BuilderFor(file_io_, schema_, GetSpecsById(), std::move(delete_manifests))); if (after_sequence_number.has_value()) { builder.AfterSequenceNumber(after_sequence_number.value()); } + if (scan_metrics != nullptr) { + builder.WithScanMetrics(std::move(scan_metrics)); + } return builder.Build(); } @@ -260,6 +267,125 @@ TEST_P(DeleteFileIndexTest, TestMinSequenceNumberFilteringForFiles) { EXPECT_EQ(deletes[0]->file_path, "/path/to/eq-delete-2.parquet"); } +TEST_P(DeleteFileIndexTest, TestMinSequenceNumberFilteringDoesNotCountAsSkipped) { + auto version = GetParam(); + + auto eq_delete_1 = MakeEqualityDeleteFile("/path/to/eq-delete-1.parquet", + PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + auto eq_delete_2 = MakeEqualityDeleteFile("/path/to/eq-delete-2.parquet", + PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + + std::vector entries; + // Dropped by the after_sequence_number filter (seq 4 is not > 4). + entries.push_back( + MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/4, eq_delete_1)); + // Kept (seq 6 > 4). + entries.push_back( + MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/6, eq_delete_2)); + + auto manifest = WriteDeleteManifest(version, /*snapshot_id=*/1000L, std::move(entries), + unpartitioned_spec_); + + auto metrics_context = MetricsContext::Default(); + std::shared_ptr scan_metrics = ScanMetrics::Make(*metrics_context); + ICEBERG_UNWRAP_OR_FAIL( + auto index, BuildIndex({manifest}, /*after_sequence_number=*/4, scan_metrics)); + + ICEBERG_UNWRAP_OR_FAIL(auto deletes, index->ForDataFile(0, *unpartitioned_file_)); + EXPECT_EQ(deletes.size(), 1); + + // Sequence-number filtering does not contribute to Java's skipped-file metric. + EXPECT_EQ(scan_metrics->skipped_delete_files->value(), 0); + EXPECT_EQ(scan_metrics->indexed_delete_files->value(), 1); +} + +TEST_P(DeleteFileIndexTest, TestDeleteManifestWithOnlyDeletedEntriesCountsAsSkipped) { + auto version = GetParam(); + + auto eq_delete = MakeEqualityDeleteFile("/path/to/eq-delete.parquet", + PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + + std::vector entries; + entries.push_back(MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/4, + eq_delete, ManifestStatus::kDeleted)); + + auto manifest = WriteDeleteManifest(version, /*snapshot_id=*/1000L, std::move(entries), + unpartitioned_spec_); + + auto metrics_context = MetricsContext::Default(); + std::shared_ptr scan_metrics = ScanMetrics::Make(*metrics_context); + ICEBERG_UNWRAP_OR_FAIL( + auto index, + BuildIndex({manifest}, /*after_sequence_number=*/std::nullopt, scan_metrics)); + + EXPECT_TRUE(index->empty()); + EXPECT_EQ(scan_metrics->skipped_delete_manifests->value(), 1); + EXPECT_EQ(scan_metrics->scanned_delete_manifests->value(), 0); +} + +// A manifest is scanned even when no manifest evaluator is configured. +TEST_P(DeleteFileIndexTest, TestScannedDeleteManifestCountedWithoutFilter) { + auto version = GetParam(); + + auto eq_delete = MakeEqualityDeleteFile("/path/to/eq-delete.parquet", + PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + + std::vector entries; + entries.push_back( + MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/4, eq_delete)); + + auto manifest = WriteDeleteManifest(version, /*snapshot_id=*/1000L, std::move(entries), + unpartitioned_spec_); + + auto metrics_context = MetricsContext::Default(); + std::shared_ptr scan_metrics = ScanMetrics::Make(*metrics_context); + ICEBERG_UNWRAP_OR_FAIL( + auto index, + BuildIndex({manifest}, /*after_sequence_number=*/std::nullopt, scan_metrics)); + + EXPECT_FALSE(index->empty()); + EXPECT_EQ(scan_metrics->skipped_delete_manifests->value(), 0); + EXPECT_EQ(scan_metrics->scanned_delete_manifests->value(), 1); +} + +TEST_P(DeleteFileIndexTest, TestPartitionSetFilterCountsSkippedDeleteFiles) { + auto version = GetParam(); + + auto partition_a = PartitionValues({Literal::Int(0)}); + auto pos_delete = MakePositionDeleteFile("/path/to/pos-delete.parquet", partition_a, + partitioned_spec_->spec_id()); + + std::vector entries; + entries.push_back( + MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/2, pos_delete)); + + auto manifest = WriteDeleteManifest(version, /*snapshot_id=*/1000L, std::move(entries), + partitioned_spec_); + + // The partition set only contains partition B, so the entry in partition A must be + // rejected at the reader level (an entry-level skip, not a manifest-level one). + auto partition_set = std::make_shared(); + ASSERT_TRUE(partition_set->add(partitioned_spec_->spec_id(), + PartitionValues({Literal::Int(1)}))); + + ICEBERG_UNWRAP_OR_FAIL( + auto builder, + DeleteFileIndex::BuilderFor(file_io_, schema_, GetSpecsById(), {manifest})); + auto metrics_context = MetricsContext::Default(); + std::shared_ptr scan_metrics = ScanMetrics::Make(*metrics_context); + builder.FilterPartitions(partition_set).WithScanMetrics(scan_metrics); + ICEBERG_UNWRAP_OR_FAIL(auto index, builder.Build()); + + ICEBERG_UNWRAP_OR_FAIL(auto deletes, index->ForDataFile(1, *file_a_)); + EXPECT_TRUE(deletes.empty()); + EXPECT_EQ(scan_metrics->skipped_delete_files->value(), 1); + EXPECT_EQ(scan_metrics->skipped_delete_manifests->value(), 0); +} + TEST_P(DeleteFileIndexTest, TestUnpartitionedDeletes) { auto version = GetParam(); @@ -1051,6 +1177,42 @@ TEST_P(DeleteFileIndexTest, TestReferencedDeleteFiles) { "/path/to/global-eq-delete.parquet")); } +TEST_P(DeleteFileIndexTest, TestDeleteFileCountedOnceAcrossMultipleDataFiles) { + auto version = GetParam(); + + auto global_eq_delete = MakeEqualityDeleteFile("/path/to/global-eq-delete.parquet", + PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + + std::vector entries; + entries.push_back( + MakeDeleteEntry(/*snapshot_id=*/1000L, /*sequence_number=*/1, global_eq_delete)); + + auto manifest = WriteDeleteManifest(version, /*snapshot_id=*/1000L, std::move(entries), + unpartitioned_spec_); + + auto metrics_context = MetricsContext::Default(); + std::shared_ptr scan_metrics = ScanMetrics::Make(*metrics_context); + ICEBERG_UNWRAP_OR_FAIL( + auto index, + BuildIndex({manifest}, /*after_sequence_number=*/std::nullopt, scan_metrics)); + + auto other_unpartitioned_file = + MakeDataFile("/path/to/data-other.parquet", PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()); + ICEBERG_UNWRAP_OR_FAIL(auto deletes_for_first, + index->ForDataFile(0, *unpartitioned_file_)); + ICEBERG_UNWRAP_OR_FAIL(auto deletes_for_second, + index->ForDataFile(0, *other_unpartitioned_file)); + EXPECT_EQ(deletes_for_first.size(), 1); + EXPECT_EQ(deletes_for_second.size(), 1); + + // Index metrics count the delete file once; task-level metrics are recorded elsewhere. + EXPECT_EQ(scan_metrics->indexed_delete_files->value(), 1); + EXPECT_EQ(scan_metrics->result_delete_files->value(), 0); + EXPECT_EQ(scan_metrics->equality_delete_files->value(), 1); +} + TEST_P(DeleteFileIndexTest, TestExistingDeleteFiles) { auto version = GetParam(); diff --git a/src/iceberg/test/expression_visitor_test.cc b/src/iceberg/test/expression_visitor_test.cc index 97d70d7a8..a3b2c4cad 100644 --- a/src/iceberg/test/expression_visitor_test.cc +++ b/src/iceberg/test/expression_visitor_test.cc @@ -17,11 +17,20 @@ * under the License. */ +#include +#include +#include +#include +#include +#include +#include + #include #include "iceberg/expression/binder.h" #include "iceberg/expression/expressions.h" #include "iceberg/expression/rewrite_not.h" +#include "iceberg/expression/sanitize_expression.h" #include "iceberg/result.h" #include "iceberg/schema.h" #include "iceberg/test/matchers.h" @@ -506,6 +515,281 @@ TEST_F(RewriteNotTest, ComplexExpression) { EXPECT_EQ(rewritten->op(), Expression::Operation::kOr); } +// gtest's MatchesRegex/ContainsRegex falls back to a minimal "simple regex" engine on +// MSVC (no groups, character classes, repetition, or alternation), unlike the POSIX +// extended regex used on Linux/macOS. std::regex (ECMAScript grammar) behaves +// consistently across all three standard libraries. +MATCHER_P(MatchesStdRegex, pattern, "") { + return std::regex_match(arg, std::regex(pattern)); +} + +class SanitizeExpressionTest : public ExpressionVisitorTest {}; + +TEST_F(SanitizeExpressionTest, Constants) { + auto true_expr = Expressions::AlwaysTrue(); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_true, SanitizeExpression::Sanitize(true_expr)); + EXPECT_TRUE(sanitized_true->Equals(*True::Instance())); + + auto false_expr = Expressions::AlwaysFalse(); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_false, SanitizeExpression::Sanitize(false_expr)); + EXPECT_TRUE(sanitized_false->Equals(*False::Instance())); +} + +TEST_F(SanitizeExpressionTest, BoundLiteralPredicateHidesValue) { + auto unbound_pred = Expressions::Equal("name", Literal::String("alice@example.com")); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred, Bind(unbound_pred)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(bound_pred)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kEq); + EXPECT_TRUE(sanitized->is_unbound_predicate()); + EXPECT_THAT(sanitized->ToString(), + MatchesStdRegex(R"re(ref\(name="name"\) == "\(hash-[0-9a-f]{8}\)")re")); + EXPECT_THAT(sanitized->ToString(), ::testing::Not(::testing::HasSubstr("alice"))); +} + +TEST_F(SanitizeExpressionTest, UnboundLiteralPredicateHidesValue) { + auto unbound_pred = Expressions::GreaterThan("age", Literal::Int(42)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kGt); + EXPECT_EQ(sanitized->ToString(), "ref(name=\"age\") > \"(2-digit-int)\""); + EXPECT_THAT(sanitized->ToString(), ::testing::Not(::testing::HasSubstr("42"))); +} + +TEST_F(SanitizeExpressionTest, StringHashMatchesJava) { + auto unbound_pred = Expressions::StartsWith("name", "aaa"); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), "ref(name=\"name\") startsWith \"(hash-34d05fb7)\""); +} + +TEST_F(SanitizeExpressionTest, UnboundUnaryPredicatePreservesOriginal) { + const std::vector> predicates = { + Expressions::IsNull("value"), Expressions::NotNull("value"), + Expressions::IsNaN("value"), Expressions::NotNaN("value")}; + + for (const auto& predicate : predicates) { + SCOPED_TRACE(predicate->ToString()); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(predicate)); + EXPECT_TRUE(sanitized->Equals(*predicate)); + } +} + +TEST_F(SanitizeExpressionTest, UnaryPredicateNeedsNoLiteral) { + auto unbound_pred = Expressions::IsNull("salary"); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred, Bind(unbound_pred)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(bound_pred)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kIsNull); + EXPECT_EQ(sanitized->ToString(), "is_null(ref(name=\"salary\"))"); +} + +TEST_F(SanitizeExpressionTest, SetPredicateSanitizesEachElement) { + auto unbound_pred = Expressions::In( + "name", + {Literal::String("alice"), Literal::String("bob"), Literal::String("carol")}); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred, Bind(unbound_pred)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(bound_pred)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kIn); + EXPECT_THAT( + sanitized->ToString(), + MatchesStdRegex( + R"re(ref\(name="name"\) in \["\(hash-[0-9a-f]{8}\)"(, "\(hash-[0-9a-f]{8}\)"){2}\])re")); + for (const auto* leaked : {"alice", "bob", "carol"}) { + EXPECT_THAT(sanitized->ToString(), ::testing::Not(::testing::HasSubstr(leaked))); + } +} + +TEST_F(SanitizeExpressionTest, LongSetPredicatePreservesSanitizedDuplicates) { + std::vector values; + for (int32_t value = 95; value < 105; ++value) { + values.push_back(Literal::Int(value)); + } + auto unbound_pred = Expressions::In("age", std::move(values)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), + "ref(name=\"age\") in [\"(2-digit-int)\", \"(2-digit-int)\", " + "\"(2-digit-int)\", \"(2-digit-int)\", \"(2-digit-int)\", " + "\"(3-digit-int)\", \"(3-digit-int)\", \"(3-digit-int)\", " + "\"(3-digit-int)\", \"(3-digit-int)\"]"); +} + +TEST_F(SanitizeExpressionTest, FractionalFloatLiteralDigitCount) { + auto unbound_pred = Expressions::LessThan("salary", Literal::Double(0.05)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), "ref(name=\"salary\") < \"(0-digit-float)\""); +} + +TEST_F(SanitizeExpressionTest, NonFiniteFloatUsesGenericPlaceholder) { + auto unbound_pred = Expressions::In( + "salary", {Literal::Double(std::numeric_limits::quiet_NaN()), + Literal::Double(std::numeric_limits::infinity()), + Literal::Double(-std::numeric_limits::infinity())}); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), + "ref(name=\"salary\") in [\"(float)\", \"(float)\", \"(float)\"]"); +} + +TEST_F(SanitizeExpressionTest, TemporalStringsMatchJavaBuckets) { + const std::vector> cases = { + {"2022-04-29", "(date)"}, + {"12:34:56.123456", "(time)"}, + {"2022-04-29T23:49:51.123456", "(timestamp)"}, + {"2022-04-29T23:49:51.123456789", "(timestamp)"}, + {"2022-04-29T23:49:51.123456-07:00", "(timestamp)"}, + {"2022-04-29T23:49:51.123456789Z", "(timestamp)"}, + }; + + for (const auto& [value, expected] : cases) { + auto unbound_pred = Expressions::Equal("value", Literal::String(value)); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), + std::format("ref(name=\"value\") == \"{}\"", expected)); + } +} + +TEST_F(SanitizeExpressionTest, InvalidTemporalStringFallsBackToHash) { + auto unbound_pred = Expressions::Equal("value", Literal::String("2022-20-29")); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_THAT(sanitized->ToString(), + MatchesStdRegex(R"re(ref\(name="value"\) == "\(hash-[0-9a-f]{8}\)")re")); +} + +TEST_F(SanitizeExpressionTest, BinaryAndFixedHashCanonicalContents) { + auto binary = Expressions::Equal("value", Literal::Binary({0x01, 0x02})); + auto other_binary = Expressions::Equal("value", Literal::Binary({0x01, 0x03})); + auto fixed = Expressions::Equal("value", Literal::Fixed({0x01, 0x02})); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_binary, SanitizeExpression::Sanitize(binary)); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_other_binary, + SanitizeExpression::Sanitize(other_binary)); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_fixed, SanitizeExpression::Sanitize(fixed)); + EXPECT_NE(sanitized_binary->ToString(), sanitized_other_binary->ToString()); + EXPECT_EQ(sanitized_binary->ToString(), sanitized_fixed->ToString()); + EXPECT_THAT(sanitized_binary->ToString(), + MatchesStdRegex(R"re(ref\(name="value"\) == "\(hash-[0-9a-f]{8}\)")re")); + EXPECT_THAT(sanitized_binary->ToString(), ::testing::Not(::testing::HasSubstr("0102"))); +} + +TEST_F(SanitizeExpressionTest, TimestampLiteralBucketsByHoursAgo) { + auto fifty_hours_ago = std::chrono::system_clock::now() - std::chrono::hours(50); + int64_t micros = std::chrono::duration_cast( + fifty_hours_ago.time_since_epoch()) + .count(); + auto unbound_pred = Expressions::LessThan("ts", Literal::Timestamp(micros)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_THAT( + sanitized->ToString(), + MatchesStdRegex(R"re(ref\(name="ts"\) < "\(timestamp-(49|50)-hours-ago\)")re")); +} + +TEST_F(SanitizeExpressionTest, TimestampLiteralBucketsByDaysAgo) { + auto ten_days_ago = std::chrono::system_clock::now() - std::chrono::hours(240); + int64_t micros = std::chrono::duration_cast( + ten_days_ago.time_since_epoch()) + .count(); + auto unbound_pred = Expressions::LessThan("ts", Literal::Timestamp(micros)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_THAT( + sanitized->ToString(), + MatchesStdRegex(R"re(ref\(name="ts"\) < "\(timestamp-(9|10)-days-ago\)")re")); +} + +TEST_F(SanitizeExpressionTest, TimestampLiteralNearInt64LimitsDoesNotWrap) { + // Regression test: the now-vs-literal distance must stay unsigned end-to-end. + // Casting an unsigned distance greater than INT64_MAX back to int64_t wraps to a + // negative number, which would previously misclassify these as "about-now" instead + // of the generic "(timestamp)" bucket. + auto min_pred = Expressions::LessThan( + "ts", Literal::Timestamp(std::numeric_limits::min())); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_min, SanitizeExpression::Sanitize(min_pred)); + EXPECT_EQ(sanitized_min->ToString(), "ref(name=\"ts\") < \"(timestamp)\""); + + auto max_pred = Expressions::LessThan( + "ts", Literal::Timestamp(std::numeric_limits::max())); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_max, SanitizeExpression::Sanitize(max_pred)); + EXPECT_EQ(sanitized_max->ToString(), "ref(name=\"ts\") < \"(timestamp)\""); +} + +TEST_F(SanitizeExpressionTest, DateLiteralNearInt32LimitsDoesNotWrap) { + auto min_pred = + Expressions::LessThan("d", Literal::Date(std::numeric_limits::min())); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_min, SanitizeExpression::Sanitize(min_pred)); + EXPECT_EQ(sanitized_min->ToString(), "ref(name=\"d\") < \"(date)\""); + + auto max_pred = + Expressions::LessThan("d", Literal::Date(std::numeric_limits::max())); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_max, SanitizeExpression::Sanitize(max_pred)); + EXPECT_EQ(sanitized_max->ToString(), "ref(name=\"d\") < \"(date)\""); +} + +TEST_F(SanitizeExpressionTest, UnboundPredicateOverTransformKeepsTransform) { + auto bucket_term = Expressions::Bucket("id", 16); + auto unbound_pred = Expressions::Equal(bucket_term, Literal::Int(5)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(unbound_pred)); + EXPECT_EQ(sanitized->ToString(), "bucket[16](ref(name=\"id\")) == \"(1-digit-int)\""); + auto sanitized_predicate = + std::dynamic_pointer_cast>(sanitized); + ASSERT_NE(sanitized_predicate, nullptr); + EXPECT_EQ(sanitized_predicate->term(), bucket_term); +} + +TEST_F(SanitizeExpressionTest, BoundPredicateOverTransformKeepsTransform) { + // Regression test: Java's unbind(BoundTerm) rebuilds a BoundTransform term as a + // transform term, not a plain reference; BoundPredicate::reference() alone would + // silently discard the transform. + auto bucket_term = Expressions::Bucket("id", 16); + auto unbound_pred = Expressions::Equal(bucket_term, Literal::Int(5)); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred, Bind(unbound_pred)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(bound_pred)); + EXPECT_TRUE(sanitized->is_unbound_predicate()); + EXPECT_EQ(sanitized->ToString(), "bucket[16](ref(name=\"id\")) == \"(1-digit-int)\""); +} + +TEST_F(SanitizeExpressionTest, PreservesAndOrNotStructure) { + auto pred1 = Expressions::Equal("name", Literal::String("alice@example.com")); + auto pred2 = Expressions::GreaterThan("age", Literal::Int(25)); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred1, Bind(pred1)); + ICEBERG_UNWRAP_OR_FAIL(auto bound_pred2, Bind(pred2)); + auto not_pred2 = Expressions::Not(bound_pred2); + auto and_expr = Expressions::And(bound_pred1, not_pred2); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, SanitizeExpression::Sanitize(and_expr)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kAnd); + EXPECT_THAT(sanitized->ToString(), ::testing::Not(::testing::HasSubstr("alice"))); + EXPECT_THAT(sanitized->ToString(), ::testing::Not(::testing::HasSubstr("25"))); +} + +TEST_F(SanitizeExpressionTest, BindWithFallbackMatchesUnboundOnSuccess) { + auto unbound_pred = Expressions::GreaterThan("age", Literal::Int(42)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_unbound, + SanitizeExpression::Sanitize(unbound_pred)); + ICEBERG_UNWRAP_OR_FAIL(auto sanitized_bound, + SanitizeExpression::Sanitize(*schema_, unbound_pred, + /*case_sensitive=*/true)); + EXPECT_EQ(sanitized_bound->ToString(), sanitized_unbound->ToString()); +} + +TEST_F(SanitizeExpressionTest, BindWithFallbackFallsBackOnUnknownColumn) { + auto unbound_pred = Expressions::GreaterThan("not_a_column", Literal::Int(42)); + + ICEBERG_UNWRAP_OR_FAIL(auto sanitized, + SanitizeExpression::Sanitize(*schema_, unbound_pred, + /*case_sensitive=*/true)); + EXPECT_EQ(sanitized->op(), Expression::Operation::kGt); + EXPECT_EQ(sanitized->ToString(), "ref(name=\"not_a_column\") > \"(2-digit-int)\""); +} + class ReferenceVisitorTest : public ExpressionVisitorTest {}; TEST_F(ReferenceVisitorTest, Constants) { diff --git a/src/iceberg/test/fast_append_test.cc b/src/iceberg/test/fast_append_test.cc index d9e9a7eb5..f88d2e011 100644 --- a/src/iceberg/test/fast_append_test.cc +++ b/src/iceberg/test/fast_append_test.cc @@ -21,10 +21,12 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -35,6 +37,9 @@ #include "iceberg/manifest/manifest_entry.h" #include "iceberg/manifest/manifest_reader.h" #include "iceberg/manifest/manifest_writer.h" +#include "iceberg/metrics/commit_report.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/metrics/metrics_reporters.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" #include "iceberg/snapshot.h" @@ -42,9 +47,12 @@ #include "iceberg/table_properties.h" #include "iceberg/test/executor.h" #include "iceberg/test/matchers.h" +#include "iceberg/test/mock_catalog.h" #include "iceberg/test/update_test_base.h" #include "iceberg/transaction.h" -#include "iceberg/update/update_properties.h" // IWYU pragma: keep +#include "iceberg/update/merge_append.h" +#include "iceberg/update/update_properties.h" +#include "iceberg/util/uuid.h" namespace iceberg { @@ -399,4 +407,335 @@ TEST_F(SnapshotUpdateTest, ConcurrentManifestPaths) { } } +namespace { + +class CapturingReporter final : public MetricsReporter { + public: + Status Report(const MetricsReport& report) override { + reports_.push_back(report); + return {}; + } + const std::vector& reports() const { return reports_; } + void clear() { reports_.clear(); } + + private: + std::vector reports_; +}; + +void RegisterCapturingReporter() { + static std::once_flag flag; + std::call_once(flag, [] { + (void)MetricsReporters::Register( + "fast.append.test.reporter", + [](const auto&) -> Result> { + return std::make_unique(); + }); + }); +} + +} // namespace + +class FastAppendMetricsTest : public ::testing::Test { + protected: + static void SetUpTestSuite() { + avro::RegisterAll(); + RegisterCapturingReporter(); + } + + void SetUp() override { + table_ident_ = TableIdentifier{.name = "metrics_test_table"}; + table_location_ = "/warehouse/metrics_test_table"; + + file_io_ = arrow::ArrowFileSystemFileIO::MakeMockFileIO(); + ICEBERG_UNWRAP_OR_FAIL( + catalog_, InMemoryCatalog::Make("metrics_test_catalog", file_io_, "/warehouse/", + {{std::string(kMetricsReporterImpl), + "fast.append.test.reporter"}})); + + auto arrow_fs = std::dynamic_pointer_cast<::arrow::fs::internal::MockFileSystem>( + static_cast(*file_io_).fs()); + ASSERT_TRUE(arrow_fs != nullptr); + ASSERT_TRUE(arrow_fs->CreateDir(table_location_ + "/metadata").ok()); + + auto metadata_location = std::format("{}/metadata/00001-{}.metadata.json", + table_location_, Uuid::GenerateV7().ToString()); + ICEBERG_UNWRAP_OR_FAIL( + auto metadata, ReadTableMetadataFromResource("TableMetadataV2ValidMinimal.json")); + metadata->location = table_location_; + ASSERT_THAT(TableMetadataUtil::Write(*file_io_, metadata_location, *metadata), + IsOk()); + ICEBERG_UNWRAP_OR_FAIL(table_, + catalog_->RegisterTable(table_ident_, metadata_location)); + + reporter_ = std::dynamic_pointer_cast(table_->reporter()); + ASSERT_NE(reporter_, nullptr); + + ICEBERG_UNWRAP_OR_FAIL(spec_, table_->spec()); + ICEBERG_UNWRAP_OR_FAIL(schema_, table_->schema()); + } + + std::shared_ptr MakeDataFile(const std::string& path, + int64_t partition_value = 1024) { + auto data_file = std::make_shared(); + data_file->content = DataFile::Content::kData; + data_file->file_path = table_location_ + path; + data_file->file_format = FileFormatType::kParquet; + data_file->partition = + PartitionValues(std::vector{Literal::Long(partition_value)}); + data_file->file_size_in_bytes = 1024; + data_file->record_count = 100; + data_file->partition_spec_id = spec_->spec_id(); + return data_file; + } + + TableIdentifier table_ident_; + std::string table_location_; + std::shared_ptr file_io_; + std::shared_ptr catalog_; + std::shared_ptr
table_; + std::shared_ptr spec_; + std::shared_ptr schema_; + std::shared_ptr reporter_; +}; + +TEST_F(FastAppendMetricsTest, CommitReportFiredAfterFastAppend) { + std::shared_ptr fast_append; + ICEBERG_UNWRAP_OR_FAIL(fast_append, table_->NewFastAppend()); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(fast_append->Commit(), IsOk()); + + ASSERT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + + const auto& reports = reporter_->reports(); + ASSERT_EQ(reports.size(), 1u); + ASSERT_TRUE(std::holds_alternative(reports[0])); + + const auto& report = std::get(reports[0]); + EXPECT_EQ(report.table_name, table_->full_name()); + EXPECT_EQ(report.table_name, "metrics_test_catalog." + table_ident_.ToString()); + EXPECT_EQ(report.snapshot_id, snapshot->snapshot_id); + EXPECT_EQ(report.sequence_number, snapshot->sequence_number); + EXPECT_EQ(report.operation, "append"); + const auto& metrics = report.commit_metrics; + ASSERT_TRUE(metrics.attempts.has_value()); + EXPECT_EQ(metrics.attempts->value, 1); + ASSERT_TRUE(metrics.added_data_files.has_value()); + EXPECT_EQ(metrics.added_data_files->value, 1); + ASSERT_TRUE(metrics.total_data_files.has_value()); + EXPECT_EQ(metrics.total_data_files->value, 1); + ASSERT_TRUE(metrics.added_records.has_value()); + EXPECT_EQ(metrics.added_records->value, 100); + ASSERT_TRUE(metrics.total_records.has_value()); + EXPECT_EQ(metrics.total_records->value, 100); + ASSERT_TRUE(metrics.added_files_size_bytes.has_value()); + EXPECT_EQ(metrics.added_files_size_bytes->value, 1024); + ASSERT_TRUE(metrics.total_files_size_bytes.has_value()); + EXPECT_EQ(metrics.total_files_size_bytes->value, 1024); + ASSERT_TRUE(metrics.created_manifest_count.has_value()); + EXPECT_EQ(metrics.created_manifest_count->value, 1); +} + +TEST_F(FastAppendMetricsTest, ReportWithOverridesTableReporter) { + auto override_reporter = std::make_shared(); + + std::shared_ptr fast_append; + ICEBERG_UNWRAP_OR_FAIL(fast_append, table_->NewFastAppend()); + fast_append->ReportWith(override_reporter); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(fast_append->Commit(), IsOk()); + + ASSERT_EQ(override_reporter->reports().size(), 1u); + EXPECT_TRUE(std::holds_alternative(override_reporter->reports()[0])); + EXPECT_TRUE(reporter_->reports().empty()); +} + +TEST_F(FastAppendMetricsTest, CapturesTableReporterWhenUpdateIsCreated) { + auto replacement_reporter = std::make_shared(); + auto mock_catalog = std::make_shared<::testing::NiceMock>(); + ON_CALL(*mock_catalog, UpdateTable(::testing::_, ::testing::_, ::testing::_)) + .WillByDefault([this, &mock_catalog, &replacement_reporter]( + const TableIdentifier&, + const std::vector>&, + const std::vector>&) + -> Result> { + return Table::Make(table_->name(), table_->metadata(), + std::string(table_->metadata_file_location()), table_->io(), + mock_catalog, table_->full_name(), replacement_reporter); + }); + + ICEBERG_UNWRAP_OR_FAIL( + auto mock_table, + Table::Make(table_->name(), table_->metadata(), + std::string(table_->metadata_file_location()), table_->io(), + mock_catalog, table_->full_name(), reporter_)); + ICEBERG_UNWRAP_OR_FAIL(auto fast_append, mock_table->NewFastAppend()); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + + ASSERT_THAT(fast_append->Commit(), IsOk()); + ASSERT_EQ(reporter_->reports().size(), 1u); + EXPECT_TRUE(replacement_reporter->reports().empty()); +} + +// An existing snapshot must not be reused as the report for a non-snapshot update. +TEST_F(FastAppendMetricsTest, PropertyOnlyCommitOnTableWithSnapshotDoesNotReport) { + std::shared_ptr fast_append; + ICEBERG_UNWRAP_OR_FAIL(fast_append, table_->NewFastAppend()); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(fast_append->Commit(), IsOk()); + ASSERT_EQ(reporter_->reports().size(), 1u); + reporter_->clear(); + + ASSERT_THAT(table_->Refresh(), IsOk()); + std::shared_ptr update_props; + ICEBERG_UNWRAP_OR_FAIL(update_props, table_->NewUpdateProperties()); + update_props->Set("test-key", "test-value"); + ASSERT_THAT(update_props->Commit(), IsOk()); + + EXPECT_TRUE(reporter_->reports().empty()); +} + +// StageOnly() adds a snapshot to table metadata without making it current. The +// CommitReport must still be fired for the staged snapshot itself. +TEST_F(FastAppendMetricsTest, CommitReportFiredForStageOnlyCommit) { + std::shared_ptr fast_append; + ICEBERG_UNWRAP_OR_FAIL(fast_append, table_->NewFastAppend()); + fast_append->StageOnly(); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(fast_append->Commit(), IsOk()); + + ASSERT_THAT(table_->Refresh(), IsOk()); + + // The staged snapshot never became current. + EXPECT_EQ(table_->metadata()->current_snapshot_id, kInvalidSnapshotId); + + const auto& reports = reporter_->reports(); + ASSERT_EQ(reports.size(), 1u); + ASSERT_TRUE(std::holds_alternative(reports[0])); + + const auto& report = std::get(reports[0]); + EXPECT_NE(report.snapshot_id, kInvalidSnapshotId); + EXPECT_TRUE(table_->metadata()->SnapshotById(report.snapshot_id).has_value()); +} + +TEST_F(FastAppendMetricsTest, ReporterOverrideAppliesOnlyToItsOwnUpdate) { + auto override_reporter = std::make_shared(); + + ICEBERG_UNWRAP_OR_FAIL(auto txn, table_->NewTransaction()); + + ICEBERG_UNWRAP_OR_FAIL(auto first_append, txn->NewFastAppend()); + first_append->ReportWith(override_reporter); + first_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(first_append->Commit(), IsOk()); + EXPECT_TRUE(override_reporter->reports().empty()); + EXPECT_TRUE(reporter_->reports().empty()); + + ICEBERG_UNWRAP_OR_FAIL(auto second_append, txn->NewFastAppend()); + second_append->AppendFile(MakeDataFile("/data/file_b.parquet", 2048)); + ASSERT_THAT(second_append->Commit(), IsOk()); + EXPECT_TRUE(override_reporter->reports().empty()); + EXPECT_TRUE(reporter_->reports().empty()); + + ASSERT_THAT(txn->Commit(), IsOk()); + ASSERT_THAT(table_->Refresh(), IsOk()); + + ASSERT_EQ(override_reporter->reports().size(), 1u); + ASSERT_TRUE(std::holds_alternative(override_reporter->reports()[0])); + ASSERT_EQ(reporter_->reports().size(), 1u); + ASSERT_TRUE(std::holds_alternative(reporter_->reports()[0])); + + const auto& first_report = std::get(override_reporter->reports()[0]); + const auto& second_report = std::get(reporter_->reports()[0]); + + ICEBERG_UNWRAP_OR_FAIL(auto current_snapshot, table_->current_snapshot()); + EXPECT_EQ(second_report.snapshot_id, current_snapshot->snapshot_id); + ASSERT_TRUE(current_snapshot->parent_snapshot_id.has_value()); + EXPECT_EQ(first_report.snapshot_id, current_snapshot->parent_snapshot_id.value()); + + EXPECT_EQ(second_report.table_name, table_->full_name()); + + ASSERT_TRUE(first_report.commit_metrics.attempts.has_value()); + EXPECT_EQ(first_report.commit_metrics.attempts->value, 1); + ASSERT_TRUE(second_report.commit_metrics.attempts.has_value()); + EXPECT_EQ(second_report.commit_metrics.attempts->value, 1); +} + +TEST_F(FastAppendMetricsTest, TransactionRetryReportsOnceAfterSuccess) { + auto mock_catalog = std::make_shared<::testing::NiceMock>(); + const std::string refreshed_metadata_location = + table_location_ + "/metadata/refreshed.metadata.json"; + + ON_CALL(*mock_catalog, LoadTable(::testing::_)) + .WillByDefault([this, &mock_catalog, &refreshed_metadata_location]( + const TableIdentifier&) -> Result> { + ICEBERG_ASSIGN_OR_RAISE( + auto metadata, + TableMetadataUtil::Read(*table_->io(), + std::string(table_->metadata_file_location()))); + return Table::Make(table_->name(), std::move(metadata), + refreshed_metadata_location, table_->io(), mock_catalog, + table_->full_name(), reporter_); + }); + + int update_call_count = 0; + ON_CALL(*mock_catalog, UpdateTable(::testing::_, ::testing::_, ::testing::_)) + .WillByDefault([this, &mock_catalog, &update_call_count]( + const TableIdentifier&, + const std::vector>&, + const std::vector>& updates) + -> Result> { + ++update_call_count; + EXPECT_TRUE(reporter_->reports().empty()); + if (update_call_count == 1) { + return CommitFailed("conflict on first attempt"); + } + + EXPECT_FALSE(updates.empty()); + return Table::Make(table_->name(), table_->metadata(), + std::string(table_->metadata_file_location()), table_->io(), + mock_catalog, table_->full_name(), reporter_); + }); + + ICEBERG_UNWRAP_OR_FAIL( + auto mock_table, + Table::Make(table_->name(), table_->metadata(), + std::string(table_->metadata_file_location()), table_->io(), + mock_catalog, table_->full_name(), reporter_)); + ICEBERG_UNWRAP_OR_FAIL(auto txn, mock_table->NewTransaction()); + ICEBERG_UNWRAP_OR_FAIL(auto fast_append, txn->NewFastAppend()); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + ASSERT_THAT(fast_append->Commit(), IsOk()); + ASSERT_TRUE(reporter_->reports().empty()); + + ASSERT_THAT(txn->Commit(), IsOk()); + EXPECT_EQ(update_call_count, 2); + ASSERT_EQ(reporter_->reports().size(), 1u); + ASSERT_TRUE(std::holds_alternative(reporter_->reports()[0])); + const auto& report = std::get(reporter_->reports()[0]); + ASSERT_TRUE(report.commit_metrics.attempts.has_value()); + EXPECT_EQ(report.commit_metrics.attempts->value, 2); +} + +TEST_F(FastAppendMetricsTest, CommitStateUnknownDoesNotReport) { + auto mock_catalog = std::make_shared<::testing::NiceMock>(); + ON_CALL(*mock_catalog, UpdateTable(::testing::_, ::testing::_, ::testing::_)) + .WillByDefault([](const TableIdentifier&, + const std::vector>&, + const std::vector>&) + -> Result> { + return CommitStateUnknown("unknown commit state"); + }); + + ICEBERG_UNWRAP_OR_FAIL( + auto mock_table, + Table::Make(table_->name(), table_->metadata(), + std::string(table_->metadata_file_location()), table_->io(), + mock_catalog, table_->full_name(), reporter_)); + ICEBERG_UNWRAP_OR_FAIL(auto fast_append, mock_table->NewFastAppend()); + fast_append->AppendFile(MakeDataFile("/data/file_a.parquet")); + + EXPECT_THAT(fast_append->Commit(), IsError(ErrorKind::kCommitStateUnknown)); + EXPECT_TRUE(reporter_->reports().empty()); +} + } // namespace iceberg diff --git a/src/iceberg/test/in_memory_catalog_test.cc b/src/iceberg/test/in_memory_catalog_test.cc index af6921f1e..b2c88f571 100644 --- a/src/iceberg/test/in_memory_catalog_test.cc +++ b/src/iceberg/test/in_memory_catalog_test.cc @@ -51,8 +51,9 @@ class InMemoryCatalogTest : public ::testing::Test { void SetUp() override { file_io_ = arrow::ArrowFileSystemFileIO::MakeLocalFileIO(); std::unordered_map properties = {{"prop1", "val1"}}; - catalog_ = std::make_shared("test_catalog", file_io_, - "/tmp/warehouse/", properties); + ICEBERG_UNWRAP_OR_FAIL( + catalog_, + InMemoryCatalog::Make("test_catalog", file_io_, "/tmp/warehouse/", properties)); } void TearDown() override { @@ -91,6 +92,14 @@ class InMemoryCatalogTest : public ::testing::Test { std::vector created_temp_paths_; }; +TEST_F(InMemoryCatalogTest, InvalidMetricsReporterImplReturnsError) { + std::unordered_map properties = { + {"metrics-reporter-impl", "this-reporter-type-does-not-exist"}}; + EXPECT_THAT( + InMemoryCatalog::Make("test_catalog", file_io_, "/tmp/warehouse/", properties), + IsError(ErrorKind::kInvalidArgument)); +} + TEST_F(InMemoryCatalogTest, CatalogName) { EXPECT_EQ(catalog_->name(), "test_catalog"); auto tablesRs = catalog_->ListTables(Namespace{{}}); @@ -123,6 +132,7 @@ TEST_F(InMemoryCatalogTest, CreateTable) { auto table = catalog_->CreateTable(table_ident, schema, spec, sort_order, metadata_location, {{"property1", "value1"}}); EXPECT_THAT(table, IsOk()); + EXPECT_EQ(table.value()->full_name(), "test_catalog.t1"); // Create table already exists auto table2 = catalog_->CreateTable(table_ident, schema, spec, sort_order, @@ -144,6 +154,7 @@ TEST_F(InMemoryCatalogTest, RegisterTable) { auto table = catalog_->RegisterTable(tableIdent, metadata_location); EXPECT_THAT(table, IsOk()); ASSERT_EQ(table.value()->name().name, "t1"); + EXPECT_EQ(table.value()->full_name(), "test_catalog.t1"); ASSERT_EQ(table.value()->location(), "s3://bucket/test/location"); } @@ -268,6 +279,7 @@ TEST_F(InMemoryCatalogTest, StageCreateTable) { auto staged_table, catalog_->StageCreateTable(table_ident, schema, spec, sort_order, GenerateTestTableLocation(table_ident.name), {})); + EXPECT_EQ(staged_table->table()->full_name(), "test_catalog.t1"); // Perform the update ICEBERG_UNWRAP_OR_FAIL(auto update_properties, staged_table->NewUpdateProperties()); @@ -276,6 +288,7 @@ TEST_F(InMemoryCatalogTest, StageCreateTable) { EXPECT_THAT(res1, IsOk()); auto created_table = res1.value(); EXPECT_EQ("t1", created_table->name().name); + EXPECT_EQ(created_table->full_name(), "test_catalog.t1"); EXPECT_EQ("value1", created_table->metadata()->properties.Get( TableProperties::Entry("property1", ""))); diff --git a/src/iceberg/test/incremental_append_scan_test.cc b/src/iceberg/test/incremental_append_scan_test.cc index 559aedd81..044942ad4 100644 --- a/src/iceberg/test/incremental_append_scan_test.cc +++ b/src/iceberg/test/incremental_append_scan_test.cc @@ -50,7 +50,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusive) { // Test: from_snapshot_inclusive(snapshot_a) should return 3 files (A, B, C) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/true); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -64,7 +64,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusive) { // files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/true).ToSnapshot(3000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -74,8 +74,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusive) { TEST_P(IncrementalAppendScanTest, FromSnapshotInclusiveWithNonExistingRef) { auto metadata = MakeTableMetadata({}, -1L); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->FromSnapshot("non_existing_ref", /*inclusive=*/true); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -104,7 +103,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusiveWithTag) { // Test: from_snapshot_inclusive(t1) should return 5 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/true); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -114,7 +113,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusiveWithTag) { // Test: from_snapshot_inclusive(t1).to_snapshot(t2) should return 3 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/true).ToSnapshot("t2"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -138,7 +137,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusiveWithBranchShouldFail) { // Test: from_snapshot_inclusive(branch_name) should fail { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("b1", /*inclusive=*/true); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -148,7 +147,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotInclusiveWithBranchShouldFail) { // Test: to_snapshot(branch_name) should fail { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/true).ToSnapshot("b1"); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -189,7 +188,7 @@ TEST_P(IncrementalAppendScanTest, UseBranch) { // Test: from_snapshot_inclusive(t1) on main should return 5 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/true); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -199,7 +198,7 @@ TEST_P(IncrementalAppendScanTest, UseBranch) { // Test: from_snapshot_inclusive(t1).use_branch(b1) should return 3 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/true).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -209,7 +208,7 @@ TEST_P(IncrementalAppendScanTest, UseBranch) { // Test: to_snapshot(snapshot_branch_b).use_branch(b1) should return 2 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(4000L).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -219,7 +218,7 @@ TEST_P(IncrementalAppendScanTest, UseBranch) { // Test: to_snapshot(snapshot_branch_c).use_branch(b1) should return 3 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(5000L).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -229,7 +228,7 @@ TEST_P(IncrementalAppendScanTest, UseBranch) { // Test: from_snapshot_exclusive(t1).to_snapshot(snapshot_branch_b).use_branch(b1) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/false).ToSnapshot(4000L).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -251,8 +250,7 @@ TEST_P(IncrementalAppendScanTest, UseBranchWithTagShouldFail) { SnapshotRef{.snapshot_id = 1000L, .retention = SnapshotRef::Tag{}})}}); // Test: use_branch(tag_name) should fail - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/true).UseBranch("t1"); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -279,7 +277,7 @@ TEST_P(IncrementalAppendScanTest, UseBranchWithInvalidSnapshotShouldFail) { // Test: to_snapshot(snapshot_main_b).use_branch(b1) should fail { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(2000L).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); EXPECT_THAT( @@ -293,7 +291,7 @@ TEST_P(IncrementalAppendScanTest, UseBranchWithInvalidSnapshotShouldFail) { // Test: from_snapshot_inclusive(snapshot_main_b).use_branch(b1) should fail { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(2000L, /*inclusive=*/true).UseBranch("b1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); EXPECT_THAT( @@ -306,8 +304,7 @@ TEST_P(IncrementalAppendScanTest, UseBranchWithInvalidSnapshotShouldFail) { TEST_P(IncrementalAppendScanTest, UseBranchWithNonExistingRef) { auto metadata = MakeTableMetadata({}, -1L); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->UseBranch("non_existing_ref"); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -332,7 +329,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusive) { // Test: from_snapshot_exclusive(snapshot_a) should return 2 files (B, C) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/false); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -346,7 +343,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusive) { // file (B) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/false).ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -357,8 +354,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusive) { TEST_P(IncrementalAppendScanTest, FromSnapshotExclusiveWithNonExistingRef) { auto metadata = MakeTableMetadata({}, -1L); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->FromSnapshot("nonExistingRef", /*inclusive=*/false); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -387,7 +383,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusiveWithTag) { // Test: from_snapshot_exclusive(t1) should return 4 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/false); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -397,7 +393,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusiveWithTag) { // Test: from_snapshot_exclusive(t1).to_snapshot(t2) should return 2 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot("t1", /*inclusive=*/false).ToSnapshot("t2"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -418,8 +414,7 @@ TEST_P(IncrementalAppendScanTest, FromSnapshotExclusiveWithBranchShouldFail) { {"b1", std::make_shared(SnapshotRef{ .snapshot_id = 1000L, .retention = SnapshotRef::Branch{}})}}); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->FromSnapshot("b1", /*inclusive=*/false); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), HasErrorMessage("Ref b1 is not a tag"))); @@ -443,7 +438,7 @@ TEST_P(IncrementalAppendScanTest, ToSnapshot) { // Test: to_snapshot(snapshot_b) should return 2 files (A, B) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -480,7 +475,7 @@ TEST_P(IncrementalAppendScanTest, ToSnapshotWithTag) { // Test: to_snapshot(t1) should return 2 files { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot("t1"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -490,7 +485,7 @@ TEST_P(IncrementalAppendScanTest, ToSnapshotWithTag) { // Test: to_snapshot(t2) should return 3 files (on branch b1) { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot("t2"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -500,8 +495,7 @@ TEST_P(IncrementalAppendScanTest, ToSnapshotWithTag) { TEST_P(IncrementalAppendScanTest, ToSnapshotWithNonExistingRef) { auto metadata = MakeTableMetadata({}, -1L); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->ToSnapshot("non_existing_ref"); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), @@ -523,8 +517,7 @@ TEST_P(IncrementalAppendScanTest, ToSnapshotWithBranchShouldFail) { {"b1", std::make_shared(SnapshotRef{ .snapshot_id = 2000L, .retention = SnapshotRef::Branch{}})}}); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->ToSnapshot("b1"); EXPECT_THAT(builder->Build(), ::testing::AllOf(IsError(ErrorKind::kValidationFailed), HasErrorMessage("Ref b1 is not a tag"))); @@ -556,7 +549,7 @@ TEST_P(IncrementalAppendScanTest, MultipleRootSnapshots) { // Test: to_snapshot(snapshot_d) should discover snapshots C and D only { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(4000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -570,7 +563,7 @@ TEST_P(IncrementalAppendScanTest, MultipleRootSnapshots) { // because B is not a parent ancestor of D { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(2000L, /*inclusive=*/false).ToSnapshot(4000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); EXPECT_THAT( @@ -584,7 +577,7 @@ TEST_P(IncrementalAppendScanTest, MultipleRootSnapshots) { // because B is not an ancestor of D { ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalAppendScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(2000L, /*inclusive=*/true).ToSnapshot(4000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); EXPECT_THAT( diff --git a/src/iceberg/test/incremental_changelog_scan_test.cc b/src/iceberg/test/incremental_changelog_scan_test.cc index 62fed0f25..62f6d4fe9 100644 --- a/src/iceberg/test/incremental_changelog_scan_test.cc +++ b/src/iceberg/test/incremental_changelog_scan_test.cc @@ -104,8 +104,8 @@ TEST_P(IncrementalChangelogScanTest, DataFilters) { EXPECT_THAT(file_io_->DeleteFile(manifest_a.manifest_path), IsOk()); // Filter by data="k" which should match only file_b (bucket("k", 16) = 1) - ICEBERG_UNWRAP_OR_FAIL(auto builder, IncrementalChangelogScanBuilder::Make( - partitioned_metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, + MakeScanBuilder(partitioned_metadata)); builder->Filter(Expressions::Equal("data", Literal::String("k"))); builder->ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); @@ -142,7 +142,7 @@ TEST_P(IncrementalChangelogScanTest, Overwrites) { // from_snapshot_exclusive(snap1).to_snapshot(snap2) should return 2 tasks ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/false).ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -208,7 +208,7 @@ TEST_P(IncrementalChangelogScanTest, DuplicatedManifests) { .snapshot_id = 2000L, .retention = SnapshotRef::Branch{}})}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -247,7 +247,7 @@ TEST_P(IncrementalChangelogScanTest, FileDeletes) { .snapshot_id = 2000L, .retention = SnapshotRef::Branch{}})}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(1000L, /*inclusive=*/false).ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -302,7 +302,7 @@ TEST_P(IncrementalChangelogScanTest, ExistingEntriesInNewDataManifestsAreIgnored // When scanning from_snapshot_inclusive(C).to_snapshot(C), should only return file_c // because file_a and file_b are marked as EXISTING entries ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->FromSnapshot(3000L, /*inclusive=*/true).ToSnapshot(3000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -355,7 +355,7 @@ TEST_P(IncrementalChangelogScanTest, DataFileRewrites) { // The changelog should only show the original appends (A and B), // not the replace operation ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(3000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -422,7 +422,7 @@ TEST_P(IncrementalChangelogScanTest, ManifestRewritesAreIgnored) { // The changelog should show all 3 files from the original appends, // ignoring the manifest rewrite snapshot ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(4000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -508,7 +508,7 @@ TEST_P(IncrementalChangelogScanTest, DeleteFilesAreNotSupported) { .snapshot_id = 2000L, .retention = SnapshotRef::Branch{}})}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - IncrementalChangelogScanBuilder::Make(metadata, file_io_)); + MakeScanBuilder(metadata)); builder->ToSnapshot(2000L); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); EXPECT_THAT(scan->PlanFiles(), diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index b8217d3cd..09fa12061 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -47,6 +47,7 @@ iceberg_tests = { }, 'table_test': { 'sources': files( + 'catalog_util_test.cc', 'location_provider_test.cc', 'metadata_table_test.cc', 'metrics_config_test.cc', @@ -99,6 +100,7 @@ iceberg_tests = { 'base64_test.cc', 'bucket_util_test.cc', 'config_test.cc', + 'content_file_util_test.cc', 'data_file_set_test.cc', 'decimal_test.cc', 'endian_test.cc', @@ -144,6 +146,7 @@ if get_option('rest').enabled() 'error_handlers_test.cc', 'rest_file_io_test.cc', 'rest_json_serde_test.cc', + 'rest_metrics_reporter_test.cc', 'rest_util_test.cc', ), 'dependencies': [iceberg_rest_dep], diff --git a/src/iceberg/test/rest_catalog_integration_test.cc b/src/iceberg/test/rest_catalog_integration_test.cc index dcb292931..b449bc50f 100644 --- a/src/iceberg/test/rest_catalog_integration_test.cc +++ b/src/iceberg/test/rest_catalog_integration_test.cc @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -41,6 +42,8 @@ #include "iceberg/catalog/rest/rest_catalog.h" #include "iceberg/catalog/session_context.h" #include "iceberg/file_io_registry.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/metrics/metrics_reporters.h" #include "iceberg/partition_spec.h" #include "iceberg/result.h" #include "iceberg/schema.h" @@ -69,6 +72,11 @@ constexpr std::string_view kWarehouseName = "default"; constexpr std::string_view kLocalhostUri = "http://localhost"; constexpr std::string_view kStdFileIOImpl = "test.StdFileIO"; +class TestMetricsReporter final : public MetricsReporter { + public: + Status Report(const MetricsReport& /*report*/) override { return {}; } +}; + /// \brief Check if a localhost port is ready to accept connections. bool CheckServiceReady(uint16_t port) { int sock = socket(AF_INET, SOCK_STREAM, 0); @@ -203,6 +211,39 @@ TEST_F(RestCatalogIntegrationTest, MakeCatalogSuccess) { EXPECT_THAT(root->WithContext(SessionContext{}), IsError(ErrorKind::kInvalidArgument)); } +TEST_F(RestCatalogIntegrationTest, LoadsConfiguredMetricsReporter) { + auto loaded = std::make_shared>(false); + ASSERT_THAT(MetricsReporters::Register( + "rest.catalog.test", + [loaded](const std::unordered_map&) + -> Result> { + loaded->store(true); + return std::make_unique(); + }), + IsOk()); + + ICEBERG_UNWRAP_OR_FAIL( + auto catalog, + CreateCatalogWithProperties( + {{std::string(kMetricsReporterImpl), "rest.catalog.test"}, + {RestCatalogProperties::kMetricsReportingEnabled.key(), "false"}})); + EXPECT_NE(catalog, nullptr); + EXPECT_TRUE(loaded->load()); +} + +TEST_F(RestCatalogIntegrationTest, AttachesRestMetricsReporterWithoutExecutor) { + ICEBERG_UNWRAP_OR_FAIL(auto catalog, CreateCatalog()); + Namespace ns{.levels = {"test_metrics_reporter_without_executor"}}; + ASSERT_THAT(catalog->CreateNamespace(ns, {}), IsOk()); + + TableIdentifier table_id{.ns = ns, .name = "events"}; + ICEBERG_UNWRAP_OR_FAIL(auto table, CreateDefaultTable(catalog, table_id)); + EXPECT_NE(table->reporter(), nullptr); + + ASSERT_THAT(catalog->DropTable(table_id, /*purge=*/false), IsOk()); + ASSERT_THAT(catalog->DropNamespace(ns), IsOk()); +} + TEST_F(RestCatalogIntegrationTest, DefaultCatalogCacheDoesNotKeepRootAlive) { std::weak_ptr weak_root; std::weak_ptr weak_catalog; @@ -375,11 +416,24 @@ TEST_F(RestCatalogIntegrationTest, CreateTable) { EXPECT_EQ(table->name().ns.levels, (std::vector{"test_create_table", "apple", "ios"})); EXPECT_EQ(table->name().name, "t1"); + EXPECT_EQ(table->full_name(), "test_catalog.test_create_table.apple.ios.t1"); // Duplicate creation should fail EXPECT_THAT(CreateDefaultTable(catalog, table_id), IsError(ErrorKind::kAlreadyExists)); } +TEST_F(RestCatalogIntegrationTest, FullNameAlwaysUsesDotSeparator) { + ICEBERG_UNWRAP_OR_FAIL(auto catalog, + CreateCatalogWithProperties( + {{RestCatalogProperties::kName.key(), "rest/catalog"}})); + Namespace ns{.levels = {"test_rest_full_name"}}; + ASSERT_THAT(catalog->CreateNamespace(ns, {}), IsOk()); + + TableIdentifier table_id{.ns = ns, .name = "events"}; + ICEBERG_UNWRAP_OR_FAIL(auto table, CreateDefaultTable(catalog, table_id)); + EXPECT_EQ(table->full_name(), "rest/catalog.test_rest_full_name.events"); +} + TEST_F(RestCatalogIntegrationTest, ListTables) { Namespace ns{.levels = {"test_list_tables"}}; ICEBERG_UNWRAP_OR_FAIL(auto catalog, CreateCatalogAndNamespace(ns)); diff --git a/src/iceberg/test/rest_metrics_reporter_test.cc b/src/iceberg/test/rest_metrics_reporter_test.cc new file mode 100644 index 000000000..29bc3ab04 --- /dev/null +++ b/src/iceberg/test/rest_metrics_reporter_test.cc @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "iceberg/catalog/rest/auth/auth_session.h" +#include "iceberg/catalog/rest/rest_metrics_reporter_internal.h" +#include "iceberg/metrics/commit_report.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/metrics/scan_report.h" +#include "iceberg/result.h" +#include "iceberg/test/matchers.h" +#include "iceberg/util/executor.h" + +namespace iceberg::rest { + +namespace { + +class ManualExecutor final : public Executor { + public: + Status Submit(ExecutorTask task) override { + task_ = std::move(task); + return {}; + } + + bool HasTask() const { return task_.has_value(); } + + void Run() { + auto task = std::move(*task_); + task_.reset(); + std::move(task)(); + } + + private: + std::optional task_; +}; + +class RejectingExecutor final : public Executor { + public: + Status Submit(ExecutorTask /*task*/) override { + return IOError("executor rejected task"); + } +}; + +} // namespace + +class RestMetricsReporterTest : public ::testing::Test { + protected: + void SetUp() override { session_ = auth::AuthSession::MakeDefault({}); } + + std::shared_ptr session_; +}; + +namespace { + +struct ReportTestCase { + std::string name; + MetricsReport report; + std::string expected_report_type; + std::vector> expected_fields; +}; + +ScanReport MakeScanReport() { + ScanReport report; + report.table_name = "ns.tbl"; + report.snapshot_id = 42; + report.schema_id = 0; + return report; +} + +ReportTestCase MakeScanReportCase() { + return {.name = "ScanReport", + .report = MakeScanReport(), + .expected_report_type = "scan-report", + .expected_fields = {{"table-name", "ns.tbl"}, {"snapshot-id", 42}}}; +} + +ReportTestCase MakeCommitReportCase() { + CommitReport report; + report.table_name = "ns.tbl"; + report.snapshot_id = 99; + report.sequence_number = 1; + report.operation = "append"; + return {.name = "CommitReport", + .report = report, + .expected_report_type = "commit-report", + .expected_fields = {{"table-name", "ns.tbl"}, + {"snapshot-id", 99}, + {"sequence-number", 1}, + {"operation", "append"}}}; +} + +} // namespace + +class RestMetricsReporterPayloadTest + : public RestMetricsReporterTest, + public ::testing::WithParamInterface {}; + +TEST_F(RestMetricsReporterTest, SuppressesHttpErrors) { + ManualExecutor executor; + RestMetricsReporter reporter( + "http://localhost:0/v1/ns/tables/tbl/metrics", session_, &executor, + [](const std::string&, const std::string&, auth::AuthSession&) { + throw std::runtime_error("HTTP failure"); + }); + MetricsReport report = MakeScanReport(); + EXPECT_THAT(reporter.Report(report), IsOk()); + ASSERT_TRUE(executor.HasTask()); + EXPECT_NO_THROW(executor.Run()); +} + +TEST_P(RestMetricsReporterPayloadTest, PostsSerializedPayloadToConfiguredEndpoint) { + const auto& test_case = GetParam(); + ManualExecutor executor; + const std::string endpoint = "http://mock-host/v1/ns/tables/tbl/metrics"; + + std::string captured_path; + std::string captured_body; + RestMetricsReporter reporter( + endpoint, session_, &executor, + [&](const std::string& path, const std::string& body, auth::AuthSession&) { + captured_path = path; + captured_body = body; + }); + + EXPECT_THAT(reporter.Report(test_case.report), IsOk()); + EXPECT_TRUE(captured_path.empty()); + ASSERT_TRUE(executor.HasTask()); + executor.Run(); + + EXPECT_EQ(captured_path, endpoint); + auto json = nlohmann::json::parse(captured_body); + EXPECT_EQ(json.at("report-type"), test_case.expected_report_type); + for (const auto& [key, value] : test_case.expected_fields) { + EXPECT_EQ(json.at(key), value); + } +} + +TEST_F(RestMetricsReporterTest, PostsSynchronouslyWithoutExecutor) { + bool posted = false; + RestMetricsReporter reporter( + "http://mock-host/v1/ns/tables/tbl/metrics", session_, nullptr, + [&](const std::string&, const std::string&, auth::AuthSession&) { posted = true; }); + + MetricsReport report = MakeScanReport(); + EXPECT_THAT(reporter.Report(report), IsOk()); + EXPECT_TRUE(posted); +} + +TEST_F(RestMetricsReporterTest, SuppressesExecutorRejection) { + RejectingExecutor executor; + RestMetricsReporter reporter( + "http://mock-host/v1/ns/tables/tbl/metrics", session_, &executor, + [](const std::string&, const std::string&, auth::AuthSession&) {}); + + MetricsReport report = MakeScanReport(); + EXPECT_THAT(reporter.Report(report), IsOk()); +} + +INSTANTIATE_TEST_SUITE_P(ScanAndCommit, RestMetricsReporterPayloadTest, + ::testing::Values(MakeScanReportCase(), MakeCommitReportCase()), + [](const ::testing::TestParamInfo& info) { + return info.param.name; + }); + +} // namespace iceberg::rest diff --git a/src/iceberg/test/scan_planning_metrics_test.cc b/src/iceberg/test/scan_planning_metrics_test.cc new file mode 100644 index 000000000..d3ef4fae9 --- /dev/null +++ b/src/iceberg/test/scan_planning_metrics_test.cc @@ -0,0 +1,647 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include "iceberg/expression/expressions.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/metrics/metrics_reporter.h" +#include "iceberg/metrics/scan_report.h" +#include "iceberg/result.h" +#include "iceberg/snapshot.h" +#include "iceberg/table.h" +#include "iceberg/table_metadata.h" +#include "iceberg/table_scan.h" +#include "iceberg/test/matchers.h" +#include "iceberg/test/mock_catalog.h" +#include "iceberg/test/scan_test_base.h" +#include "iceberg/transform.h" +#include "iceberg/util/timepoint.h" + +namespace iceberg { + +namespace { + +class CapturingReporter final : public MetricsReporter { + public: + Status Report(const MetricsReport& report) override { + if (std::holds_alternative(report)) { + last_ = std::get(report); + } + return {}; + } + + const std::optional& last() const { return last_; } + + private: + std::optional last_; +}; + +} // namespace + +class ScanPlanningMetricsTest : public ScanTestBase { + protected: + void SetUp() override { + ScanTestBase::SetUp(); + reporter_ = std::make_shared(); + + ICEBERG_UNWRAP_OR_FAIL( + id_identity_spec_, + PartitionSpec::Make(/*spec_id=*/2, + {PartitionField(/*source_id=*/1, /*field_id=*/1001, "id", + Transform::Identity())})); + } + + std::shared_ptr MakeDataFile(const std::string& path, + const PartitionValues& partition, + int32_t spec_id, int64_t record_count = 1, + std::optional lower_id = std::nullopt, + std::optional upper_id = std::nullopt) { + auto file = std::make_shared(DataFile{ + .file_path = path, + .file_format = FileFormatType::kParquet, + .partition = partition, + .record_count = record_count, + .file_size_in_bytes = 10, + .sort_order_id = 0, + .partition_spec_id = spec_id, + }); + if (lower_id.has_value()) { + file->lower_bounds[1] = Literal::Int(lower_id.value()).Serialize().value(); + } + if (upper_id.has_value()) { + file->upper_bounds[1] = Literal::Int(upper_id.value()).Serialize().value(); + } + return file; + } + + std::shared_ptr MakePositionDeleteFile( + const std::string& path, const PartitionValues& partition, int32_t spec_id, + std::optional referenced_file = std::nullopt) { + return std::make_shared(DataFile{ + .content = DataFile::Content::kPositionDeletes, + .file_path = path, + .file_format = FileFormatType::kParquet, + .partition = partition, + .record_count = 1, + .file_size_in_bytes = 10, + .referenced_data_file = std::move(referenced_file), + .partition_spec_id = spec_id, + }); + } + + std::shared_ptr MakeDV(const std::string& path, + const PartitionValues& partition, int32_t spec_id, + const std::string& referenced_file) { + return std::make_shared(DataFile{ + .content = DataFile::Content::kPositionDeletes, + .file_path = path, + .file_format = FileFormatType::kPuffin, + .partition = partition, + .record_count = 1, + .file_size_in_bytes = 10, + .referenced_data_file = referenced_file, + .content_offset = 4, + .content_size_in_bytes = 6, + .partition_spec_id = spec_id, + }); + } + + std::shared_ptr MakeEqualityDeleteFile(const std::string& path, + const PartitionValues& partition, + int32_t spec_id, + std::vector equality_ids = {1}) { + return std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = path, + .file_format = FileFormatType::kParquet, + .partition = partition, + .record_count = 1, + .file_size_in_bytes = 10, + .equality_ids = std::move(equality_ids), + .partition_spec_id = spec_id, + }); + } + + std::shared_ptr BuildMetadata( + int8_t format_version, int64_t snapshot_id, int64_t sequence_number, + const std::string& manifest_list_path, + std::shared_ptr spec = nullptr) { + if (!spec) spec = partitioned_spec_; + std::vector> specs = {spec}; + if (spec->spec_id() != unpartitioned_spec_->spec_id()) { + specs.push_back(unpartitioned_spec_); + } + const auto ts = TimePointMsFromUnixMs(1609459200000L); + auto snapshot = + std::make_shared(Snapshot{.snapshot_id = snapshot_id, + .parent_snapshot_id = std::nullopt, + .sequence_number = sequence_number, + .timestamp_ms = ts, + .manifest_list = manifest_list_path, + .schema_id = schema_->schema_id()}); + return std::make_shared( + TableMetadata{.format_version = format_version, + .table_uuid = "test-table-uuid", + .location = "/tmp/table", + .last_sequence_number = sequence_number, + .last_updated_ms = ts, + .last_column_id = 2, + .schemas = {schema_}, + .current_schema_id = schema_->schema_id(), + .partition_specs = std::move(specs), + .default_spec_id = spec->spec_id(), + .last_partition_id = 1001, + .current_snapshot_id = snapshot_id, + .snapshots = {snapshot}, + .snapshot_log = {SnapshotLogEntry{.timestamp_ms = ts, + .snapshot_id = snapshot_id}}, + .default_sort_order_id = 0, + .refs = {{"main", std::make_shared(SnapshotRef{ + .snapshot_id = snapshot_id, + .retention = SnapshotRef::Branch{}})}}}); + } + + std::string WriteManifestList(int8_t format_version, int64_t snapshot_id, + int64_t sequence_number, + const std::vector& manifests) { + return ScanTestBase::WriteManifestList(format_version, snapshot_id, + /*parent_snapshot_id=*/0L, sequence_number, + manifests); + } + + std::shared_ptr reporter_; + std::shared_ptr id_identity_spec_; +}; + +TEST_P(ScanPlanningMetricsTest, ReportsToTableAndScanReporters) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2000L; + const auto part = PartitionValues({Literal::Int(0)}); + + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part, partitioned_spec_->spec_id(), + /*record_count=*/100, + /*lower_id=*/1, /*upper_id=*/50))}, + partitioned_spec_); + auto manifest_list = + WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, {data_manifest}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, manifest_list); + + auto catalog = std::make_shared<::testing::NiceMock>(); + ICEBERG_UNWRAP_OR_FAIL( + auto table, + Table::Make(TableIdentifier{.ns = Namespace{.levels = {"db"}}, .name = "table"}, + metadata, "/tmp/table/metadata.json", file_io_, catalog, "test.table", + reporter_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(*table)); + auto scan_reporter = std::make_shared(); + builder->ReportWith(scan_reporter); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 1u); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& report = *reporter_->last(); + EXPECT_EQ(report.table_name, "test.table"); + EXPECT_EQ(report.snapshot_id, kSnapshotId); + + const auto& m = report.scan_metrics; + ASSERT_TRUE(m.total_planning_duration.has_value()); + EXPECT_EQ(m.total_planning_duration->count, 1); + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 1); + ASSERT_TRUE(scan_reporter->last().has_value()); + EXPECT_EQ(scan_reporter->last()->table_name, "test.table"); +} + +TEST_P(ScanPlanningMetricsTest, ScanReportFilterUsesBoundCaseInsensitiveResolution) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2000L; + const auto part = PartitionValues({Literal::Int(0)}); + + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part, partitioned_spec_->spec_id(), + /*record_count=*/100, + /*lower_id=*/1, /*upper_id=*/50))}, + partitioned_spec_); + auto manifest_list = + WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, {data_manifest}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, manifest_list); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + // "ID" only resolves against the "id" schema field when binding is case-insensitive; + // the sanitized filter should reflect the resolved bound reference, not fail/pass + // through unresolved. + builder->ReportWith(reporter_).CaseSensitive(false).Filter( + Expressions::Equal("ID", Literal::Int(10))); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& report = *reporter_->last(); + ASSERT_NE(report.filter, nullptr); + EXPECT_EQ(report.filter->ToString(), "ref(name=\"id\") == \"(2-digit-int)\""); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithMultipleDataManifests) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2001L; + const auto part = PartitionValues({Literal::Int(0)}); + + auto manifest1 = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part, partitioned_spec_->spec_id())), + MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_b.parquet", part, partitioned_spec_->spec_id()))}, + partitioned_spec_); + + auto manifest2 = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_c.parquet", part, partitioned_spec_->spec_id()))}, + partitioned_spec_); + + auto manifest_list = WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, + {manifest1, manifest2}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, manifest_list); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 3u); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 3); + ASSERT_TRUE(m.result_delete_files.has_value()); + EXPECT_EQ(m.result_delete_files->value, 0); + ASSERT_TRUE(m.scanned_data_manifests.has_value()); + EXPECT_EQ(m.scanned_data_manifests->value, 2); + ASSERT_TRUE(m.scanned_delete_manifests.has_value()); + EXPECT_EQ(m.scanned_delete_manifests->value, 0); + ASSERT_TRUE(m.skipped_data_manifests.has_value()); + EXPECT_EQ(m.skipped_data_manifests->value, 0); + ASSERT_TRUE(m.skipped_delete_manifests.has_value()); + EXPECT_EQ(m.skipped_delete_manifests->value, 0); + ASSERT_TRUE(m.total_data_manifests.has_value()); + EXPECT_EQ(m.total_data_manifests->value, 2); + ASSERT_TRUE(m.total_delete_manifests.has_value()); + EXPECT_EQ(m.total_delete_manifests->value, 0); + ASSERT_TRUE(m.total_file_size_in_bytes.has_value()); + EXPECT_EQ(m.total_file_size_in_bytes->value, 30); + ASSERT_TRUE(m.total_delete_file_size_in_bytes.has_value()); + EXPECT_EQ(m.total_delete_file_size_in_bytes->value, 0); + ASSERT_TRUE(m.skipped_data_files.has_value()); + EXPECT_EQ(m.skipped_data_files->value, 0); + ASSERT_TRUE(m.skipped_delete_files.has_value()); + EXPECT_EQ(m.skipped_delete_files->value, 0); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithManifestPruning) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2002L; + + // Manifest 1: id partition = 1 (files with id=1) + const auto part1 = PartitionValues({Literal::Int(1)}); + auto manifest1 = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part1, id_identity_spec_->spec_id()))}, + id_identity_spec_); + + // Manifest 2: id partition = 2 (files with id=2) + const auto part2 = PartitionValues({Literal::Int(2)}); + auto manifest2 = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_b.parquet", part2, id_identity_spec_->spec_id()))}, + id_identity_spec_); + + auto manifest_list = WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, + {manifest1, manifest2}); + auto metadata = BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, + manifest_list, id_identity_spec_); + + // Filter id = 1: only manifest 1 survives the manifest-level evaluator. + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_).Filter(Expressions::Equal("id", Literal::Int(1))); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 1u); + EXPECT_EQ(tasks[0]->data_file()->file_path, "/data/file_a.parquet"); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.total_data_manifests.has_value()); + EXPECT_EQ(m.total_data_manifests->value, 2); + ASSERT_TRUE(m.scanned_data_manifests.has_value()); + EXPECT_EQ(m.scanned_data_manifests->value, 1); + ASSERT_TRUE(m.skipped_data_manifests.has_value()); + EXPECT_EQ(m.skipped_data_manifests->value, 1); + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 1); + ASSERT_TRUE(m.skipped_data_files.has_value()); + EXPECT_EQ(m.skipped_data_files->value, 0); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithSkippedDataFiles) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2003L; + const auto part = PartitionValues({Literal::Int(0)}); + + // Both files share the same bucket partition so the manifest is not pruned. + // file_a covers id [1, 50]; file_b covers id [51, 100]. + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part, partitioned_spec_->spec_id(), + /*record_count=*/50, /*lower_id=*/1, /*upper_id=*/50)), + MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_b.parquet", part, partitioned_spec_->spec_id(), + /*record_count=*/50, /*lower_id=*/51, /*upper_id=*/100))}, + partitioned_spec_); + auto manifest_list = + WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, {data_manifest}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, manifest_list); + + // Filter id = 25: within file_a's range, outside file_b's range. + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_).Filter(Expressions::Equal("id", Literal::Int(25))); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 1u); + EXPECT_EQ(tasks[0]->data_file()->file_path, "/data/file_a.parquet"); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.total_data_manifests.has_value()); + EXPECT_EQ(m.total_data_manifests->value, 1); + ASSERT_TRUE(m.scanned_data_manifests.has_value()); + EXPECT_EQ(m.scanned_data_manifests->value, 1); + ASSERT_TRUE(m.skipped_data_manifests.has_value()); + EXPECT_EQ(m.skipped_data_manifests->value, 0); + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 1); + ASSERT_TRUE(m.skipped_data_files.has_value()); + EXPECT_EQ(m.skipped_data_files->value, 1); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithDeleteFiles) { + auto version = GetParam(); + if (version < 2) { + GTEST_SKIP() << "Delete files are only supported in format version 2+"; + } + constexpr int64_t kSnapshotId = 2004L; + const auto part = PartitionValues({Literal::Int(0)}); + + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", part, partitioned_spec_->spec_id())), + MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_b.parquet", part, partitioned_spec_->spec_id()))}, + partitioned_spec_); + + auto delete_file = + version == 2 + ? MakePositionDeleteFile("/data/pos_delete.parquet", part, + partitioned_spec_->spec_id(), "/data/file_a.parquet") + : MakeDV("/data/dv.puffin", part, partitioned_spec_->spec_id(), + "/data/file_a.parquet"); + auto delete_manifest = + WriteDeleteManifest(version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, + /*sequence_number=*/2, std::move(delete_file))}, + partitioned_spec_); + + auto manifest_list = WriteManifestList(version, kSnapshotId, /*sequence_number=*/2, + {data_manifest, delete_manifest}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/2, manifest_list); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 2u); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 2); + ASSERT_TRUE(m.result_delete_files.has_value()); + EXPECT_EQ(m.result_delete_files->value, 1); + ASSERT_TRUE(m.scanned_data_manifests.has_value()); + EXPECT_EQ(m.scanned_data_manifests->value, 1); + ASSERT_TRUE(m.scanned_delete_manifests.has_value()); + EXPECT_EQ(m.scanned_delete_manifests->value, 1); + ASSERT_TRUE(m.total_data_manifests.has_value()); + EXPECT_EQ(m.total_data_manifests->value, 1); + ASSERT_TRUE(m.total_delete_manifests.has_value()); + EXPECT_EQ(m.total_delete_manifests->value, 1); + ASSERT_TRUE(m.total_file_size_in_bytes.has_value()); + EXPECT_EQ(m.total_file_size_in_bytes->value, 20); + ASSERT_TRUE(m.total_delete_file_size_in_bytes.has_value()); + EXPECT_EQ(m.total_delete_file_size_in_bytes->value, version == 2 ? 10 : 6); + ASSERT_TRUE(m.indexed_delete_files.has_value()); + EXPECT_EQ(m.indexed_delete_files->value, 1); + ASSERT_TRUE(m.positional_delete_files.has_value()); + EXPECT_EQ(m.positional_delete_files->value, version == 2 ? 1 : 0); + ASSERT_TRUE(m.equality_delete_files.has_value()); + EXPECT_EQ(m.equality_delete_files->value, 0); + ASSERT_TRUE(m.dvs.has_value()); + EXPECT_EQ(m.dvs->value, version == 3 ? 1 : 0); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithIgnoreDeletedManifest) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2006L; + const auto part = PartitionValues({Literal::Int(0)}); + + // Manifest 1: the single entry is kDeleted, + // IgnoreDeleted() will skip this manifest at the manifest level. + auto deleted_only_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kDeleted, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/deleted_file.parquet", part, + partitioned_spec_->spec_id()))}, + partitioned_spec_); + + // Manifest 2: one kAdded entry — survives IgnoreDeleted and contributes + // one task to the result. + auto added_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/live_file.parquet", part, partitioned_spec_->spec_id()))}, + partitioned_spec_); + + auto manifest_list = WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, + {deleted_only_manifest, added_manifest}); + auto metadata = + BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, manifest_list); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 1u); + EXPECT_EQ(tasks[0]->data_file()->file_path, "/data/live_file.parquet"); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.total_data_manifests.has_value()); + EXPECT_EQ(m.total_data_manifests->value, 2); + ASSERT_TRUE(m.scanned_data_manifests.has_value()); + EXPECT_EQ(m.scanned_data_manifests->value, 1); + ASSERT_TRUE(m.skipped_data_manifests.has_value()); + EXPECT_EQ(m.skipped_data_manifests->value, 1); + ASSERT_TRUE(m.skipped_data_files.has_value()); + EXPECT_EQ(m.skipped_data_files->value, 0); + ASSERT_TRUE(m.result_data_files.has_value()); + EXPECT_EQ(m.result_data_files->value, 1); +} + +TEST_P(ScanPlanningMetricsTest, ScanningWithDeleteFileSharedAcrossDataFiles) { + auto version = GetParam(); + if (version < 2) { + GTEST_SKIP() << "Delete files are only supported in format version 2+"; + } + constexpr int64_t kSnapshotId = 2005L; + const auto empty_partition = PartitionValues(std::vector{}); + + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", empty_partition, + unpartitioned_spec_->spec_id())), + MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_b.parquet", empty_partition, + unpartitioned_spec_->spec_id()))}, + unpartitioned_spec_); + + // A single global equality-delete file applies to every unpartitioned data file. + auto delete_manifest = WriteDeleteManifest( + version, kSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/2, + MakeEqualityDeleteFile("/data/global-eq-delete.parquet", empty_partition, + unpartitioned_spec_->spec_id()))}, + unpartitioned_spec_); + + auto manifest_list = WriteManifestList(version, kSnapshotId, /*sequence_number=*/2, + {data_manifest, delete_manifest}); + auto metadata = BuildMetadata(version, kSnapshotId, /*sequence_number=*/2, + manifest_list, unpartitioned_spec_); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 2u); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& m = reporter_->last()->scan_metrics; + + ASSERT_TRUE(m.indexed_delete_files.has_value()); + EXPECT_EQ(m.indexed_delete_files->value, 1); + ASSERT_TRUE(m.equality_delete_files.has_value()); + EXPECT_EQ(m.equality_delete_files->value, 1); + + ASSERT_TRUE(m.result_delete_files.has_value()); + EXPECT_EQ(m.result_delete_files->value, 2); + ASSERT_TRUE(m.total_delete_file_size_in_bytes.has_value()); + EXPECT_EQ(m.total_delete_file_size_in_bytes->value, 20); +} + +TEST_P(ScanPlanningMetricsTest, ScanReportIncludesNestedProjectedFields) { + auto version = GetParam(); + constexpr int64_t kSnapshotId = 2020L; + + // Override the base flat schema with one that has a nested struct column. + schema_ = std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeRequired( + 2, "location", + struct_({SchemaField::MakeRequired(3, "lat", float64()), + SchemaField::MakeRequired(4, "long", float64())}))}, + /*schema_id=*/0); + + auto data_manifest = WriteDataManifest( + version, kSnapshotId, + {MakeEntry( + ManifestStatus::kAdded, kSnapshotId, /*sequence_number=*/1, + MakeDataFile("/data/file_a.parquet", PartitionValues(std::vector{}), + unpartitioned_spec_->spec_id()))}, + unpartitioned_spec_); + auto manifest_list = + WriteManifestList(version, kSnapshotId, /*sequence_number=*/1, {data_manifest}); + auto metadata = BuildMetadata(version, kSnapshotId, /*sequence_number=*/1, + manifest_list, unpartitioned_spec_); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); + builder->ReportWith(reporter_); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); + ASSERT_EQ(tasks.size(), 1u); + + ASSERT_TRUE(reporter_->last().has_value()); + const auto& report = *reporter_->last(); + + EXPECT_THAT(report.projected_field_ids, ::testing::UnorderedElementsAre(1, 2, 3, 4)); + EXPECT_THAT( + report.projected_field_names, + ::testing::UnorderedElementsAre("id", "location", "location.lat", "location.long")); +} + +INSTANTIATE_TEST_SUITE_P(ScanPlanningMetricsVersions, ScanPlanningMetricsTest, + testing::Values(2, 3)); + +} // namespace iceberg diff --git a/src/iceberg/test/scan_test_base.h b/src/iceberg/test/scan_test_base.h index 4e32febe2..618b9c140 100644 --- a/src/iceberg/test/scan_test_base.h +++ b/src/iceberg/test/scan_test_base.h @@ -38,11 +38,14 @@ #include "iceberg/partition_spec.h" #include "iceberg/schema.h" #include "iceberg/snapshot.h" +#include "iceberg/table.h" #include "iceberg/table_metadata.h" #include "iceberg/table_scan.h" #include "iceberg/test/matchers.h" +#include "iceberg/test/mock_catalog.h" #include "iceberg/transform.h" #include "iceberg/type.h" +#include "iceberg/util/macros.h" namespace iceberg { @@ -68,6 +71,17 @@ class ScanTestBase : public testing::TestWithParam { "data_bucket_16_2", Transform::Bucket(16))})); } + template + Result>> MakeScanBuilder( + std::shared_ptr metadata) { + ICEBERG_ASSIGN_OR_RAISE( + auto table, + Table::Make(TableIdentifier{.name = "table"}, std::move(metadata), + "/tmp/table/metadata.json", file_io_, + std::make_shared<::testing::NiceMock>(), "test.table")); + return TableScanBuilder::Make(*table); + } + /// \brief Generate a unique manifest file path. std::string MakeManifestPath() { return std::format("manifest-{}-{}.avro", manifest_counter_++, diff --git a/src/iceberg/test/sql_catalog_test.cc b/src/iceberg/test/sql_catalog_test.cc index 41953b14d..8378a05d3 100644 --- a/src/iceberg/test/sql_catalog_test.cc +++ b/src/iceberg/test/sql_catalog_test.cc @@ -226,6 +226,7 @@ TEST_F(SqlCatalogTest, TableLifecycle) { catalog_->CreateTable(ident, MakeSchema(), PartitionSpec::Unpartitioned(), SortOrder::Unsorted(), location, {}); ASSERT_THAT(created, IsOk()); + EXPECT_EQ(created.value()->full_name(), "test_catalog.db.t1"); EXPECT_THAT(catalog_->TableExists(ident), HasValue(::testing::Eq(true))); // Duplicate create fails. @@ -241,6 +242,7 @@ TEST_F(SqlCatalogTest, TableLifecycle) { auto loaded = catalog_->LoadTable(ident); ASSERT_THAT(loaded, IsOk()); + EXPECT_EQ(loaded.value()->full_name(), "test_catalog.db.t1"); // Rename. TableIdentifier renamed{.ns = ns, .name = "t2"}; diff --git a/src/iceberg/test/table_scan_test.cc b/src/iceberg/test/table_scan_test.cc index 1fb02762d..375c9aa53 100644 --- a/src/iceberg/test/table_scan_test.cc +++ b/src/iceberg/test/table_scan_test.cc @@ -151,8 +151,7 @@ class TableScanTest : public ScanTestBase { TEST_P(TableScanTest, TableScanBuilderOptions) { // Test basic scan creation and default values - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); ICEBERG_UNWRAP_OR_FAIL(auto basic_scan, builder->Build()); EXPECT_NE(basic_scan, nullptr); EXPECT_EQ(basic_scan->metadata(), table_metadata_); @@ -166,8 +165,7 @@ TEST_P(TableScanTest, TableScanBuilderOptions) { constexpr int64_t kMinRows = 1000; constexpr int64_t kSnapshotId = 1000L; - ICEBERG_UNWRAP_OR_FAIL(auto builder2, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder2, MakeScanBuilder(table_metadata_)); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder2->Option("key1", "value1") .Option("key2", "value2") .CaseSensitive(false) @@ -199,8 +197,7 @@ TEST_P(TableScanTest, TableScanBuilderOptions) { EXPECT_EQ(context.snapshot_id.value(), kSnapshotId); // Test UseRef separately - ICEBERG_UNWRAP_OR_FAIL(auto builder3, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder3, MakeScanBuilder(table_metadata_)); builder3->UseRef("main"); ICEBERG_UNWRAP_OR_FAIL(auto ref_scan, builder3->Build()); ICEBERG_UNWRAP_OR_FAIL(auto snapshot, ref_scan->snapshot()); @@ -220,8 +217,7 @@ TEST_P(TableScanTest, UseRefPreservesInt64SnapshotIds) { table_metadata_->refs["branch-with-large-snapshot-id"] = std::make_shared( SnapshotRef{.snapshot_id = kLargeSnapshotId, .retention = SnapshotRef::Branch{}}); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); builder->UseRef("branch-with-large-snapshot-id"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); @@ -257,8 +253,7 @@ TEST_P(TableScanTest, IncludeColumnStatsUsesFinalSnapshotSchema) { SnapshotRef{.snapshot_id = kEvolvedSnapshotId, .retention = SnapshotRef::Branch{}}); { - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); builder->IncludeColumnStats({"id"}).UseSnapshot(kEvolvedSnapshotId); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto scan_schema, scan->schema()); @@ -271,8 +266,7 @@ TEST_P(TableScanTest, IncludeColumnStatsUsesFinalSnapshotSchema) { } { - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); builder->IncludeColumnStats({"id"}).UseRef("evolved-branch"); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto scan_schema, scan->schema()); @@ -286,8 +280,7 @@ TEST_P(TableScanTest, IncludeColumnStatsUsesFinalSnapshotSchema) { } TEST_P(TableScanTest, IncludeColumnStatsRejectsMissingColumn) { - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); builder->IncludeColumnStats({"missing"}); EXPECT_THAT(builder->Build(), @@ -297,28 +290,19 @@ TEST_P(TableScanTest, IncludeColumnStatsRejectsMissingColumn) { TEST_P(TableScanTest, TableScanBuilderValidationErrors) { // Test negative min rows - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(table_metadata_)); builder->MinRowsRequested(-1); EXPECT_THAT(builder->Build(), IsError(ErrorKind::kValidationFailed)); // Test invalid snapshot ID - ICEBERG_UNWRAP_OR_FAIL(auto builder2, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder2, MakeScanBuilder(table_metadata_)); builder2->UseSnapshot(9999L); EXPECT_THAT(builder2->Build(), IsError(ErrorKind::kValidationFailed)); // Test invalid ref - ICEBERG_UNWRAP_OR_FAIL(auto builder3, - DataTableScanBuilder::Make(table_metadata_, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder3, MakeScanBuilder(table_metadata_)); builder3->UseRef("non-existent-ref"); EXPECT_THAT(builder3->Build(), IsError(ErrorKind::kValidationFailed)); - - // Test null inputs - EXPECT_THAT(DataTableScanBuilder::Make(nullptr, file_io_), - IsError(ErrorKind::kInvalidArgument)); - EXPECT_THAT(DataTableScanBuilder::Make(table_metadata_, nullptr), - IsError(ErrorKind::kInvalidArgument)); } TEST_P(TableScanTest, DataTableScanPlanFilesEmpty) { @@ -332,8 +316,7 @@ TEST_P(TableScanTest, DataTableScanPlanFilesEmpty) { .snapshots = {}, .refs = {}}); - ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(empty_metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(empty_metadata)); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); EXPECT_TRUE(tasks.empty()); @@ -391,7 +374,7 @@ TEST_P(TableScanTest, PlanFilesWithDataManifests) { })}}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(metadata_with_manifest, file_io_)); + MakeScanBuilder(metadata_with_manifest)); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); ASSERT_EQ(tasks.size(), 2); @@ -435,7 +418,7 @@ TEST_P(TableScanTest, PlanRowLineage) { .snapshot_id = kSnapshotId, .retention = SnapshotRef::Branch{}})}}, unpartitioned_spec_); - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); ASSERT_EQ(tasks.size(), 1); @@ -502,7 +485,7 @@ TEST_P(TableScanTest, PlanFilesWithMultipleManifests) { })}}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(metadata_with_manifests, file_io_)); + MakeScanBuilder(metadata_with_manifests)); test::ThreadExecutor executor; builder->PlanWith(executor); @@ -569,7 +552,7 @@ TEST_P(TableScanTest, PlanFilesWithFilter) { // Test 1: Filter matches only data1.parquet (id=25 is in range [1, 50]) { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Filter(Expressions::Equal("id", Literal::Int(25))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -579,7 +562,7 @@ TEST_P(TableScanTest, PlanFilesWithFilter) { // Test 2: Filter matches only data2.parquet (id=75 is in range [51, 100]) { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Filter(Expressions::Equal("id", Literal::Int(75))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -589,7 +572,7 @@ TEST_P(TableScanTest, PlanFilesWithFilter) { // Test 3: Filter matches both files (id > 0 covers both ranges) { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Filter(Expressions::GreaterThan("id", Literal::Int(0))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -600,7 +583,7 @@ TEST_P(TableScanTest, PlanFilesWithFilter) { // Test 4: Filter matches no files (id=200 is outside both ranges) { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Filter(Expressions::Equal("id", Literal::Int(200))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); @@ -676,7 +659,7 @@ TEST_P(TableScanTest, PlanFilesWithDeleteFiles) { })}}}); ICEBERG_UNWRAP_OR_FAIL(auto builder, - DataTableScanBuilder::Make(metadata_with_manifests, file_io_)); + MakeScanBuilder(metadata_with_manifests)); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto tasks, scan->PlanFiles()); ASSERT_EQ(tasks.size(), 2); @@ -726,7 +709,7 @@ TEST_P(TableScanTest, SchemaWithSelectedColumnsAndFilter) { // Select "data" column, filter on "id" column { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Select({"data"}).Filter(Expressions::Equal("id", Literal::Int(42))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto projected_schema, scan->schema()); @@ -744,7 +727,7 @@ TEST_P(TableScanTest, SchemaWithSelectedColumnsAndFilter) { // Select "id" and "value", filter on "data" { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Select({"id", "value"}) .Filter(Expressions::Equal("data", Literal::String("test"))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); @@ -764,7 +747,7 @@ TEST_P(TableScanTest, SchemaWithSelectedColumnsAndFilter) { // Select "id", filter on "id" - should only have "id" once { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Select({"id"}).Filter(Expressions::Equal("id", Literal::Int(42))); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto projected_schema, scan->schema()); @@ -778,7 +761,7 @@ TEST_P(TableScanTest, SchemaWithSelectedColumnsAndFilter) { // Select columns without filter { - ICEBERG_UNWRAP_OR_FAIL(auto builder, DataTableScanBuilder::Make(metadata, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto builder, MakeScanBuilder(metadata)); builder->Select({"data"}); ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); ICEBERG_UNWRAP_OR_FAIL(auto projected_schema, scan->schema()); diff --git a/src/iceberg/test/table_test.cc b/src/iceberg/test/table_test.cc index 716d92967..94a00e879 100644 --- a/src/iceberg/test/table_test.cc +++ b/src/iceberg/test/table_test.cc @@ -110,6 +110,7 @@ TYPED_TEST(TypedTableTest, BasicMetadata) { EXPECT_EQ(table->name().name, "test_table"); EXPECT_EQ(table->name().ns.levels, (std::vector{"db"})); + EXPECT_EQ(table->full_name(), "db.test_table"); EXPECT_EQ(table->metadata()->format_version, 2); EXPECT_EQ(table->metadata()->schemas.size(), 1); } @@ -154,9 +155,11 @@ TEST(StaticTableTest, NewMutatingOperationsAreNotSupported) { auto metadata = std::make_shared( TableMetadata{.format_version = 2, .schemas = {schema}, .current_schema_id = 1}); TableIdentifier ident{.ns = Namespace{.levels = {"db"}}, .name = "test_table"}; - ICEBERG_UNWRAP_OR_FAIL(auto table, StaticTable::Make(ident, std::move(metadata), - "s3://bucket/meta.json", io)); + ICEBERG_UNWRAP_OR_FAIL( + auto table, StaticTable::Make(ident, std::move(metadata), "s3://bucket/meta.json", + io, "catalog.db.test_table")); + EXPECT_EQ(table->full_name(), "catalog.db.test_table"); EXPECT_THAT(table->NewUpdateStatistics(), IsError(ErrorKind::kNotSupported)); EXPECT_THAT(table->NewUpdatePartitionStatistics(), IsError(ErrorKind::kNotSupported)); EXPECT_THAT(table->NewFastAppend(), IsError(ErrorKind::kNotSupported)); diff --git a/src/iceberg/test/update_partition_spec_test.cc b/src/iceberg/test/update_partition_spec_test.cc index e310c0d1f..ebbb80243 100644 --- a/src/iceberg/test/update_partition_spec_test.cc +++ b/src/iceberg/test/update_partition_spec_test.cc @@ -51,7 +51,8 @@ class UpdatePartitionSpecTest : public ::testing::TestWithParam { protected: void SetUp() override { file_io_ = arrow::ArrowFileSystemFileIO::MakeMockFileIO(); - catalog_ = InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", {}); + ICEBERG_UNWRAP_OR_FAIL( + catalog_, InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", {})); format_version_ = GetParam(); test_schema_ = std::make_shared( std::vector{SchemaField::MakeRequired(1, "id", int64()), diff --git a/src/iceberg/test/update_schema_test.cc b/src/iceberg/test/update_schema_test.cc index db6b1d02e..4057c52c8 100644 --- a/src/iceberg/test/update_schema_test.cc +++ b/src/iceberg/test/update_schema_test.cc @@ -75,8 +75,9 @@ class UpdateSchemaTest : public ::testing::Test { return mock_file_io_ptr->FileIO::WriteFile(file_location, content); }); file_io_ = std::move(mock_file_io); - catalog_ = - InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", /*properties=*/{}); + ICEBERG_UNWRAP_OR_FAIL(catalog_, + InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", + /*properties=*/{})); } void RegisterTable(std::unique_ptr metadata) { diff --git a/src/iceberg/test/update_test_base.h b/src/iceberg/test/update_test_base.h index 8ddbe959c..122720ce5 100644 --- a/src/iceberg/test/update_test_base.h +++ b/src/iceberg/test/update_test_base.h @@ -56,8 +56,9 @@ class UpdateTestBase : public ::testing::Test { /// \brief Initialize file IO and create necessary directories. void InitializeFileIO() { file_io_ = arrow::ArrowFileSystemFileIO::MakeMockFileIO(); - catalog_ = - InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", /*properties=*/{}); + ICEBERG_UNWRAP_OR_FAIL(catalog_, + InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", + /*properties=*/{})); // Arrow MockFS cannot automatically create directories. auto arrow_fs = std::dynamic_pointer_cast<::arrow::fs::internal::MockFileSystem>( diff --git a/src/iceberg/transaction.cc b/src/iceberg/transaction.cc index 503121658..80d39c8a8 100644 --- a/src/iceberg/transaction.cc +++ b/src/iceberg/transaction.cc @@ -20,7 +20,6 @@ #include #include -#include #include "iceberg/catalog.h" #include "iceberg/location_provider.h" @@ -419,7 +418,7 @@ Result> Transaction::CommitOnce(bool is_first_attempt) { ctx_->metadata_builder = TableMetadataBuilder::BuildFrom(ctx_->table->metadata().get()); for (const auto& update : pending_updates_) { - ICEBERG_RETURN_UNEXPECTED(Apply(*update)); + ICEBERG_RETURN_UNEXPECTED(update->Commit()); } } ICEBERG_ASSIGN_OR_RAISE(requirements, TableRequirements::ForUpdateTable( diff --git a/src/iceberg/transaction.h b/src/iceberg/transaction.h index 42a504565..007b1057e 100644 --- a/src/iceberg/transaction.h +++ b/src/iceberg/transaction.h @@ -48,7 +48,10 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this> Make(std::shared_ptr
table, TransactionKind kind); - /// \brief Create a transaction from an existing context (used by PendingUpdate::Commit) + /// \brief Create a detached transaction from an existing context. + /// + /// Used by PendingUpdate::Commit for table-created updates. The transaction is not + /// stored in TransactionContext because it exists only for that commit call. static Result> Make( std::shared_ptr ctx); @@ -170,6 +173,7 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this ctx_; // Keep track of all created pending updates. std::vector> pending_updates_; + // To make the state simple, we require updates are added and committed in order. bool last_update_committed_ = true; // Tracks if transaction has been committed to prevent double-commit diff --git a/src/iceberg/type_fwd.h b/src/iceberg/type_fwd.h index f437e3db7..0d2b17fae 100644 --- a/src/iceberg/type_fwd.h +++ b/src/iceberg/type_fwd.h @@ -227,6 +227,14 @@ class LocationProvider; class SessionCatalog; struct SessionContext; +/// \brief Task execution. +class Executor; + +/// \brief Metrics reporting. +class MetricsReporter; +class ScanMetrics; +class CommitMetrics; + /// \brief Table. class Table; class TableProperties; diff --git a/src/iceberg/update/snapshot_update.cc b/src/iceberg/update/snapshot_update.cc index 1d6916da6..6e8d28bd5 100644 --- a/src/iceberg/update/snapshot_update.cc +++ b/src/iceberg/update/snapshot_update.cc @@ -31,6 +31,9 @@ #include "iceberg/manifest/manifest_reader.h" #include "iceberg/manifest/manifest_writer.h" #include "iceberg/manifest/rolling_manifest_writer.h" +#include "iceberg/metrics/commit_report.h" +#include "iceberg/metrics/metrics_context.h" +#include "iceberg/metrics/metrics_reporter.h" #include "iceberg/partition_summary_internal.h" #include "iceberg/table.h" // IWYU pragma: keep #include "iceberg/transaction.h" @@ -194,7 +197,36 @@ SnapshotUpdate::SnapshotUpdate(std::shared_ptr ctx) base().properties.Get(TableProperties::kSnapshotIdInheritanceEnabled)), commit_uuid_(Uuid::GenerateV7().ToString()), target_manifest_size_bytes_( - base().properties.Get(TableProperties::kManifestTargetSizeBytes)) {} + base().properties.Get(TableProperties::kManifestTargetSizeBytes)), + commit_metrics_(CommitMetrics::Make(*MetricsContext::Default())), + reporter_(ctx_->table->reporter()) {} + +Status SnapshotUpdate::Commit() { + commit_metrics_->attempts->Increment(); + [[maybe_unused]] auto commit_timer = commit_metrics_->total_duration->Start(); + return PendingUpdate::Commit(); +} + +void SnapshotUpdate::ReportCommit() const { + ICEBERG_DCHECK(staged_snapshot_ != nullptr, + "Staged snapshot is null after a successful commit"); + + if (!reporter_) { + return; + } + + const auto operation = staged_snapshot_->Operation(); + CommitReport report{ + .table_name = ctx_->table->full_name(), + .snapshot_id = staged_snapshot_->snapshot_id, + .sequence_number = staged_snapshot_->sequence_number, + .operation = operation.has_value() ? std::string(operation.value()) : "", + .commit_metrics = + CommitMetricsResult::From(*commit_metrics_, staged_snapshot_->summary), + .metadata = {}, + }; + std::ignore = reporter_->Report(report); +} void SnapshotUpdate::SetSummaryProperty(const std::string& property, const std::string& value) { @@ -386,6 +418,8 @@ Status SnapshotUpdate::Finalize(Result commit_result) { } } + ReportCommit(); + return {}; } diff --git a/src/iceberg/update/snapshot_update.h b/src/iceberg/update/snapshot_update.h index e29d96d57..1812b7072 100644 --- a/src/iceberg/update/snapshot_update.h +++ b/src/iceberg/update/snapshot_update.h @@ -59,6 +59,16 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdate { Kind kind() const override { return Kind::kUpdateSnapshot; } bool IsRetryable() const override { return true; } + Status Commit() override; + + /// \brief Set the metrics reporter for this snapshot update. + /// + /// \param reporter The metrics reporter to use. + /// \return Reference to this for method chaining. + auto& ReportWith(this auto& self, std::shared_ptr reporter) { + static_cast(self).reporter_ = std::move(reporter); + return self; + } /// \brief Set a callback to delete files instead of the table's default. /// @@ -267,6 +277,9 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdate { /// \brief Clean up all uncommitted files Status CleanAll(); + /// \brief Report metrics for the most recently staged snapshot. + void ReportCommit() const; + protected: SnapshotSummaryBuilder summary_; @@ -285,6 +298,8 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdate { std::function delete_func_; std::string target_branch_{SnapshotRef::kMainBranch}; std::shared_ptr staged_snapshot_; + std::unique_ptr commit_metrics_; + std::shared_ptr reporter_; }; } // namespace iceberg diff --git a/src/iceberg/util/content_file_util.cc b/src/iceberg/util/content_file_util.cc index 89c875087..23eeaec8d 100644 --- a/src/iceberg/util/content_file_util.cc +++ b/src/iceberg/util/content_file_util.cc @@ -83,6 +83,13 @@ std::string ContentFileUtil::DVDesc(const DataFile& file) { file.referenced_data_file.value_or("")); } +int64_t ContentFileUtil::ContentSizeInBytes(const DataFile& file) { + // content_size_in_bytes is only meaningful for deletion vectors; other content files + // (data files, positional/equality delete files) must use file_size_in_bytes. + return IsDV(file) ? file.content_size_in_bytes.value_or(file.file_size_in_bytes) + : file.file_size_in_bytes; +} + void ContentFileUtil::DropAllStats(DataFile& data_file) { data_file.column_sizes.clear(); data_file.value_counts.clear(); diff --git a/src/iceberg/util/content_file_util.h b/src/iceberg/util/content_file_util.h index f547716d2..0db2e755d 100644 --- a/src/iceberg/util/content_file_util.h +++ b/src/iceberg/util/content_file_util.h @@ -51,6 +51,10 @@ struct ICEBERG_EXPORT ContentFileUtil { /// \brief Generate a description string for a deletion vector. static std::string DVDesc(const DataFile& file); + /// \brief Size in bytes of the portion of `file` relevant to a scan task — the DV's + /// own content size when set, otherwise the file's total size. + static int64_t ContentSizeInBytes(const DataFile& file); + /// \brief In-place drop stats. static void DropAllStats(DataFile& data_file);