[opt](lance) reduce redundant work in LanceTableReader - #68335
zhangstar333 wants to merge 2 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
Static review of PR #68335 at head c64d2d7 (base 70bf96e).
Two actionable regressions are called out inline:
- Nested registered-extension arrays: the new normalizer compacts a sliced parent before unwrapping extension children, so Arrow 24 cannot build list/struct parents containing ExtensionType children; the existing sliced nested JSON regression path fails.
- Row-ID/take_rows lifecycle: scanner profile counters are now lazy, so scanner-less phase-two fetches skip Lance data-cache byte statistics that were previously created and collected.
Review checkpoints:
- All ten authoritative changed paths were swept, including the converter extraction, nested normalization, scanner projection, generated search-result columns, global row IDs, runtime-filter/schema gating, close/reset paths, and test changes.
- Projection, global-rowid, runtime-filter, normal scanner reuse, build integration, and other candidate concerns were independently checked; no additional actionable issue was substantiated.
- Existing inline review comments were empty at the final duplicate-fence check, and the live base/head matched the authoritative bundle.
- No user-provided focus points were supplied beyond the complete-review requirement.
- This is a static review only: no builds or tests were run, so author/CI claims are not independent validation.
There was a problem hiding this comment.
Static review of PR #68335 at head c64d2d7 (base 70bf96e).
Two actionable regressions are called out inline:
- Nested registered-extension arrays: the new normalizer compacts a sliced parent before unwrapping extension children, so Arrow 24 cannot build list/struct parents containing ExtensionType children; the existing sliced nested JSON regression path fails.
- Row-ID/take_rows lifecycle: scanner profile counters are now lazy, so scanner-less phase-two fetches skip Lance data-cache byte statistics that were previously created and collected.
Review checkpoints:
- All ten authoritative changed paths were swept, including the converter extraction, nested normalization, scanner projection, generated search-result columns, global row IDs, runtime-filter/schema gating, close/reset paths, and test changes.
- Projection, global-rowid, runtime-filter, normal scanner reuse, build integration, and other candidate concerns were independently checked; no additional actionable issue was substantiated.
- Existing inline review comments were empty at the final duplicate-fence check, and the live base/head matched the authoritative bundle.
- No user-provided focus points were supplied beyond the complete-review requirement.
- This is a static review only: no builds or tests were run, so author/CI claims are not independent validation.
There was a problem hiding this comment.
Static review of PR #68335 at head c64d2d7 (base 70bf96e).
Two actionable regressions are called out inline:
- Nested registered-extension arrays: the new normalizer compacts a sliced parent before unwrapping extension children, so Arrow 24 cannot build list/struct parents containing ExtensionType children; the existing sliced nested JSON regression path fails.
- Row-ID/take_rows lifecycle: scanner profile counters are now lazy, so scanner-less phase-two fetches skip Lance data-cache byte statistics that were previously created and collected.
Review checkpoints:
- All ten authoritative changed paths were swept, including the converter extraction, nested normalization, scanner projection, generated search-result columns, global row IDs, runtime-filter/schema gating, close/reset paths, and test changes.
- Projection, global-rowid, runtime-filter, normal scanner reuse, build integration, and other candidate concerns were independently checked; no additional actionable issue was substantiated.
- Existing inline review comments were empty at the final duplicate-fence check, and the live base/head matched the authoritative bundle.
- No user-provided focus points were supplied beyond the complete-review requirement.
- This is a static review only: no builds or tests were run, so author/CI claims are not independent validation.
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Problem Summary:
optimize LanceTableReader by lazily initializing schemas and scanner metrics, reusing the record batch converter, caching Arrow normalization plans, and avoiding unnecessary full-column reads. Also improve code structure by extracting schema import and record batch conversion logic.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)