[fix](be) Keep Paimon JNI predicate compatibility#65328
Open
Gabriel39 wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Format V2 Paimon JNI reader only accepted scan-level paimon_predicate and rejected scan ranges that still carried the predicate in the legacy split-level field. This made the v2 path stricter than the v1 reader and could break rolling upgrades or older planning paths that still populate the split-level predicate. The fix centralizes predicate selection so scan-level predicate remains preferred, while v2 falls back to the split-level predicate when the scan-level value is missing or empty. The scanner params and validation now share the same selection rule.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- Added BE unit tests for scan-level priority, split-level fallback when scan-level is missing or empty, and clear failure when both locations are missing. Ran build-support/clang-format.sh. Attempted ./run-be-ut.sh --run --filter='PaimonJniReaderTest*', but local BE UT CMake configuration is blocked by incomplete thirdparty/installed dependencies after reaching missing Protobuf library.
- Behavior changed: Yes. Format V2 Paimon JNI reader now accepts legacy split-level paimon_predicate as a fallback for compatibility.
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review complete. I did not find a blocking issue worth an inline review comment in this PR.
Critical checkpoint conclusions:
- Goal and correctness: the change restores format-v2 Paimon JNI predicate compatibility by preferring scan-level
paimon_predicateand falling back to the legacy split-level field when the scan-level value is missing or empty.validate_scan_range()andbuild_scanner_params()now use the same selection rule. - Scope: the implementation is small and localized to the v2 Paimon JNI reader plus focused BE unit coverage.
- Parallel paths: v1 Paimon JNI already prefers scan-level predicate and falls back to the split-level field; the v2 path is now aligned. Current FE planning still sets scan-level predicate/options and serialized table at scan level, and Java
PaimonJniScannerconsumes the resultingpaimon_predicateparameter uniformly. - Concurrency/lifecycle: no new concurrency, global state, thread lifecycle, or lock behavior was introduced.
- Configuration/session propagation: no new config item was added. Existing scan-level
serialized_table, storage properties, and Paimon options propagation were checked aroundPaimonScanNodeandFileQueryScanNode. - Compatibility: the split-level predicate fallback is a compatibility improvement for older planning paths or rolling-upgrade scenarios that still carry the predicate in
TPaimonFileDesc. - Error handling: missing predicate still fails during scan-range validation, and the later scanner-param build asserts the same invariant after validation.
- Tests: the added BE unit tests cover scan-level precedence, missing scan-level fallback, empty scan-level fallback, and the missing-both error. The new test file is included by the BE test glob and BE UT intentionally compiles with
-fno-access-control, so the protected test helper access is consistent with local test conventions. - Validation limits: I did not run BE UT locally because this GitHub Actions checkout is missing
.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protoc; review validation was static.
User focus: no additional user-provided review focus was present.
Subagent conclusions:
optimizer-rewriteproposed OPT-001 about protected member access in the new test. I dismissed it with evidence becausedoris_be_testis compiled with-fno-access-controlandbe/test/AGENTS.mddocuments that BE-UT ignores access control.tests-session-configfound no new candidate and independently noted the same BE UT access-control configuration.- Convergence round 1 ended with both
optimizer-rewriteandtests-session-configreplyingNO_NEW_VALUABLE_FINDINGSfor the same ledger and empty proposed inline comment set.
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run performance |
Contributor
Author
|
run external |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Format V2 Paimon JNI reader now falls back to legacy split-level paimon_predicate when scan-level predicate is missing or empty. Added BE unit coverage for scan priority, legacy fallback, and missing predicate failure. Validation: build-support/clang-format.sh passed; ./run-be-ut.sh --run --filter=PaimonJniReaderTest* was attempted but local CMake is blocked by incomplete thirdparty/installed dependencies after missing Protobuf library.