Skip to content

Conversation

@DrakeLin
Copy link
Collaborator

@DrakeLin DrakeLin commented Jan 21, 2026

🥞 Stacked PR

Use this link to review incremental changes.


What changes are proposed in this pull request?

Fixes a bug in nested transform expression evaluation where null rows in the source struct were losing their null bitmap, causing null structs to incorrectly appear as non-null structs with null fields.

When evaluating nested transform expressions (transforms with an input_path that operate on a nested struct), the output StructArray was created with None for the null buffer:
let data = StructArray::try_new(output_fields.into(), output_cols, None)?;
This meant that if the source struct had null rows (e.g., an add action that is null in a checkpoint batch), the output would lose that null information. The struct would appear as non-null but with all-null fields, which is semantically different.

How was this change tested?

Existing transform tests pass. The stats transform integration tests (in a follow-up PR) exercise this code path.

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.65%. Comparing base (daa9a1e) to head (37e7550).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...src/engine/arrow_expression/evaluate_expression.rs 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1645   +/-   ##
=======================================
  Coverage   84.65%   84.65%           
=======================================
  Files         123      123           
  Lines       34109    34114    +5     
  Branches    34109    34114    +5     
=======================================
+ Hits        28875    28880    +5     
  Misses       3905     3905           
  Partials     1329     1329           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the breaking-change Change that require a major version bump label Jan 21, 2026
@DrakeLin DrakeLin marked this pull request as ready for review January 21, 2026 00:21
@DrakeLin DrakeLin force-pushed the stack/null-propagation branch from d37ed82 to 85063cc Compare January 21, 2026 00:21
@DrakeLin DrakeLin requested review from dengsh12 and nicklan January 21, 2026 00:24
@DrakeLin DrakeLin force-pushed the stack/null-propagation branch from 85063cc to 166dd26 Compare January 21, 2026 00:37
@github-actions github-actions bot removed the breaking-change Change that require a major version bump label Jan 21, 2026
@DrakeLin DrakeLin force-pushed the stack/null-propagation branch 2 times, most recently from c13395e to 9dd11dd Compare January 21, 2026 23:38
@github-actions github-actions bot added the breaking-change Change that require a major version bump label Jan 21, 2026
Copy link
Member

@nicklan nicklan left a comment

Choose a reason for hiding this comment

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

nice catch! i'm assuming from your PR comment that this is tested by code later in the stack, so lgtm

Copy link
Collaborator

@dengsh12 dengsh12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@DrakeLin DrakeLin force-pushed the stack/null-propagation branch from 9dd11dd to 37e7550 Compare January 23, 2026 03:14
@DrakeLin DrakeLin merged commit 25fa27f into delta-io:main Jan 23, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Change that require a major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants