Skip to content

fix(commit): allow disabled manifest delete stats option - #187

Merged
SteNicholas merged 1 commit into
apache:mainfrom
liujiayi771:fix/allow-false-manifest-delete-file-drop-stats
Aug 6, 2026
Merged

fix(commit): allow disabled manifest delete stats option#187
SteNicholas merged 1 commit into
apache:mainfrom
liujiayi771:fix/allow-false-manifest-delete-file-drop-stats

Conversation

@liujiayi771

@liujiayi771 liujiayi771 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Purpose

Allow the C++ commit path to accept manifest.delete-file-drop-stats=false. The disabled value matches the existing C++ behavior, which preserves statistics in DELETE manifest entries. The enabled value remains unsupported and rejected.

Tests

  • ./build-release/release/paimon-core-test --gtest_filter='FileStoreCommitImplTest.ValidateCommitOptionsAllowsDisabledManifestDeleteFileDropStats:FileStoreCommitImplTest.ValidateCommitOptionsRejectsUnsupportedOptions'
  • ./build-release/release/paimon-core-test --gtest_brief=1 (1579 tests passed)
  • pre-commit run --files src/paimon/core/operation/file_store_commit_impl.cpp src/paimon/core/operation/file_store_commit_impl_test.cpp

API and Format

No public API, storage format, or protocol changes.

Documentation

No documentation changes. This fixes validation of an existing option.

Generative AI tooling

Generated-by: Codex, GPT-5.

Parse manifest.delete-file-drop-stats and reject it only when enabled, because false matches the existing C++ commit behavior.

Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5
AI-Contributed/Feature: 8/8
AI-Contributed/UT: 6/6
@liujiayi771
liujiayi771 marked this pull request as ready for review August 6, 2026 02:37

@lucasfang lucasfang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@SteNicholas SteNicholas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@SteNicholas
SteNicholas merged commit 9329819 into apache:main Aug 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants