[AURON #2472] Fix nullable struct field null propagation - #2473
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes incorrect null handling when extracting a child field from a nullable StructArray in the native engine: the child field now becomes null whenever the parent struct is null (matching vanilla Spark semantics), while keeping the zero-copy path when the parent struct has no nulls.
Changes:
- Combine parent struct and child field null bitmaps in
GetIndexedFieldExprusingNullBuffer::union. - Preserve zero-copy behavior by returning the child array directly when the parent struct has no nulls.
- Add coverage via a Rust unit test for parent/child null propagation and a Spark Parquet E2E query test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| native-engine/datafusion-ext-exprs/src/get_indexed_field.rs | Propagates parent struct nulls to the selected child field and adds Rust unit tests for the behavior (and zero-copy path). |
| spark-extension-shims-spark/src/test/scala/org/apache/auron/AuronQuerySuite.scala | Adds an E2E Parquet test ensuring s.child is null when s is null. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
slfan1989
left a comment
There was a problem hiding this comment.
Thanks for the fix.
The parent and child validity handling looks correct, while preserving the zero-copy path when the parent struct has no nulls.
The Rust unit tests and Spark E2E test provide good coverage for this change.
LGTM.
|
@lyne7-sc Thanks for the contribution! Merged into trunk. |
| test("get field from nullable struct propagates parent null") { | ||
| withTable("t_nullable_struct") { | ||
| sql(""" | ||
| |create table t_nullable_struct using parquet as |
There was a problem hiding this comment.
Thank you for your fix. I verified that this problem only exists using parquet. If it is orc or stored as parquet, this problem will not be triggered. I don’t know if I understand it correctly.
There was a problem hiding this comment.
Thanks for verifying this. I rechecked the orc-rust implementation, and I think the orc path may not trigger this issue.
orc-rust passes the struct’s present bitmap to its child decoders, so parent-null positions are already marked null in the child arrow array. Therefore, the old GetIndexedFieldExpr also returns the correct result for orc because null propagation has already happened during decoding.
however, the parquet reader can keep the parent validity only on the StructArray, which exposes the issue.
Which issue does this PR close?
Closes #2472
Rationale for this change
Auron does not propagate the parent struct's null bitmap when accessing a child field. As a result, a field of a null struct may incorrectly return a physical value such as
0instead ofnull.What changes are included in this PR?
GetIndexedFieldExpr.Are there any user-facing changes?
Bug fix only, accessing a field of a null struct now returns null, consistent with vanilla Spark.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
If yes, include:
Generated-by: gpt-5.6ASF guidance: https://www.apache.org/legal/generative-tooling.html