Skip to content

[fix](be) Fill skipped map side during lazy pruned reads#65344

Closed
HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:fix-doris-26821-map-lazy-fill
Closed

[fix](be) Fill skipped map side during lazy pruned reads#65344
HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:fix-doris-26821-map-lazy-fill

Conversation

@HappenLee

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Lazy pruned complex-column reads can materialize only KEYS or VALUES for a MAP nested under a STRUCT when predicate evaluation reads another path. The skipped MAP side stayed empty in the lazy phase, so ColumnMap offsets no longer matched key/value column sizes. This patch fills defaults for a skipped MAP child according to the element count while leaving predicate-materialized children untouched.

Release note

Fix MAP nested column pruning with lazy materialization when only map keys or values are projected.

Check List (For Author)

  • Test: Unit Test: ./run-be-ut.sh -j 48 --run --filter=ColumnReaderTest.MapNextBatchLazyFillsSkippedKeyOrValueColumn:ColumnReaderTest.MapReadByRowidsLazyFillsSkippedKeyOrValueColumn
  • Test: Regression test: ./run-regression-test.sh --run --conf /tmp/doris_26821_regression-conf.groovy -d datatype_p0/complex_types -s test_pruned_struct_map_lazy
  • Test: Static analysis: CLANG_TIDY_BINARY=/mnt/disk6/common/ldb_toolchain_028/bin/clang-tidy build-support/run-clang-tidy.sh --files be/src/storage/segment/column_reader.cpp be/test/storage/segment/column_reader_test.cpp --build-dir be/ut_build_ASAN
  • Behavior changed: Yes. MAP nested lazy materialization now keeps key/value child sizes aligned when one side is pruned.
  • Does this need documentation: No

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Lazy pruned complex-column reads can materialize only KEYS or VALUES for a MAP nested under a STRUCT when predicate evaluation reads another path. The skipped MAP side stayed empty in the lazy phase, so ColumnMap offsets no longer matched key/value column sizes. This patch fills defaults for a skipped MAP child according to the element count while leaving predicate-materialized children untouched.

### Release note

Fix MAP nested column pruning with lazy materialization when only map keys or values are projected.

### Check List (For Author)

- Test: Unit Test: ./run-be-ut.sh -j 48 --run --filter=ColumnReaderTest.MapNextBatchLazyFillsSkippedKeyOrValueColumn:ColumnReaderTest.MapReadByRowidsLazyFillsSkippedKeyOrValueColumn
- Test: Regression test: ./run-regression-test.sh --run --conf /tmp/doris_26821_regression-conf.groovy -d datatype_p0/complex_types -s test_pruned_struct_map_lazy
- Test: Static analysis: CLANG_TIDY_BINARY=/mnt/disk6/common/ldb_toolchain_028/bin/clang-tidy build-support/run-clang-tidy.sh --files be/src/storage/segment/column_reader.cpp be/test/storage/segment/column_reader_test.cpp --build-dir be/ut_build_ASAN
- Behavior changed: Yes. MAP nested lazy materialization now keeps key/value child sizes aligned when one side is pruned.
- Does this need documentation: No
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@HappenLee

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes for one correctness issue in lazy-pruned map materialization.

Critical checkpoint conclusions:

  • Goal/test proof: the PR aims to keep map key/value child sizes aligned when one side is pruned during lazy reads. The new tests cover predicates on sibling struct fields, but not a reachable map-level metadata predicate such as map_size(m) or cardinality(m) combined with map_keys(m) / map_values(m).
  • Scope: the code change is focused on map child materialization in next_batch() and read_by_rowids().
  • Concurrency/lifecycle: no new shared locking or thread lifecycle is introduced. The relevant lifecycle is predicate-phase placeholder/default materialization followed by lazy-phase recovery on the same filtered ColumnMap; the accepted issue is in that lifecycle.
  • Configuration/compatibility: no new config, wire field, or storage-format compatibility change.
  • Parallel paths: both sequential and rowid map paths were updated. The accepted issue is specifically on the lazy-pruned rowid recovery path after predicate-phase metadata reads.
  • Special conditions: the new helper distinguishes SKIP, PREDICATE, and LAZY_OUTPUT; the SKIP branch is unsafe when the skipped side was already default-filled during predicate phase.
  • Test coverage/results: BE unit tests and one regression were added, but coverage misses the map-level OFFSET predicate plus lazy key/value projection case. I could not run BE/regression tests in this runner because thirdparty/installed and thirdparty/installed/bin/protoc are absent. build-support/check-format.sh also could not run because the available clang-format is 18.1.3, while Doris requires v16.
  • Observability, persistence, data writes, FE/BE variables, and transactionality: not materially changed by this PR.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions: optimizer-rewrite proposed OR-1, which I independently verified and accepted as inline comment M-1. tests-session-config reported no new valuable findings. In convergence round 1, both live subagents reviewed the final ledger/comment set and replied NO_NEW_VALUABLE_FINDINGS.

}

if (iterator->read_requirement() == ColumnIterator::ReadRequirement::SKIP) {
dst->insert_many_defaults(item_count);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With a map-level metadata predicate this SKIP branch can append the skipped side twice. For example, with nested pruning enabled, SELECT map_keys(m) FROM t WHERE map_size(m) > 0 reaches BE as a map iterator that is PREDICATE, with the key child LAZY_OUTPUT and the value child SKIP (map_size emits an OFFSET predicate path, while map_keys emits a KEYS output path). The predicate-phase next_batch() reads the map offsets and this line fills default values for every map item. After filtering, _read_lazy_pruned_columns() calls read_by_rowids() on the same ColumnMap in LAZY phase; need_to_read_meta_columns() is false, but the skipped value child still hits this branch and appends another item_count defaults. The key side clears its placeholder and reads the selected keys, so the key/value child sizes no longer match each other or the offsets. Please distinguish the already-filled predicate-phase case from the whole-map-placeholder case, or only add the missing default elements during lazy recovery.

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.

2 participants