Skip to content

[fix](paimon) Accept required Variant array fallback value - #66626

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
Gabriel39:fix/doris-27886-paimon-required-array-value
Aug 11, 2026
Merged

[fix](paimon) Accept required Variant array fallback value#66626
yiguolei merged 1 commit into
apache:branch-4.1from
Gabriel39:fix/doris-27886-paimon-required-array-value

Conversation

@Gabriel39

Copy link
Copy Markdown
Contributor

Problem

Paimon can write an unannotated shredded Variant array whose element wrapper contains only a required BYTE_ARRAY value fallback leaf. Doris already recognizes Paimon's optional wrapper groups, but rejected this required fallback leaf during Parquet schema validation.

Changes

  • Allow a required fallback leaf only for a Paimon unannotated, fallback-only array element.
  • Keep annotated Variant layouts, object wrappers, and array elements with a typed carrier under the existing strict validation rules.
  • Verify that the non-nullable fallback leaf is materialized into Variant array elements.

Validation

  • ASAN BE unit tests: ParquetSchemaTest.*, VariantColumnReaderTest.*, and PaimonVariantReaderTest.* (82 tests passed).
  • clang-format 16 validation on all affected C/C++ files.
  • git diff --check.

Jira: DORIS-27886

Allow the unannotated Paimon Variant override to accept a required BYTE_ARRAY fallback leaf only for fallback-only array elements. Keep the standard annotated layout and all other shredded wrappers strict.
Add schema boundary tests and verify materialization from a non-nullable fallback leaf.
Jira: DORIS-27886
@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
ContributorAuthor

run buildall

@Gabriel39

Copy link
Copy Markdown
ContributorAuthor

/review

@github-actionsgithub-actionsBot 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.

Automated review opinion: approval. I found no blocking correctness issue in the reviewed diff.

Critical checkpoints:

  • External compatibility and scope: the relaxation is reachable only through the explicit Paimon table-schema Variant override. It admits the unannotated fallback-only array-element layout with a required BYTE_ARRAY value, while annotated Parquet Variant groups, object-field wrappers, and array elements that also have typed_value retain strict validation.
  • Schema, nested-level, and materialization correctness: table/file/leaf identities are unchanged. The optional array-element wrapper and required payload remain in their existing repetition/definition-level domains; valid payloads reconstruct the Variant element, while null or missing array wrappers still return corruption. No decoder cursor, lifecycle, cache, resource-ownership, or hot-path performance behavior changes.
  • Negative compatibility cases: focused tests preserve rejection for a required object fallback, a required fallback beside a typed carrier, and the same required fallback in an annotated Variant layout.
  • Tests: the new schema test reaches the formerly rejected validation branch, and the materialization test validates the reconstructed array values. Existing nested sparse-filter/cross-page coverage exercises the same level-domain behavior. A generated Paimon-file end-to-end case for this exact layout, including null/empty arrays and small page/batch limits, would be useful additional hardening but is not blocking based on the traced production path.
  • Review focus and convergence: no additional user focus was supplied. The complete four-file PR was reviewed; both the normal full-review and separate risk-focused Round 1 scans returned NO_NEW_VALUABLE_FINDINGS, and no existing inline thread required duplicate suppression.

This review-only runner did not execute builds or tests; the conclusion is based on the authoritative diff, caller/implementation/test tracing, and the PR-reported validation.

@Gabriel39
Gabriel39 marked this pull request as ready for review August 10, 2026 16:46
@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (20/20) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.77% (30544/41407)
Line Coverage57.82% (336807/582547)
Region Coverage54.68% (280168/512405)
Branch Coverage55.61% (125607/225868)

@yiguolei
yiguolei merged commit bfa2324 into apache:branch-4.1Aug 11, 2026
33 of 35 checks passed
suxiaogang223 added a commit to suxiaogang223/doris that referenced this pull request Sep 3, 2026
### What problem does this PR solve?
Issue Number: closeapache#65086
Related PR: apache#66334, apache#66547, apache#66626
Problem Summary: Forward-port Variant V2 decoding across the Paimon Java scanner and native JNI bridge. The reader propagates nested access paths, materializes encoded Variant bytes into ColumnVariantV2, and accepts Paimon required-value leaves for fallback-only Variant arrays.
### Release note
Improve Paimon Variant reads with nested projection and native Variant V2 materialization.
### Check List (For Author)
- Test: Unit Test
- Added Java vector/projection and BE Parquet schema/reader coverage; BE format/check, header hygiene, and FE checkstyle passed. Final compilation and test execution are deferred until all forward-port picks are complete.
- Behavior changed: Yes, Paimon Variant reads support nested projection and fallback-array schemas.
- Does this need documentation: No
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Gabriel39@hello-stephen@yiguolei