[test](regression) Add ORC reader V2 Hive coverage#65480
Open
xylaaaaa wants to merge 1 commit into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review result: request changes due to one test-coverage gap.
Critical checkpoint conclusions:
- Goal/test proof: this PR is intended to add ORC Reader V2 Hive coverage, but the transactional Hive ACID additions do not prove ScannerV2 coverage because those ranges still fall back to the old scanner.
- Scope/focus: the changes are limited to regression tests and are otherwise focused.
- Concurrency/lifecycle/data writes/persistence: no production code paths are changed by this PR.
- Configuration/session propagation: the accepted issue is specifically that setting
enable_file_scanner_v2=trueis insufficient fortransactional_hiveranges because scanner selection rejects that table format. - Parallel paths: transactional Hive has a separate old-scanner reader path, which is the reported coverage gap.
- Test coverage/results: static review only in this runner; the base commit is absent locally,
.worktree_initializedandthirdparty/installedare missing, and external Hive execution is not available here. - User focus: no additional user-provided review focus was present.
Subagent conclusions: optimizer-rewrite reported no optimizer/rewrite candidates. tests-session-config proposed TSC-001, accepted as the inline comment below. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.
xylaaaaa
force-pushed
the
codex/orc-v2-hive-regression
branch
from
eda21cf to
5bbdf3f
Compare
Contributor
|
run buildall |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Automated review found one blocking issue in the new regression coverage: the Hive ORC predicate assertions force ScannerV2 but rely on a SARG-success session guard that only the legacy ORC reader enforces.
Critical checkpoint conclusions:
- Goal/test proof: the PR is focused on ORC reader V2 Hive regression coverage, but the accepted inline issue means part of that coverage can pass without proving the intended V2 SARG/min-max path.
- Scope: the PR is test-only and otherwise narrow.
- Concurrency, lifecycle, persistence, transactionality, data writes, and FE-BE protocol compatibility: no runtime code is changed directly.
- Configuration/session propagation: the issue is specifically a session-variable coverage gap between old and V2 ORC reader paths.
- Parallel paths: the existing transactional Hive ScannerV2 thread is already known; this review comment covers a distinct non-transactional Hive ORC V2 case.
- Test results and style: GitHub PR patch whitespace validation passed. I did not run regression tests because this checkout lacks
thirdparty/installedand the external Hive environment. - User focus: no additional user-provided review focus was supplied.
Subagent conclusions: OPT-1 and TSC-1 were merged into the single inline comment. A possible profile-token pruning concern was dismissed because FE profile summaries record the original SQL text. Convergence round 1 ended with both optimizer-rewrite and tests-session-config replying NO_NEW_VALUABLE_FINDINGS for the exact final ledger/comment set.
| @@ -45,8 +58,66 @@ suite("test_hive_orc_predicate", "p0,external") { | |||
|
|
|||
| qt_predicate_null_aware_equal_in_rt """select * from table_a inner join table_b on table_a.age <=> table_b.age and table_b.id in (1,3) order by table_a.id;""" | |||
Contributor
There was a problem hiding this comment.
This still does not prove that ScannerV2 initialized an ORC SearchArgument. The suite forces enable_file_scanner_v2, and ordinary Hive ORC ranges are eligible for V2, but the guard you rely on here is enforced only by the legacy be/src/format/orc/vorc_reader.cpp path. In the V2 path, be/src/format_v2/orc/orc_reader.cpp::_init_search_argument_from_local_filters() just returns OK when has_pushdown is false and never checks check_orc_init_sargs_success. These assertions can therefore pass via normal row filtering even if the V2 SARG/min-max pruning path was not initialized. Please either make the V2 ORC reader honor this session variable, or assert V2-specific profile evidence such as UseScannerV2=true plus SearchArgument/stripe-pruning activity.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The refactored ORC reader lacked focused Hive external-table regression coverage for SARG conversion across null-safe, integer min/max, decimal, date, and timestamp predicates. It also lacked explicit Reader V2 coverage for lazy materialization result integrity, Hive ACID delete predicates, and virtual row-id planning. Add focused assertions and force FileScannerV2 in the relevant suites while preserving existing generated result files.
### Release note
None
### Check List (For Author)
- Test: Regression test
- Ran ./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile,test_transactional_hive,test_hive_topn_lazy_mat
- Verified all four suites load successfully; Hive external execution was not enabled in the local configuration
- Behavior changed: No
- Does this need documentation: No
xylaaaaa
force-pushed
the
codex/orc-v2-hive-regression
branch
from
5bbdf3f to
4f97c46
Compare
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29432 ms |
Contributor
TPC-DS: Total hot run time: 180139 ms |
Contributor
ClickBench: Total hot run time: 24.88 s |
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.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
This PR adds regression coverage for the refactored ORC reader V2 path on Hive external tables. It covers ORC SARG initialization for min/max pruning, null-safe predicates, decimal/date/timestamp predicates, lazy materialization profile metrics, delete predicate behavior on Hive ACID ORC tables, and virtual row id coverage through the existing TopN lazy materialization suite.
Release note
None
Check List (For Author)
./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile,test_transactional_hive,test_hive_topn_lazy_mat./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile./run-regression-test.sh --run -d external_table_p0/hive -s test_orc_lazy_mat_profile