[fix](meta path) Preserve metadata access path type for map_entries - #68040
Merged
Merged
Conversation
linrrzqqq
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
September 16, 2026 00:46
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16774 ms |
Contributor
TPC-DS: Total hot run time: 82124 ms |
Contributor
ClickBench: Total hot run time: 14.67 s |
Collaborator
Author
|
/review |
Contributor
There was a problem hiding this comment.
Approval opinion: no blocking or correctness issues found.
Checkpoint conclusions:
- Goal and data correctness: the change preserves
ColumnAccessPathType.METAwhile translating the syntheticmap_entriesentry field toKEYSorVALUES. The loweredtransform_valuespath now reaches FE pruning and BEOFFSET_ONLYorNULL_MAP_ONLYreaders without treatingOFFSETorNULLas data fields, while the required map keys remain a DATA path. - Scope and focus: the production fix is one focused context-state copy. No additional user-provided focus was supplied; the full PR was reviewed.
- Tests: the FE tests require the exact META paths and reject the pre-fix same-spelling DATA paths. The regression cases enable nested pruning and cover populated, empty, and NULL maps, array offsets, and nullable Struct values with deterministic ordering. Coverage is present but was reviewed statically only; no local build or test was run under the review-runner contract.
- Concurrency and lifecycle: no shared mutable state, thread interaction, lock, or nontrivial lifecycle change is introduced.
CollectorContextis request-local, and all of its state is now preserved or deliberately translated. - Configuration and compatibility: no production configuration, persistence format, function symbol, or wire schema changes. Existing typed Thrift access paths are reused, and non-OLAP readers retain their safe META-to-DATA fallback. Rolling-upgrade behavior is unchanged.
- Parallel paths and conditions: all
MapEntries-based map-lambda lowerings share the corrected translation. DATA versus META remains enum-discriminated, so real fields namedNULLorOFFSETare not confused with metadata. Key-sideNULLandOFFSETrouting was also checked against nullable map-child encoding. - Error handling, observability, transactions, and writes: no new error boundary, transaction, persistence, or data-write path is added. Existing path validation and reader diagnostics remain applicable; no additional metric or log is needed for this local fix.
- Performance and other risks: the added enum assignment is negligible and restores the intended metadata-only read modes. No additional performance, memory, or correctness issue was substantiated.
Review completion: complete after one converged round. Both normal full-review passes and the separate risk-focused pass returned NO_NEW_VALUABLE_FINDINGS; all initial risk items were independently resolved, with no inline comments required.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
924060929
approved these changes
Sep 17, 2026
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:
When
transform_valuesoperates on physical nested Map columns with nested-column pruning enabled:MAP<..., ARRAY<...>>may fail becauseVALUES.OFFSETis incorrectly marked as a DATA access path.MAP<..., STRUCT<...>>may crash the BE becauseVALUES.NULLis incorrectly treated as a Struct data field, causing all physical Struct child iterators to be pruned.Root cause
AccessPathExpressionCollector.visitMapEntriescreates a newCollectorContextwhile translating access paths frommap_entries.The new context did not inherit the original
ColumnAccessPathType, so META paths such asOFFSETandNULLfell back to the default DATA type.What is changed?
Propagate the original access-path type to the new context in
visitMapEntries.Added regression coverage for:
transform_values((k, v) -> size(v), map<int, array<int>>)transform_values((k, v) -> v is null, map<int, struct<...>>)The tests verify that
VALUES.OFFSETandVALUES.NULLremain META access paths.