fix

Author: lucasfang

src/paimon/core/operation/commit/overwrite_changes_provider.cpp

@@ -16,10 +16,8 @@
 
 #include "paimon/core/operation/commit/overwrite_changes_provider.h"
 
-#include <unordered_set>
 #include <utility>
 
-#include "paimon/core/manifest/file_entry.h"
 #include "paimon/core/manifest/file_kind.h"
 
 namespace paimon {
@@ -46,21 +44,12 @@ Result<std::shared_ptr<CommitChanges>> OverwriteChangesProvider::Provide(
 
     PAIMON_ASSIGN_OR_RAISE(std::vector<ManifestEntry> entries,
                            manifest_scan_(latest_snapshot.value()));
-    std::unordered_set<FileEntry::Identifier> existing_identifiers;
-    existing_identifiers.reserve(entries.size());
     for (const auto& entry : entries) {
-        existing_identifiers.insert(entry.CreateIdentifier());
         delta_files.emplace_back(FileKind::Delete(), entry.Partition(), entry.Bucket(),
                                  entry.TotalBuckets(), entry.File());
     }
 
-    for (const auto& change : changes_) {
-        if (change.Kind() == FileKind::Add() &&
-            existing_identifiers.find(change.CreateIdentifier()) != existing_identifiers.end()) {
-            continue;
-        }
-        delta_files.push_back(change);
-    }
+    delta_files.insert(delta_files.end(), changes_.begin(), changes_.end());
 
     PAIMON_ASSIGN_OR_RAISE(std::vector<IndexManifestEntry> previous_index_entries,
                            index_scan_(latest_snapshot.value()));

src/paimon/core/operation/commit/overwrite_changes_provider_test.cpp

@@ -138,7 +138,7 @@ TEST(OverwriteChangesProviderTest, TestProvideWithoutLatestSnapshotUsesChangesOn
     EXPECT_EQ("index-new", provided_index_entries[0].index_file->FileName());
 }
 
-TEST(OverwriteChangesProviderTest, TestProvideWithLatestSnapshotAddsDeletesAndDeduplicatesAdds) {
+TEST(OverwriteChangesProviderTest, TestProvideWithLatestSnapshotAddsDeletesAndAppendsAllChanges) {
     std::vector<ManifestEntry> changes = {
         CreateManifestEntry("existing-a", FileKind::Add(), /*partition_value=*/1),
         CreateManifestEntry("new-b", FileKind::Add(), /*partition_value=*/1),
@@ -167,16 +167,19 @@ TEST(OverwriteChangesProviderTest, TestProvideWithLatestSnapshotAddsDeletesAndDe
 
     ASSERT_TRUE(changelog_files.empty());
 
-    // delta = delete(existing-a), delete(existing-x), add(new-b), delete(force-delete-c)
-    ASSERT_EQ(4u, delta_files.size());
+    // delta = delete(existing-a), delete(existing-x),
+    //         add(existing-a), add(new-b), delete(force-delete-c)
+    ASSERT_EQ(5u, delta_files.size());
     EXPECT_TRUE(delta_files[0].Kind() == FileKind::Delete());
     EXPECT_EQ("existing-a", delta_files[0].FileName());
     EXPECT_TRUE(delta_files[1].Kind() == FileKind::Delete());
     EXPECT_EQ("existing-x", delta_files[1].FileName());
     EXPECT_TRUE(delta_files[2].Kind() == FileKind::Add());
-    EXPECT_EQ("new-b", delta_files[2].FileName());
-    EXPECT_TRUE(delta_files[3].Kind() == FileKind::Delete());
-    EXPECT_EQ("force-delete-c", delta_files[3].FileName());
+    EXPECT_EQ("existing-a", delta_files[2].FileName());
+    EXPECT_TRUE(delta_files[3].Kind() == FileKind::Add());
+    EXPECT_EQ("new-b", delta_files[3].FileName());
+    EXPECT_TRUE(delta_files[4].Kind() == FileKind::Delete());
+    EXPECT_EQ("force-delete-c", delta_files[4].FileName());
 
     // index = provided new + delete old
     ASSERT_EQ(2u, provided_index_entries.size());

src/paimon/core/operation/file_store_commit_impl_test.cpp

@@ -2108,11 +2108,9 @@ TEST_F(FileStoreCommitImplTest, TestOverwriteWithSameFile) {
     ASSERT_TRUE(IsStringInSet(file_names, "data-6828284c-e707-49b5-af6b-69be79af120c-0.orc"));
     ASSERT_TRUE(IsStringInSet(file_names, "data-8dc7f04c-3c98-48b2-9d56-834d746c4a40-0.orc"));
 
-    // same file delete, then add, file will also be removed in result
-    ASSERT_OK(commit_impl->Overwrite({}, msgs1, 2));
-    ASSERT_OK_AND_ASSIGN(auto snapshot2, commit_impl->snapshot_manager_->LatestSnapshot());
-    ASSERT_OK_AND_ASSIGN(auto entries2, commit_impl->GetAllFiles(snapshot2.value(), {}));
-    ASSERT_EQ(0u, entries2.size());
+    // Java parity: overwrite provider adds DELETE(old) + ADD(newChanges) without dedup,
+    // so same-file overwrite fails on duplicate add.
+    ASSERT_NOK_WITH_MSG(commit_impl->Overwrite({}, msgs1, 2), "Trying to add file");
 }
 
 TEST_F(FileStoreCommitImplTest, TestAppendDiscardDuplicateFiles) {