Skip to content

[SPARK-56431][SQL] Fix NPE in FilterExec CSE when notNull columns are null before short-circuit#55288

Closed
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:SPARK-56431
Closed

[SPARK-56431][SQL] Fix NPE in FilterExec CSE when notNull columns are null before short-circuit#55288
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:SPARK-56431

Conversation

@LuciferYang
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

In FilterExec.doConsume, bind otherPreds against child.output (instead of output) for both CSE analysis and predicate code generation.

Why are the changes needed?

SPARK-56032 added CSE support to FilterExec. The CSE precomputation code runs before the IsNotNull predicates short-circuit, at the start of the do {} while(false) block. However, the boundOtherPreds passed to CSE analysis were bound against output, where columns in notNullAttributes have nullable=false. This caused the CSE-generated code to omit null checks for those columns. At runtime, when a row has a null value in such a column, the precomputation
calls an arithmetic operator (e.g. Decimal.subtract) on the null value before the IsNotNull check can filter the row out, resulting in an NPE.

The original SPARK-56032 design comment states:

CSE evaluation is placed before predicate short-circuit checks -- This is safe
because Spark SQL expressions handle null inputs gracefully (returning null rather
than throwing).

This assumption is only correct when the expressions preserve their input columns' original nullability. Binding against output (with tightened nullability) breaks it.

Reproducer: a FilterExec over a source with a nullable DecimalType column, where the filter has both IsNotNull(col) and a repeated arithmetic expression on col (creating a CSE candidate), and at least one input row has col = null.

Does this PR introduce any user-facing change?

No. This optimization has not been released yet.

How was this patch tested?

Added a new test in WholeStageCodegenSuite.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code

… null before short-circuit

In `FilterExec.doConsume`, bind `otherPreds` against `child.output`
(instead of `output`) for both CSE analysis and predicate code generation.

SPARK-56032 added CSE support to FilterExec. The CSE precomputation runs
before the IsNotNull predicates short-circuit. However, the bound
expressions were using `output` (where notNullAttributes columns have
nullable=false), causing the CSE-generated code to omit null checks.
At runtime, when a row has a null value in such a column, the
precomputation crashes with NPE (e.g. Decimal.subtract on null) before
the IsNotNull check can filter the row out.

Binding against `child.output` preserves the original nullability so
the CSE-extracted code emits proper null guards.
@LuciferYang
Copy link
Copy Markdown
Contributor Author

Merged into master. Thanks @dongjoon-hyun

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