Skip to content

[fix](parquet) Handle nested nullability in predicate scans - #66799

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
Gabriel39:fix/doris-27887-parquet-nested-nullability
Aug 17, 2026
Merged

[fix](parquet) Handle nested nullability in predicate scans#66799
yiguolei merged 1 commit into
apache:branch-4.1from
Gabriel39:fix/doris-27887-parquet-nested-nullability

Conversation

@Gabriel39

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: DORIS-27887

Problem Summary:

On branch-4.1, a Parquet predicate scan can compare a reader type such as a struct with nullable descendants against a block type with nullability represented at a different nesting level. The existing debug check removes only the outer nullable wrapper and aborts the BE even though the recursive type and shape are compatible.

This change compares Array, Map, and Struct types recursively while ignoring only nullability at each nesting level. Primitive type and complex-type shape mismatches remain rejected.

This is a narrow backport of the relevant type-compatibility fix already present on master; unrelated changes are intentionally excluded.

Release note

Fix a BE crash when Parquet nested predicate columns use equivalent types with different nested nullability representations.

Check List (For Author)

  • Test: Unit Test
    • Added a focused test for equivalent nested Struct nullability and an incompatible nested primitive type.
    • Ran the focused Parquet scan BE unit test.
  • Behavior changed: No. The change prevents a debug assertion for semantically compatible nested types.
  • Does this need documentation: No.

@Gabriel39
Gabriel39 requested a review from yiguolei as a code owner August 15, 2026 14:35
@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?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

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.

Review result: no actionable findings; comment-only approval opinion.

Critical checkpoints:

  • Schema and type safety: the recursive comparator removes only Nullable wrappers at each Array, Map, or Struct node. It preserves Struct child count/order, both Map branches, and exact concrete equality for every leaf, including parameterized types.
  • Predicate and materialization correctness: this changes only a DCHECK compatibility condition. Native readers still inspect the actual destination column and physical schema, scalar leaves retain exact type checks, selection/compaction cardinality remains validated, and TableReader still aligns and validates nested null maps recursively.
  • Lifecycle, performance, and observability: no cursor, EOF, cleanup, ownership, cache, I/O, profile-counter, or release hot-path behavior changes. The recursive traversal runs only in debug/test assertion evaluation.
  • Compatibility and interface boundaries: the implementation is the narrow backport of the helper already present on master. The detail declaration/definition links through the existing Format test target, and no TableReader/FileReader/ColumnMapper ownership boundary is changed.
  • Tests: the focused positive/negative Struct helper test pins the broadened equality condition. A remaining coverage limitation is that it does not reproduce the original predicate-scheduler abort end to end or explicitly exercise Array, Map, and deeper compositions. Existing tests cover the surrounding nested-predicate, selection, and TableReader materialization paths, so I do not consider this blocking, but a focused scan-path regression would strengthen the fix.
  • User focus: no additional review focus was provided; the full PR was reviewed.
  • Completion: two review rounds converged with no new valuable findings, and the final changed-file/candidate sweep found no unresolved or accepted inline issues.

No local build or test was run, as required by this review runner. Normal CI remains authoritative for compilation and test execution.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 70.73% (29/41) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 73.91% (30819/41698)
Line Coverage 58.02% (341083/587822)
Region Coverage 54.95% (284344/517493)
Branch Coverage 55.87% (128011/229138)

@yiguolei
yiguolei merged commit f651e99 into apache:branch-4.1 Aug 17, 2026
30 of 33 checks passed
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.

3 participants