fix test failure

Author: gty404

src/iceberg/partition_spec.cc

@@ -187,11 +187,10 @@ Status PartitionSpec::ValidatePartitionName(const Schema& schema,
   std::unordered_set<std::string> partition_names;
   for (const auto& partition_field : spec.fields()) {
     auto name = std::string(partition_field.name());
-    ICEBERG_PRECHECK(!name.empty(), "Cannot use empty partition name: {}", name);
+    ICEBERG_CHECK(!name.empty(), "Cannot use empty partition name: {}", name);
 
-    if (partition_names.contains(name)) {
-      return InvalidArgument("Cannot use partition name more than once: {}", name);
-    }
+    ICEBERG_PRECHECK(!partition_names.contains(name),
+                     "Cannot use partition name more than once: {}", name);
     partition_names.insert(name);
 
     ICEBERG_ASSIGN_OR_RAISE(auto schema_field, schema.FindFieldByName(name));
@@ -202,17 +201,15 @@ Status PartitionSpec::ValidatePartitionName(const Schema& schema,
       // field name as long as they are sourced from the same schema field
       if (schema_field.has_value() &&
           schema_field.value().get().field_id() != partition_field.source_id()) {
-        return InvalidArgument(
+        return ValidationFailed(
             "Cannot create identity partition sourced from different field in schema: {}",
             name);
       }
     } else {
       // for all other transforms we don't allow conflicts between partition name and
       // schema field name
-      if (schema_field.has_value()) {
-        return InvalidArgument(
-            "Cannot create partition from name that exists in schema: {}", name);
-      }
+      ICEBERG_CHECK(!schema_field.has_value(),
+                    "Cannot create partition from name that exists in schema: {}", name);
     }
   }
 
@@ -276,9 +273,9 @@ Status PartitionSpec::ValidateRedundantPartitions(const PartitionSpec& spec) {
     // Check if this dedup key already exists
     // If it does, we have found a redundant partition field
     auto existing_field_iter = dedup_fields.find(dedup_key);
-    ICEBERG_PRECHECK(existing_field_iter == dedup_fields.end(),
-                     "Cannot add redundant partition: {} conflicts with {}",
-                     field.ToString(), existing_field_iter->second->ToString());
+    ICEBERG_CHECK(existing_field_iter == dedup_fields.end(),
+                  "Cannot add redundant partition: {} conflicts with {}",
+                  field.ToString(), existing_field_iter->second->ToString());
 
     // Add this field to the dedup map for future conflict detection
     dedup_fields[dedup_key] = &field;

src/iceberg/test/update_partition_spec_test.cc

@@ -729,39 +729,41 @@ TEST_P(UpdatePartitionSpecTest, TestRemoveAndAddMultiTimes) {
   update3->AddField(Expressions::Day("ts"), "ts_date");
   auto add_second_time_spec = ApplyUpdateAndGetSpec(update3);
 
-  // Remove second time
-  auto table3 = CreateTableWithSpec(add_second_time_spec, "test_table3");
-  auto update4 = CreateUpdateFromTable(table3);
-  update4->RemoveField(Expressions::Day("ts"));
-  auto remove_second_time_spec = ApplyUpdateAndGetSpec(update4);
-
   // Add third time with month
-  auto table4 = CreateTableWithSpec(remove_second_time_spec, "test_table4");
+  auto table4 = CreateTableWithSpec(add_second_time_spec, "test_table4");
   auto update5 = CreateUpdateFromTable(table4);
-  update5->AddField(Expressions::Month("ts"));
+  update5->AddField(Expressions::Month("ts"), "ts_month");
   auto add_third_time_spec = ApplyUpdateAndGetSpec(update5);
 
   // Rename ts_month to ts_date
   auto table5 = CreateTableWithSpec(add_third_time_spec, "test_table5");
   auto update6 = CreateUpdateFromTable(table5);
-  update6->RenameField("ts_month", "ts_date");
+  update6->RenameField("ts_month", "__ts_date");
   auto updated_spec = ApplyUpdateAndGetSpec(update6);
 
   if (format_version_ == 1) {
+    // Remove second time
+    auto table3 = CreateTableWithSpec(add_second_time_spec, "test_table3");
+    auto update4 = CreateUpdateFromTable(table3);
+    update4->RemoveField(Expressions::Day("ts"));
+    ExpectError(update4, ErrorKind::kValidationFailed,
+                "Cannot add redundant partition: ts_date");
+
     ASSERT_EQ(updated_spec->fields().size(), 3);
     // In V1, we expect void transforms for deleted fields
     EXPECT_TRUE(updated_spec->fields()[0].name().find("ts_date") == 0);
     EXPECT_TRUE(updated_spec->fields()[1].name().find("ts_date") == 0);
-    EXPECT_EQ(updated_spec->fields()[2].name(), "ts_date");
+    EXPECT_EQ(updated_spec->fields()[2].name(), "__ts_date");
     EXPECT_EQ(*updated_spec->fields()[0].transform(), *Transform::Void());
-    EXPECT_EQ(*updated_spec->fields()[1].transform(), *Transform::Void());
+    EXPECT_EQ(*updated_spec->fields()[1].transform(), *Transform::Day());
     EXPECT_EQ(*updated_spec->fields()[2].transform(), *Transform::Month());
   } else {
     ICEBERG_UNWRAP_OR_FAIL(
         auto expected_spec,
         PartitionSpec::Make(PartitionSpec::kInitialSpecId,
                             std::vector<PartitionField>{
-                                PartitionField(2, 1000, "ts_date", Transform::Month())},
+                                PartitionField(2, 1000, "ts_date", Transform::Day()),
+                                PartitionField(2, 1001, "__ts_date", Transform::Month())},
                             1000));
     auto expected = std::shared_ptr<PartitionSpec>(expected_spec.release());
     AssertPartitionSpecEquals(*expected, *updated_spec);