You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "liujiayi771 (via GitHub)" <gi...@apache.org> on 2026/08/09 01:40:18 UTC

[PR] feat(commit): support dropping delete file stats [paimon-cpp]

liujiayi771 opened a new pull request, #190:
URL: https://github.com/apache/paimon-cpp/pull/190

   ### Purpose
   
   Linked issue: N/A
   
   This is a follow-up to #187 and adds full support for `manifest.delete-file-drop-stats=true` in the C++ commit path.
   
   - Parse and expose the existing Paimon table option.
   - Drop `value_stats` and `value_stats_cols` from final DELETE manifest entries.
   - Apply the optimization after scan filtering and pruning for overwrite and append compaction paths.
   - Preserve complete statistics for ADD entries, generic metadata scans, and writer restore.
   - Keep writer restore statistics until the final entry kind is known, intentionally avoiding the metadata-only level-upgrade bug tracked in https://github.com/apache/paimon/issues/7026.
   
   ### Tests
   
   - `cmake --build build --target paimon_static paimon_shared -j8`
   - `cmake --build build --target paimon-core-test -j8`
   - `./build/debug/paimon-core-test --gtest_brief=1` (1629 tests passed)
   - `pre-commit run --files <changed files>`
   - `git diff --check apache/main...HEAD`
   
   ### API and Format
   
   Adds the public option key `kManifestDeleteFileDropStats`.
   
   There is no manifest schema or protocol change. When enabled, DELETE entries store empty value statistics and an empty value-statistics column list, matching the existing Paimon format. ADD entries retain complete statistics for file skipping.
   
   ### Documentation
   
   This implements an existing Paimon table option. No documentation files are included in this PR.
   
   ### Generative AI tooling
   
   Generated-by: Codex (GPT-5) and Claude Code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(commit): support dropping delete file stats [paimon-cpp]

Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #190:
URL: https://github.com/apache/paimon-cpp/pull/190#discussion_r3742558029


##########
include/paimon/defs.h:
##########
@@ -173,6 +173,10 @@ struct PAIMON_EXPORT Options {
     /// compaction of manifest, default value is 16MB.
     static const char MANIFEST_FULL_COMPACTION_FILE_SIZE[];
 
+    /// "manifest.delete-file-drop-stats" - Whether final DELETE manifest entries should omit
+    /// value statistics. Default is false.
+    static const char MANIFEST_DELETE_FILE_DROP_STATS[];

Review Comment:
   Also, could you complete the comment here? I noticed that the Java version also includes: “Default value is false only for compatibility with old readers.”



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(commit): support dropping delete file stats [paimon-cpp]

Posted by "liujiayi771 (via GitHub)" <gi...@apache.org>.
liujiayi771 commented on PR #190:
URL: https://github.com/apache/paimon-cpp/pull/190#issuecomment-5229542048

   cc @lucasfang @lxy-9602


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(commit): support dropping delete file stats [paimon-cpp]

Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #190:
URL: https://github.com/apache/paimon-cpp/pull/190#discussion_r3742538685


##########
src/paimon/core/operation/abstract_file_store_write.cpp:
##########
@@ -302,6 +302,10 @@ Result<std::shared_ptr<RestoreFiles>> AbstractFileStoreWrite::ScanExistingFileMe
     if (dv_maintainer_factory_) {
         index_file_handler = dv_maintainer_factory_->GetIndexFileHandler();
     }
+    // Paimon Java currently drops value stats during writer restore. This is a known bug: a
+    // restored file can become a compact-after ADD via metadata-only level upgrade and lose its
+    // stats (https://github.com/apache/paimon/issues/7026). C++ intentionally does not align with
+    // that behavior; stats are dropped later only when the final entry kind is DELETE.

Review Comment:
   Nice catch! Could you please add a PK compaction end-to-end test in `pk_compaction_inte_test.cpp` to demonstrate that level upgrades do not lose their stats?



##########
src/paimon/core/operation/commit/manifest_entry_changes.h:
##########
@@ -39,7 +39,11 @@ namespace paimon {
 /// Detailed changes from `CommitMessage`s.
 class ManifestEntryChanges {
  public:
-    explicit ManifestEntryChanges(int32_t default_num_bucket);
+    /// Create a change collector.
+    ///
+    /// @param default_num_bucket Bucket count used when a commit message omits it.
+    /// @param drop_delete_file_stats Whether DELETE data-file entries should omit value stats.
+    explicit ManifestEntryChanges(int32_t default_num_bucket, bool drop_delete_file_stats = false);
 

Review Comment:
   Could we avoid using a default argument here? In production code, it’s better to pass the parameter explicitly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(commit): support dropping delete file stats [paimon-cpp]

Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas merged PR #190:
URL: https://github.com/apache/paimon-cpp/pull/190


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org