diff --git a/src/iceberg/catalog/rest/json_serde.cc b/src/iceberg/catalog/rest/json_serde.cc index eebdc1969..e369f0f21 100644 --- a/src/iceberg/catalog/rest/json_serde.cc +++ b/src/iceberg/catalog/rest/json_serde.cc @@ -379,9 +379,9 @@ Result CreateTableRequestFromJson(const nlohmann::json& json if (json.contains(kPartitionSpec)) { ICEBERG_ASSIGN_OR_RAISE(auto partition_spec, GetJsonValue(json, kPartitionSpec)); - ICEBERG_ASSIGN_OR_RAISE(request.partition_spec, - PartitionSpecFromJson(request.schema, partition_spec, - PartitionSpec::kInitialSpecId)); + ICEBERG_ASSIGN_OR_RAISE( + request.partition_spec, + PartitionSpecFromJson(request.schema, partition_spec, kInitialSpecId)); } if (json.contains(kWriteOrder)) { ICEBERG_ASSIGN_OR_RAISE(auto sort_order, diff --git a/src/iceberg/constants.h b/src/iceberg/constants.h index 1d5941626..a736a375c 100644 --- a/src/iceberg/constants.h +++ b/src/iceberg/constants.h @@ -38,6 +38,19 @@ constexpr int64_t kInvalidSequenceNumber = -1; /// adapter. constexpr int64_t kUnassignedSequenceNumber = -1; -// TODO(gangwu): move other commonly used constants here. +constexpr int32_t kInitialSchemaId = 0; +constexpr int32_t kInitialColumnId = 0; +constexpr int32_t kInvalidColumnId = -1; + +constexpr int32_t kInvalidFieldId = -1; + +constexpr int32_t kInitialSpecId = 0; +/// \brief The start ID for partition field. It is only used to generate +/// partition field id for v1 metadata where it is tracked. +constexpr int32_t kLegacyPartitionDataIdStart = 1000; +constexpr int32_t kInvalidPartitionFieldId = -1; + +constexpr int32_t kUnsortedOrderId = 0; +constexpr int32_t kInitialSortOrderId = 1; } // namespace iceberg diff --git a/src/iceberg/json_serde.cc b/src/iceberg/json_serde.cc index 2d8c22255..1fc0c0912 100644 --- a/src/iceberg/json_serde.cc +++ b/src/iceberg/json_serde.cc @@ -564,7 +564,7 @@ Result> SchemaFromJson(const nlohmann::json& json) { auto identifier_field_ids, GetJsonValueOrDefault>(json, kIdentifierFieldIds)); - return Schema::Make(std::move(fields), schema_id_opt.value_or(Schema::kInitialSchemaId), + return Schema::Make(std::move(fields), schema_id_opt.value_or(kInitialSchemaId), std::move(identifier_field_ids)); } @@ -599,8 +599,8 @@ Result> PartitionFieldFromJson( int32_t field_id; if (allow_field_id_missing) { // Partition field id in v1 is not tracked, so we use -1 to indicate that. - ICEBERG_ASSIGN_OR_RAISE(field_id, GetJsonValueOrDefault( - json, kFieldId, SchemaField::kInvalidFieldId)); + ICEBERG_ASSIGN_OR_RAISE( + field_id, GetJsonValueOrDefault(json, kFieldId, kInvalidFieldId)); } else { ICEBERG_ASSIGN_OR_RAISE(field_id, GetJsonValue(json, kFieldId)); } @@ -1001,14 +1001,14 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, SafeDumpJson(partition_spec_json)); } - int32_t next_partition_field_id = PartitionSpec::kLegacyPartitionDataIdStart; + int32_t next_partition_field_id = kLegacyPartitionDataIdStart; std::vector fields; for (const auto& entry_json : partition_spec_json) { ICEBERG_ASSIGN_OR_RAISE( auto field, PartitionFieldFromJson( entry_json, /*allow_field_id_missing=*/format_version == 1)); int32_t field_id = field->field_id(); - if (field_id == SchemaField::kInvalidFieldId) { + if (field_id == kInvalidFieldId) { // If the field ID is not set, we need to assign a new one field_id = next_partition_field_id++; } @@ -1018,9 +1018,8 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, // Create partition spec with schema validation ICEBERG_ASSIGN_OR_RAISE( - auto spec, - PartitionSpec::Make(*current_schema, PartitionSpec::kInitialSpecId, - std::move(fields), /*allow_missing_fields=*/false)); + auto spec, PartitionSpec::Make(*current_schema, kInitialSpecId, std::move(fields), + /*allow_missing_fields=*/false)); default_spec_id = spec->spec_id(); partition_specs.push_back(std::move(spec)); } diff --git a/src/iceberg/manifest/manifest_list.h b/src/iceberg/manifest/manifest_list.h index 2f3185a18..e86682d68 100644 --- a/src/iceberg/manifest/manifest_list.h +++ b/src/iceberg/manifest/manifest_list.h @@ -92,7 +92,7 @@ struct ICEBERG_EXPORT ManifestFile { /// Field id: 502 /// ID of a partition spec used to write the manifest; must be listed in table metadata /// partition-specs - int32_t partition_spec_id = PartitionSpec::kInitialSpecId; + int32_t partition_spec_id = kInitialSpecId; /// Field id: 517 /// The type of files tracked by the manifest, either data or delete files; 0 for all v1 /// manifests diff --git a/src/iceberg/partition_spec.cc b/src/iceberg/partition_spec.cc index c00eab7d2..f198f7534 100644 --- a/src/iceberg/partition_spec.cc +++ b/src/iceberg/partition_spec.cc @@ -275,7 +275,7 @@ Result> PartitionSpec::Make( bool PartitionSpec::HasSequentialFieldIds(const PartitionSpec& spec) { for (size_t i = 0; i < spec.fields().size(); i += 1) { - if (spec.fields()[i].field_id() != PartitionSpec::kLegacyPartitionDataIdStart + i) { + if (spec.fields()[i].field_id() != kLegacyPartitionDataIdStart + i) { return false; } } diff --git a/src/iceberg/partition_spec.h b/src/iceberg/partition_spec.h index 0fb8814b8..5e88fae15 100644 --- a/src/iceberg/partition_spec.h +++ b/src/iceberg/partition_spec.h @@ -30,6 +30,7 @@ #include #include +#include "iceberg/constants.h" #include "iceberg/iceberg_export.h" #include "iceberg/partition_field.h" #include "iceberg/result.h" @@ -46,12 +47,6 @@ namespace iceberg { /// evolution. class ICEBERG_EXPORT PartitionSpec : public util::Formattable { public: - static constexpr int32_t kInitialSpecId = 0; - /// \brief The start ID for partition field. It is only used to generate - /// partition field id for v1 metadata where it is tracked. - static constexpr int32_t kLegacyPartitionDataIdStart = 1000; - static constexpr int32_t kInvalidPartitionFieldId = -1; - /// \brief Get an unsorted partition spec singleton. static const std::shared_ptr& Unpartitioned(); diff --git a/src/iceberg/schema.cc b/src/iceberg/schema.cc index 00905378a..cc19d31cc 100644 --- a/src/iceberg/schema.cc +++ b/src/iceberg/schema.cc @@ -363,7 +363,7 @@ Result SchemaCache::InitHighestFieldId(const Schema* schema) { ICEBERG_ASSIGN_OR_RAISE(auto id_to_field, InitIdToFieldMap(schema)); if (id_to_field.empty()) { - return Schema::kInitialColumnId; + return kInitialColumnId; } auto max_it = std::ranges::max_element( diff --git a/src/iceberg/schema.h b/src/iceberg/schema.h index 3c84bc2af..5f769dcae 100644 --- a/src/iceberg/schema.h +++ b/src/iceberg/schema.h @@ -30,6 +30,7 @@ #include #include +#include "iceberg/constants.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/schema_field.h" @@ -48,10 +49,6 @@ class SchemaCache; /// evolution. class ICEBERG_EXPORT Schema : public StructType { public: - static constexpr int32_t kInitialSchemaId = 0; - static constexpr int32_t kInitialColumnId = 0; - static constexpr int32_t kInvalidColumnId = -1; - /// \brief Special value to select all columns from manifest files. static constexpr std::string_view kAllColumns = "*"; diff --git a/src/iceberg/schema_field.h b/src/iceberg/schema_field.h index fd20226a5..9b943e99c 100644 --- a/src/iceberg/schema_field.h +++ b/src/iceberg/schema_field.h @@ -28,6 +28,7 @@ #include #include +#include "iceberg/constants.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -38,8 +39,6 @@ namespace iceberg { /// \brief A type combined with a name. class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable { public: - static constexpr int32_t kInvalidFieldId = -1; - /// \brief Construct a field. /// \param[in] field_id The field ID. /// \param[in] name The field name. diff --git a/src/iceberg/schema_internal.cc b/src/iceberg/schema_internal.cc index dedb603e2..82e7a2f28 100644 --- a/src/iceberg/schema_internal.cc +++ b/src/iceberg/schema_internal.cc @@ -310,7 +310,7 @@ std::unique_ptr FromStructType(StructType&& struct_type, for (auto& field : struct_type.fields()) { fields.emplace_back(std::move(field)); } - auto schema_id = schema_id_opt.value_or(Schema::kInitialSchemaId); + auto schema_id = schema_id_opt.value_or(kInitialSchemaId); return std::make_unique(std::move(fields), schema_id); } diff --git a/src/iceberg/sort_order.h b/src/iceberg/sort_order.h index 7c9b799fb..fc42caf70 100644 --- a/src/iceberg/sort_order.h +++ b/src/iceberg/sort_order.h @@ -25,6 +25,7 @@ #include #include +#include "iceberg/constants.h" #include "iceberg/iceberg_export.h" #include "iceberg/sort_field.h" #include "iceberg/type_fwd.h" @@ -39,9 +40,6 @@ namespace iceberg { /// applied to the data. class ICEBERG_EXPORT SortOrder : public util::Formattable { public: - static constexpr int32_t kUnsortedOrderId = 0; - static constexpr int32_t kInitialSortOrderId = 1; - /// \brief Get an unsorted sort order singleton. static const std::shared_ptr& Unsorted(); diff --git a/src/iceberg/table_metadata.cc b/src/iceberg/table_metadata.cc index d3b5629c8..11fb6eadb 100644 --- a/src/iceberg/table_metadata.cc +++ b/src/iceberg/table_metadata.cc @@ -71,7 +71,7 @@ Result> FreshPartitionSpec(int32_t spec_id, const Schema& fresh_schema) { std::vector partition_fields; partition_fields.reserve(spec.fields().size()); - int32_t last_partition_field_id = PartitionSpec::kInvalidPartitionFieldId; + int32_t last_partition_field_id = kInvalidPartitionFieldId; for (auto& field : spec.fields()) { ICEBERG_ASSIGN_OR_RAISE(auto source_name, base_schema.FindColumnNameById(field.source_id())); @@ -145,7 +145,7 @@ std::vector> ChangesForCreate( if (auto partition_spec_result = metadata.PartitionSpec(); partition_spec_result.has_value()) { auto spec = partition_spec_result.value(); - if (spec && spec->spec_id() != PartitionSpec::kInitialSpecId) { + if (spec && spec->spec_id() != kInitialSpecId) { changes.push_back(std::make_unique(spec)); } else { changes.push_back( @@ -205,17 +205,16 @@ Result> TableMetadata::Make( int32_t last_column_id = 0; auto next_id = [&last_column_id]() -> int32_t { return ++last_column_id; }; ICEBERG_ASSIGN_OR_RAISE(auto fresh_schema, - AssignFreshIds(Schema::kInitialSchemaId, schema, next_id)); + AssignFreshIds(kInitialSchemaId, schema, next_id)); // Rebuild the partition spec using the new column ids ICEBERG_ASSIGN_OR_RAISE( - auto fresh_spec, - FreshPartitionSpec(PartitionSpec::kInitialSpecId, spec, schema, *fresh_schema)); + auto fresh_spec, FreshPartitionSpec(kInitialSpecId, spec, schema, *fresh_schema)); // rebuild the sort order using the new column ids ICEBERG_ASSIGN_OR_RAISE( auto fresh_order, - FreshSortOrder(SortOrder::kInitialSortOrderId, sort_order, schema, *fresh_schema)) + FreshSortOrder(kInitialSortOrderId, sort_order, schema, *fresh_schema)) // Validata the metrics configuration. ICEBERG_RETURN_UNEXPECTED( @@ -549,11 +548,11 @@ class TableMetadataBuilder::Impl { metadata_.format_version = format_version; metadata_.last_sequence_number = TableMetadata::kInitialSequenceNumber; metadata_.last_updated_ms = kInvalidLastUpdatedMs; - metadata_.last_column_id = Schema::kInvalidColumnId; - metadata_.default_spec_id = PartitionSpec::kInitialSpecId; - metadata_.last_partition_id = PartitionSpec::kInvalidPartitionFieldId; + metadata_.last_column_id = kInvalidColumnId; + metadata_.default_spec_id = kInitialSpecId; + metadata_.last_partition_id = kInvalidPartitionFieldId; metadata_.current_snapshot_id = kInvalidSnapshotId; - metadata_.default_sort_order_id = SortOrder::kInitialSortOrderId; + metadata_.default_sort_order_id = kInitialSortOrderId; metadata_.next_row_id = TableMetadata::kInitialRowId; } @@ -1367,10 +1366,10 @@ Result> TableMetadataBuilder::Impl::Build() { int32_t TableMetadataBuilder::Impl::ReuseOrCreateNewSortOrderId( const SortOrder& new_order) { if (new_order.is_unsorted()) { - return SortOrder::kUnsortedOrderId; + return kUnsortedOrderId; } // determine the next order id - int32_t new_order_id = SortOrder::kInitialSortOrderId; + int32_t new_order_id = kInitialSortOrderId; for (const auto& order : metadata_.sort_orders) { if (order->SameOrder(new_order)) { return order->order_id(); @@ -1384,7 +1383,7 @@ int32_t TableMetadataBuilder::Impl::ReuseOrCreateNewSortOrderId( int32_t TableMetadataBuilder::Impl::ReuseOrCreateNewPartitionSpecId( const PartitionSpec& new_spec) { // if the spec already exists, use the same ID. otherwise, use the highest ID + 1. - int32_t new_spec_id = PartitionSpec::kInitialSpecId; + int32_t new_spec_id = kInitialSpecId; for (const auto& spec : metadata_.partition_specs) { if (new_spec.CompatibleWith(*spec)) { return spec->spec_id(); diff --git a/src/iceberg/table_scan.cc b/src/iceberg/table_scan.cc index 937af94b1..28bd38b6b 100644 --- a/src/iceberg/table_scan.cc +++ b/src/iceberg/table_scan.cc @@ -388,7 +388,7 @@ TableScanBuilder::ResolveSnapshotSchema() { if (context_.snapshot_id.has_value()) { ICEBERG_ASSIGN_OR_RAISE(auto snapshot, metadata_->SnapshotById(*context_.snapshot_id)); - int32_t schema_id = snapshot->schema_id.value_or(Schema::kInitialSchemaId); + int32_t schema_id = snapshot->schema_id.value_or(kInitialSchemaId); ICEBERG_ASSIGN_OR_RAISE(snapshot_schema_, metadata_->SchemaById(schema_id)); } else { ICEBERG_ASSIGN_OR_RAISE(snapshot_schema_, metadata_->Schema()); diff --git a/src/iceberg/test/assign_id_visitor_test.cc b/src/iceberg/test/assign_id_visitor_test.cc index 8bec0ad53..9a36a739c 100644 --- a/src/iceberg/test/assign_id_visitor_test.cc +++ b/src/iceberg/test/assign_id_visitor_test.cc @@ -83,7 +83,7 @@ Result> CreateNestedSchema( SchemaField::MakeOptional(/*field_id=*/30, "map", CreateMapWithStructValue()), SchemaField::MakeRequired(/*field_id=*/40, "struct", CreateNestedStruct()), }, - Schema::kInitialSchemaId, std::move(identifier_field_ids)); + kInitialSchemaId, std::move(identifier_field_ids)); } } // namespace @@ -94,7 +94,7 @@ TEST(AssignFreshIdVisitorTest, FlatSchema) { std::atomic id = 0; auto next_id = [&id]() { return ++id; }; ICEBERG_UNWRAP_OR_FAIL(auto fresh_schema, - AssignFreshIds(Schema::kInitialSchemaId, schema, next_id)); + AssignFreshIds(kInitialSchemaId, schema, next_id)); ASSERT_EQ(fresh_schema->fields().size(), schema.fields().size()); EXPECT_EQ(Schema( @@ -104,7 +104,7 @@ TEST(AssignFreshIdVisitorTest, FlatSchema) { SchemaField::MakeOptional(/*field_id=*/3, "age", iceberg::int32()), SchemaField::MakeRequired(/*field_id=*/4, "data", iceberg::float64()), }, - Schema::kInitialSchemaId), + kInitialSchemaId), *fresh_schema); } @@ -113,7 +113,7 @@ TEST(AssignFreshIdVisitorTest, NestedSchema) { std::atomic id = 0; auto next_id = [&id]() { return ++id; }; ICEBERG_UNWRAP_OR_FAIL(auto fresh_schema, - AssignFreshIds(Schema::kInitialSchemaId, *schema, next_id)); + AssignFreshIds(kInitialSchemaId, *schema, next_id)); ASSERT_EQ(4, fresh_schema->fields().size()); for (int32_t i = 0; i < fresh_schema->fields().size(); ++i) { @@ -175,7 +175,7 @@ TEST(AssignFreshIdVisitorTest, RefreshIdentifierId) { ICEBERG_UNWRAP_OR_FAIL(auto schema, CreateNestedSchema({10, 301})); ICEBERG_UNWRAP_OR_FAIL(auto fresh_schema, - AssignFreshIds(Schema::kInitialSchemaId, *schema, next_id)); + AssignFreshIds(kInitialSchemaId, *schema, next_id)); EXPECT_THAT(fresh_schema->IdentifierFieldIds(), testing::ElementsAre(1, 12)); ICEBERG_UNWRAP_OR_FAIL(auto identifier_field_names, fresh_schema->IdentifierFieldNames()); diff --git a/src/iceberg/test/data_writer_test.cc b/src/iceberg/test/data_writer_test.cc index a3a8fc088..a61efa6d5 100644 --- a/src/iceberg/test/data_writer_test.cc +++ b/src/iceberg/test/data_writer_test.cc @@ -486,7 +486,7 @@ TEST_F(EqualityDeleteWriterTest, MetadataAfterClose) { // Partition spec id must be set ASSERT_TRUE(data_file->partition_spec_id.has_value()); - EXPECT_EQ(data_file->partition_spec_id.value(), PartitionSpec::kInitialSpecId); + EXPECT_EQ(data_file->partition_spec_id.value(), kInitialSpecId); // Equality field ids must be set ASSERT_EQ(data_file->equality_ids.size(), 2); diff --git a/src/iceberg/test/location_provider_test.cc b/src/iceberg/test/location_provider_test.cc index c78eb588e..20610b56d 100644 --- a/src/iceberg/test/location_provider_test.cc +++ b/src/iceberg/test/location_provider_test.cc @@ -102,9 +102,9 @@ TEST_F(LocationProviderTest, ObjectStorageWithPartition) { ICEBERG_UNWRAP_OR_FAIL( auto mock_spec, - PartitionSpec::Make(PartitionSpec::kInitialSpecId, + PartitionSpec::Make(kInitialSpecId, {PartitionField(1, 1, "data#1", Transform::Identity())}, - PartitionSpec::kInvalidPartitionFieldId + 1)); + kInvalidPartitionFieldId + 1)); PartitionValues mock_partition_data({Literal::String("val#1")}); ICEBERG_UNWRAP_OR_FAIL( auto location, diff --git a/src/iceberg/test/manifest_evaluator_test.cc b/src/iceberg/test/manifest_evaluator_test.cc index 4562e74ec..c1f60c778 100644 --- a/src/iceberg/test/manifest_evaluator_test.cc +++ b/src/iceberg/test/manifest_evaluator_test.cc @@ -70,7 +70,7 @@ class ManifestEvaluatorTest : public ::testing::Test { std::vector BuildIdentityFields() { std::vector fields; - int32_t partition_field_id = PartitionSpec::kLegacyPartitionDataIdStart; + int32_t partition_field_id = kLegacyPartitionDataIdStart; auto add_field = [&](int32_t source_id, std::string name) { fields.emplace_back(source_id, partition_field_id++, std::move(name), Transform::Identity()); diff --git a/src/iceberg/test/metadata_serde_test.cc b/src/iceberg/test/metadata_serde_test.cc index 0d3b5959b..a688ea7ce 100644 --- a/src/iceberg/test/metadata_serde_test.cc +++ b/src/iceberg/test/metadata_serde_test.cc @@ -114,7 +114,7 @@ TEST(MetadataSerdeTest, DeserializeV1Valid) { .last_updated_ms = TimePointMsFromUnixMs(1602638573874), .last_column_id = 3, .schemas = {expected_schema}, - .current_schema_id = Schema::kInitialSchemaId, + .current_schema_id = kInitialSchemaId, .partition_specs = {expected_spec}, .default_spec_id = 0, .last_partition_id = 1000, diff --git a/src/iceberg/test/partition_spec_test.cc b/src/iceberg/test/partition_spec_test.cc index 89c4cdc83..9efb5551b 100644 --- a/src/iceberg/test/partition_spec_test.cc +++ b/src/iceberg/test/partition_spec_test.cc @@ -133,7 +133,7 @@ TEST(PartitionSpecTest, PartitionTypeTest) { auto field6 = SchemaField::MakeRequired(6, "id_truncate", int32()); auto const schema = std::make_shared( std::vector{field1, field2, field3, field4, field5, field6}, - Schema::kInitialSchemaId); + kInitialSchemaId); ICEBERG_UNWRAP_OR_FAIL(auto parsed_spec, PartitionSpecFromJson(schema, json, 1)); ICEBERG_UNWRAP_OR_FAIL(auto partition_type, parsed_spec->PartitionType(*schema)); @@ -152,7 +152,7 @@ TEST(PartitionSpecTest, PartitionTypeTest) { TEST(PartitionSpecTest, InvalidTransformForType) { // Test Day transform on string type (should fail) auto field_string = SchemaField::MakeRequired(6, "s", string()); - Schema schema_string({field_string}, Schema::kInitialSchemaId); + Schema schema_string({field_string}, kInitialSchemaId); PartitionField pt_field_invalid(6, 1005, "s_day", Transform::Day()); auto result = PartitionSpec::Make(schema_string, 1, {pt_field_invalid}, false); @@ -169,7 +169,7 @@ TEST(PartitionSpecTest, InvalidTransformForType) { TEST(PartitionSpecTest, SourceIdNotFound) { auto field1 = SchemaField::MakeRequired(1, "id", int64()); auto field2 = SchemaField::MakeRequired(2, "ts", timestamp()); - Schema schema({field1, field2}, Schema::kInitialSchemaId); + Schema schema({field1, field2}, kInitialSchemaId); // Try to create partition field with source ID 99 which doesn't exist PartitionField pt_field_invalid(99, 1000, "Test", Transform::Identity()); @@ -182,7 +182,7 @@ TEST(PartitionSpecTest, SourceIdNotFound) { TEST(PartitionSpecTest, AllowMissingFields) { auto field1 = SchemaField::MakeRequired(1, "id", int64()); auto field2 = SchemaField::MakeRequired(2, "ts", timestamp()); - Schema schema({field1, field2}, Schema::kInitialSchemaId); + Schema schema({field1, field2}, kInitialSchemaId); // Create partition field with source ID 99 which doesn't exist PartitionField pt_field_missing(99, 1000, "Test", Transform::Identity()); @@ -204,13 +204,13 @@ TEST(PartitionSpecTest, AllowMissingFields) { TEST(PartitionSpecTest, PartitionFieldInStruct) { auto field1 = SchemaField::MakeRequired(1, "id", int64()); auto field2 = SchemaField::MakeRequired(2, "ts", timestamp()); - Schema base_schema({field1, field2}, Schema::kInitialSchemaId); + Schema base_schema({field1, field2}, kInitialSchemaId); auto struct_type = std::make_shared(std::vector{field1, field2}); auto outer_struct = SchemaField::MakeRequired(11, "MyStruct", struct_type); - Schema schema({outer_struct}, Schema::kInitialSchemaId); + Schema schema({outer_struct}, kInitialSchemaId); PartitionField pt_field(1, 1000, "id_partition", Transform::Identity()); EXPECT_THAT(PartitionSpec::Make(schema, 1, {pt_field}, false), IsOk()); @@ -226,7 +226,7 @@ TEST(PartitionSpecTest, PartitionFieldInStructInStruct) { auto outer_struct = std::make_shared(std::vector{inner_field}); SchemaField outer_field(12, "Outer", outer_struct, true); - Schema schema({outer_field}, Schema::kInitialSchemaId); + Schema schema({outer_field}, kInitialSchemaId); PartitionField pt_field(1, 1000, "id_partition", Transform::Identity()); EXPECT_THAT(PartitionSpec::Make(schema, 1, {pt_field}, false), IsOk()); } @@ -234,7 +234,7 @@ TEST(PartitionSpecTest, PartitionFieldInStructInStruct) { TEST(PartitionSpecTest, PartitionFieldInList) { auto list_type = std::make_shared(1, int32(), /*element_required=*/false); auto list_field = SchemaField::MakeRequired(2, "MyList", list_type); - Schema schema({list_field}, Schema::kInitialSchemaId); + Schema schema({list_field}, kInitialSchemaId); // Try to partition on the list element field (field ID 1 is the element) PartitionField pt_field(1, 1000, "element_partition", Transform::Identity()); @@ -251,7 +251,7 @@ TEST(PartitionSpecTest, PartitionFieldInStructInList) { /*element_required=*/false); auto list_field = SchemaField::MakeRequired(3, "MyList", list_type); - Schema schema({list_field}, Schema::kInitialSchemaId); + Schema schema({list_field}, kInitialSchemaId); PartitionField pt_field(1, 1000, "foo_partition", Transform::Identity()); auto result = PartitionSpec::Make(schema, 1, {pt_field}, false); @@ -265,7 +265,7 @@ TEST(PartitionSpecTest, PartitionFieldInMap) { auto map_type = std::make_shared(key_field, value_field); auto map_field = SchemaField::MakeRequired(3, "MyMap", map_type); - Schema schema({map_field}, Schema::kInitialSchemaId); + Schema schema({map_field}, kInitialSchemaId); PartitionField pt_field_key(1, 1000, "key_partition", Transform::Identity()); auto result_key = PartitionSpec::Make(schema, 1, {pt_field_key}, false); @@ -290,7 +290,7 @@ TEST(PartitionSpecTest, PartitionFieldInStructInMap) { auto map_type = std::make_shared(key_field, value_field); auto map_field = SchemaField::MakeRequired(5, "MyMap", map_type); - Schema schema({map_field}, Schema::kInitialSchemaId); + Schema schema({map_field}, kInitialSchemaId); PartitionField pt_field_key(1, 1000, "foo_partition", Transform::Identity()); auto result_key = PartitionSpec::Make(schema, 1, {pt_field_key}, false); @@ -308,7 +308,7 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsExactDuplicates) { // Create a schema with different field types auto ts_field = SchemaField::MakeRequired(1, "ts", timestamp()); auto id_field = SchemaField::MakeRequired(2, "id", int64()); - Schema schema({ts_field, id_field}, Schema::kInitialSchemaId); + Schema schema({ts_field, id_field}, kInitialSchemaId); // Test: exact duplicate transforms on same field (same dedup name) { @@ -334,7 +334,7 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsExactDuplicates) { // Test: same truncate width on same field (redundant) { auto name_field = SchemaField::MakeRequired(3, "name", string()); - Schema schema_with_string({name_field}, Schema::kInitialSchemaId); + Schema schema_with_string({name_field}, kInitialSchemaId); PartitionField truncate1(3, 1000, "name_trunc_4_3_1", Transform::Truncate(4)); PartitionField truncate2(3, 1001, "name_trunc_4_3_2", Transform::Truncate(4)); @@ -351,7 +351,7 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsAllowedCases) { auto ts_field = SchemaField::MakeRequired(1, "ts", timestamp()); auto id_field = SchemaField::MakeRequired(2, "id", int64()); auto name_field = SchemaField::MakeRequired(3, "name", string()); - Schema schema({ts_field, id_field, name_field}, Schema::kInitialSchemaId); + Schema schema({ts_field, id_field, name_field}, kInitialSchemaId); // Test: different bucket sizes on same field (allowed - different dedup names) { @@ -404,7 +404,7 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsIdentityTransforms) { // Create a schema with different field types auto id_field = SchemaField::MakeRequired(1, "id", int64()); auto name_field = SchemaField::MakeRequired(2, "name", string()); - Schema schema({id_field, name_field}, Schema::kInitialSchemaId); + Schema schema({id_field, name_field}, kInitialSchemaId); // Test: multiple identity transforms on same field (redundant) { @@ -431,7 +431,7 @@ TEST(PartitionSpecTest, PartitionPath) { auto id_field = SchemaField::MakeRequired(1, "id", int64()); auto name_field = SchemaField::MakeRequired(2, "name", string()); auto ts_field = SchemaField::MakeRequired(3, "ts", timestamp()); - Schema schema({id_field, name_field, ts_field}, Schema::kInitialSchemaId); + Schema schema({id_field, name_field, ts_field}, kInitialSchemaId); // Create partition fields PartitionField id_field_partition(1, 1000, "id_partition", Transform::Identity()); diff --git a/src/iceberg/test/snapshot_util_test.cc b/src/iceberg/test/snapshot_util_test.cc index a47b403da..3e701542b 100644 --- a/src/iceberg/test/snapshot_util_test.cc +++ b/src/iceberg/test/snapshot_util_test.cc @@ -77,10 +77,10 @@ std::shared_ptr CreateTableMetadataWithSnapshots( metadata->last_column_id = 2; metadata->current_schema_id = 0; metadata->schemas.push_back(CreateTestSchema()); - metadata->default_spec_id = PartitionSpec::kInitialSpecId; + metadata->default_spec_id = kInitialSpecId; metadata->last_partition_id = 0; metadata->current_snapshot_id = main2_snapshot_id; - metadata->default_sort_order_id = SortOrder::kInitialSortOrderId; + metadata->default_sort_order_id = kInitialSortOrderId; metadata->sort_orders.push_back(SortOrder::Unsorted()); metadata->next_row_id = TableMetadata::kInitialRowId; diff --git a/src/iceberg/test/sort_order_test.cc b/src/iceberg/test/sort_order_test.cc index d2bef580a..9e8e8587e 100644 --- a/src/iceberg/test/sort_order_test.cc +++ b/src/iceberg/test/sort_order_test.cc @@ -197,17 +197,17 @@ TEST_F(SortOrderTest, MakeInvalidSortOrderEmptyFields) { } TEST_F(SortOrderTest, MakeInvalidSortOrderUnsortedId) { - auto sort_order = SortOrder::Make(*schema_, SortOrder::kUnsortedOrderId, - std::vector{*sort_field1_}); + auto sort_order = + SortOrder::Make(*schema_, kUnsortedOrderId, std::vector{*sort_field1_}); EXPECT_THAT(sort_order, IsError(ErrorKind::kInvalidArgument)); EXPECT_THAT(sort_order, HasErrorMessage(std::format("{} is reserved for unsorted sort order", - SortOrder::kUnsortedOrderId))); + kUnsortedOrderId))); } TEST_F(SortOrderTest, MakeValidUnsortedSortOrder) { - ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(SortOrder::kUnsortedOrderId, - std::vector{})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order, + SortOrder::Make(kUnsortedOrderId, std::vector{})); ASSERT_NE(sort_order, nullptr); EXPECT_TRUE(sort_order->is_unsorted()); diff --git a/src/iceberg/test/table_metadata_builder_test.cc b/src/iceberg/test/table_metadata_builder_test.cc index 4146cb01f..bd671ef09 100644 --- a/src/iceberg/test/table_metadata_builder_test.cc +++ b/src/iceberg/test/table_metadata_builder_test.cc @@ -74,14 +74,14 @@ std::unique_ptr CreateBaseMetadata( metadata->schemas.push_back(CreateTestSchema()); if (spec == nullptr) { metadata->partition_specs.push_back(PartitionSpec::Unpartitioned()); - metadata->default_spec_id = PartitionSpec::kInitialSpecId; + metadata->default_spec_id = kInitialSpecId; } else { metadata->default_spec_id = spec->spec_id(); metadata->partition_specs.push_back(std::move(spec)); } metadata->last_partition_id = 0; metadata->current_snapshot_id = kInvalidSnapshotId; - metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + metadata->default_sort_order_id = kUnsortedOrderId; metadata->sort_orders.push_back(SortOrder::Unsorted()); metadata->next_row_id = TableMetadata::kInitialRowId; return metadata; @@ -118,17 +118,17 @@ TEST(TableMetadataTest, Make) { // Check partition spec ASSERT_EQ(1, metadata->partition_specs.size()); - EXPECT_EQ(PartitionSpec::kInitialSpecId, metadata->partition_specs[0]->spec_id()); + EXPECT_EQ(kInitialSpecId, metadata->partition_specs[0]->spec_id()); auto spec_fields = metadata->partition_specs[0]->fields() | std::ranges::to(); ASSERT_EQ(1, spec_fields.size()); - EXPECT_EQ(PartitionSpec::kInvalidPartitionFieldId + 1, spec_fields[0].field_id()); + EXPECT_EQ(kInvalidPartitionFieldId + 1, spec_fields[0].field_id()); EXPECT_EQ(2, spec_fields[0].source_id()); EXPECT_EQ("part_name", spec_fields[0].name()); // Check sort order ASSERT_EQ(1, metadata->sort_orders.size()); - EXPECT_EQ(SortOrder::kInitialSortOrderId, metadata->sort_orders[0]->order_id()); + EXPECT_EQ(kInitialSortOrderId, metadata->sort_orders[0]->order_id()); auto order_fields = metadata->sort_orders[0]->fields() | std::ranges::to(); ASSERT_EQ(1, order_fields.size()); EXPECT_EQ(3, order_fields[0].source_id()); @@ -215,8 +215,8 @@ TEST(TableMetadataBuilderTest, BuildFromEmpty) { EXPECT_EQ(metadata->format_version, 2); EXPECT_EQ(metadata->last_sequence_number, TableMetadata::kInitialSequenceNumber); - EXPECT_EQ(metadata->default_spec_id, PartitionSpec::kInitialSpecId); - EXPECT_EQ(metadata->default_sort_order_id, SortOrder::kUnsortedOrderId); + EXPECT_EQ(metadata->default_spec_id, kInitialSpecId); + EXPECT_EQ(metadata->default_sort_order_id, kUnsortedOrderId); EXPECT_EQ(metadata->current_snapshot_id, kInvalidSnapshotId); EXPECT_TRUE(metadata->metadata_log.empty()); } @@ -839,7 +839,7 @@ TEST(TableMetadataBuilderTest, SetCurrentSchemaWithPartitionSpecRebuild) { auto schema = CreateTestSchema(); ICEBERG_UNWRAP_OR_FAIL( auto spec, - PartitionSpec::Make(PartitionSpec::kInitialSpecId, + PartitionSpec::Make(kInitialSpecId, {PartitionField(1, 1000, "id_bucket", Transform::Bucket(16))}, 1000)); base->partition_specs.push_back(std::move(spec)); @@ -956,7 +956,7 @@ TEST(TableMetadataBuilderTest, SetCurrentSchemaRebuildsSpecsAndOrders) { auto schema = CreateTestSchema(); ICEBERG_UNWRAP_OR_FAIL( auto spec, - PartitionSpec::Make(PartitionSpec::kInitialSpecId, + PartitionSpec::Make(kInitialSpecId, {PartitionField(1, 1000, "id_bucket", Transform::Bucket(16))}, 1000)); base->partition_specs.push_back(std::move(spec)); @@ -1024,7 +1024,7 @@ TEST(TableMetadataBuilderTest, RemoveSchemasBasic) { builder->RemoveSchemas({2, 3}); ICEBERG_UNWRAP_OR_FAIL(metadata, builder->Build()); ASSERT_EQ(metadata->schemas.size(), 1); - EXPECT_EQ(metadata->schemas[0]->schema_id(), Schema::kInitialSchemaId); + EXPECT_EQ(metadata->schemas[0]->schema_id(), kInitialSchemaId); } TEST(TableMetadataBuilderTest, RemoveSchemasCannotRemoveCurrent) { @@ -1076,7 +1076,7 @@ TEST(TableMetadataBuilderTest, RemoveSchemasNonExistent) { builder->RemoveSchemas({1, 999, 888}); ICEBERG_UNWRAP_OR_FAIL(metadata, builder->Build()); ASSERT_EQ(metadata->schemas.size(), 1); - EXPECT_EQ(metadata->schemas[0]->schema_id(), Schema::kInitialSchemaId); + EXPECT_EQ(metadata->schemas[0]->schema_id(), kInitialSchemaId); } TEST(TableMetadataBuilderTest, RemoveSchemasEmptySet) { diff --git a/src/iceberg/test/table_requirements_test.cc b/src/iceberg/test/table_requirements_test.cc index dd0aa8f63..848362f76 100644 --- a/src/iceberg/test/table_requirements_test.cc +++ b/src/iceberg/test/table_requirements_test.cc @@ -50,11 +50,11 @@ std::unique_ptr CreateBaseMetadata( metadata->last_sequence_number = 0; metadata->last_updated_ms = TimePointMs{std::chrono::milliseconds(1000)}; metadata->last_column_id = 0; - metadata->current_schema_id = Schema::kInitialSchemaId; - metadata->default_spec_id = PartitionSpec::kInitialSpecId; + metadata->current_schema_id = kInitialSchemaId; + metadata->default_spec_id = kInitialSpecId; metadata->last_partition_id = 0; metadata->current_snapshot_id = kInvalidSnapshotId; - metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + metadata->default_sort_order_id = kUnsortedOrderId; metadata->next_row_id = TableMetadata::kInitialRowId; return metadata; } @@ -491,11 +491,10 @@ TEST(TableRequirementsTest, SetDefaultPartitionSpec) { TEST(TableRequirementsTest, SetDefaultPartitionSpecFailure) { auto metadata = CreateBaseMetadata(); - metadata->default_spec_id = PartitionSpec::kInitialSpecId; + metadata->default_spec_id = kInitialSpecId; std::vector> updates; - updates.push_back( - std::make_unique(PartitionSpec::kInitialSpecId)); + updates.push_back(std::make_unique(kInitialSpecId)); auto result = TableRequirements::ForUpdateTable(*metadata, updates); ASSERT_THAT(result, IsOk()); @@ -504,7 +503,7 @@ TEST(TableRequirementsTest, SetDefaultPartitionSpecFailure) { // Create updated metadata with different default_spec_id auto updated = CreateBaseMetadata(); - updated->default_spec_id = PartitionSpec::kInitialSpecId + 1; + updated->default_spec_id = kInitialSpecId + 1; // Find and validate the AssertDefaultSpecID requirement for (const auto& req : requirements) { @@ -801,11 +800,10 @@ TEST(TableRequirementsTest, SetDefaultSortOrder) { TEST(TableRequirementsTest, SetDefaultSortOrderFailure) { auto metadata = CreateBaseMetadata(); - metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + metadata->default_sort_order_id = kUnsortedOrderId; std::vector> updates; - updates.push_back( - std::make_unique(SortOrder::kUnsortedOrderId)); + updates.push_back(std::make_unique(kUnsortedOrderId)); auto result = TableRequirements::ForUpdateTable(*metadata, updates); ASSERT_THAT(result, IsOk()); @@ -814,7 +812,7 @@ TEST(TableRequirementsTest, SetDefaultSortOrderFailure) { // Create updated metadata with different default_sort_order_id auto updated = CreateBaseMetadata(); - updated->default_sort_order_id = SortOrder::kUnsortedOrderId + 1; + updated->default_sort_order_id = kUnsortedOrderId + 1; // Find and validate the AssertDefaultSortOrderID requirement for (const auto& req : requirements) { diff --git a/src/iceberg/test/table_update_test.cc b/src/iceberg/test/table_update_test.cc index 3e41318cf..0e600d683 100644 --- a/src/iceberg/test/table_update_test.cc +++ b/src/iceberg/test/table_update_test.cc @@ -73,11 +73,11 @@ std::unique_ptr CreateBaseMetadata() { metadata->current_schema_id = 0; metadata->schemas.push_back(CreateTestSchema()); metadata->partition_specs.push_back(PartitionSpec::Unpartitioned()); - metadata->default_spec_id = PartitionSpec::kInitialSpecId; + metadata->default_spec_id = kInitialSpecId; metadata->last_partition_id = 0; metadata->current_snapshot_id = kInvalidSnapshotId; metadata->sort_orders.push_back(SortOrder::Unsorted()); - metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + metadata->default_sort_order_id = kUnsortedOrderId; metadata->next_row_id = TableMetadata::kInitialRowId; return metadata; } diff --git a/src/iceberg/test/update_partition_spec_test.cc b/src/iceberg/test/update_partition_spec_test.cc index 632c4a55e..fd8c627d3 100644 --- a/src/iceberg/test/update_partition_spec_test.cc +++ b/src/iceberg/test/update_partition_spec_test.cc @@ -63,12 +63,12 @@ class UpdatePartitionSpecTest : public ::testing::TestWithParam { // Create unpartitioned and partitioned specs matching Java test ICEBERG_UNWRAP_OR_FAIL( auto unpartitioned_spec, - PartitionSpec::Make(PartitionSpec::kInitialSpecId, std::vector{}, - PartitionSpec::kLegacyPartitionDataIdStart - 1)); + PartitionSpec::Make(kInitialSpecId, std::vector{}, + kLegacyPartitionDataIdStart - 1)); ICEBERG_UNWRAP_OR_FAIL( partitioned_spec_, PartitionSpec::Make( - PartitionSpec::kInitialSpecId, + kInitialSpecId, std::vector{ PartitionField(3, 1000, "category", Transform::Identity()), PartitionField(2, 1001, "ts_day", Transform::Day()), @@ -131,7 +131,7 @@ class UpdatePartitionSpecTest : public ::testing::TestWithParam { metadata->default_spec_id = spec->spec_id(); metadata->last_partition_id = spec->last_assigned_field_id(); metadata->current_snapshot_id = kInvalidSnapshotId; - metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + metadata->default_sort_order_id = kUnsortedOrderId; metadata->sort_orders.push_back(SortOrder::Unsorted()); metadata->next_row_id = TableMetadata::kInitialRowId; metadata->partition_specs.push_back(std::move(spec)); @@ -160,8 +160,8 @@ class UpdatePartitionSpecTest : public ::testing::TestWithParam { // Helper to create an expected partition spec std::shared_ptr MakeExpectedSpec( const std::vector& fields, int32_t last_assigned_field_id) { - auto spec_result = PartitionSpec::Make(PartitionSpec::kInitialSpecId, fields, - last_assigned_field_id); + auto spec_result = + PartitionSpec::Make(kInitialSpecId, fields, last_assigned_field_id); if (!spec_result.has_value()) { ADD_FAILURE() << "Failed to create expected spec: " << spec_result.error().message; return nullptr; @@ -535,7 +535,7 @@ TEST_P(UpdatePartitionSpecTest, TestAddDeletedName) { ICEBERG_UNWRAP_OR_FAIL( auto expected_spec, PartitionSpec::Make( - PartitionSpec::kInitialSpecId, + kInitialSpecId, std::vector{ PartitionField(3, 1000, "category", Transform::Identity()), PartitionField(2, 1001, "ts_day", Transform::Day()), @@ -547,7 +547,7 @@ TEST_P(UpdatePartitionSpecTest, TestAddDeletedName) { ICEBERG_UNWRAP_OR_FAIL( auto expected_spec, PartitionSpec::Make( - PartitionSpec::kInitialSpecId, + kInitialSpecId, std::vector{ PartitionField(3, 1000, "category", Transform::Identity()), PartitionField(2, 1001, "ts_day", Transform::Day())}, @@ -757,7 +757,7 @@ TEST_P(UpdatePartitionSpecTest, TestRemoveAndAddMultiTimes) { } else { ICEBERG_UNWRAP_OR_FAIL( auto expected_spec, - PartitionSpec::Make(PartitionSpec::kInitialSpecId, + PartitionSpec::Make(kInitialSpecId, std::vector{ PartitionField(2, 1000, "ts_date", Transform::Day()), PartitionField(2, 1001, "__ts_date", Transform::Month())}, diff --git a/src/iceberg/update/update_partition_spec.cc b/src/iceberg/update/update_partition_spec.cc index 2be6a59a5..4ff89d844 100644 --- a/src/iceberg/update/update_partition_spec.cc +++ b/src/iceberg/update/update_partition_spec.cc @@ -63,7 +63,7 @@ UpdatePartitionSpec::UpdatePartitionSpec(std::shared_ptr ctx schema_ = std::move(schema_result.value()); last_assigned_partition_id_ = - std::max(base().last_partition_id, PartitionSpec::kLegacyPartitionDataIdStart - 1); + std::max(base().last_partition_id, kLegacyPartitionDataIdStart - 1); name_to_field_ = IndexSpecByName(*spec_); transform_to_field_ = IndexSpecByTransform(*spec_);