fix: IS NULL panic with invalid function without input arguments by Acfboy · Pull Request #20306 · apache/datafusion

Acfboy · 2026-02-12T09:38:28Z

Which issue does this PR close?

Closes IS NULL doesn't type-check its input #20201 .

Rationale for this change

Unlike Projection, expressions within a Filter node do not always have their types resolved during the initial LogicalPlan generation. Validation is often deferred until the type_coercion phase. When invoking f_up on the expression tree to perform type coercion, the check is bypassed for function nodes with empty arguments, leading to a panic during subsequent execution.

For example, all statements below cause a panic:

SELECT * FROM (SELECT 1) WHERE (STARTS_WITH() IS NULL);
SELECT * FROM (SELECT 1) WHERE (STARTS_WITH() IS NOT NULL);
SELECT * FROM (SELECT 'a') WHERE (STARTS_WITH() SIMILAR TO 'abc%');
SELECT * FROM (SELECT 1) WHERE CAST(STARTS_WITH() AS STRING) = 'x';
SELECT * FROM (SELECT 1) WHERE TRY_CAST(STARTS_WITH() AS INT) IS NULL;

This pr aims to stop panic. It would be better if reject these invalid cases at planning.

What changes are included in this PR?

 fn coerce_arguments_for_signature<F: UDFCoercionExt>(
      schema: &DFSchema,
      func: &F,
  ) -> Result<Vec<Expr>> {
-    if expressions.is_empty() {
-        return Ok(expressions);
-    }

Deleted the early return. Thanks to @neilconway .

Are these changes tested?

All original tests passed. Added some unit tests and sqllogictests.

Are there any user-facing changes?

No.

neilconway

Thanks for taking a look at this!

neilconway · 2026-02-12T14:52:00Z

                });
                Ok(Transformed::yes(new_expr))
            }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate


I don't think comments should reference "previously" (previously to what?); they should just talk about the current state of the code.

I'd rephrase this comment to something like:

// IS NULL, IS NOT NULL, SIMILAR TO, CAST, and TRY CAST don't require // type coercion, but we need to handle them here to ensure that their // arguments have type coercion and type checking applied (see #20201 // for context).

Thanks, modified.

neilconway · 2026-02-12T14:53:42Z

+        Expr::ScalarFunction(ScalarFunction::new_udf(Arc::new(fun), vec![]))
+    }
+
+    #[test]


These tests feel redundant to the SLT tests, no? In my opinion we should be fine with just adding the SLT tests only (which arguably is closer to testing the user-visible behavior anyway).

I don't see it as overlapping. slt ensures the whole path works, and unit tests are there to help us isolate the root cause when something goes wrong.

I agree with @neilconway here -- I do think the tests are redundant and I think we can just use the slt tests

neilconway · 2026-02-12T14:54:38Z

+
+###############################################################################
+## Test that IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate  ##
+## their inner expressions during type coercion (previously they were in     ##


I'm omit discussion of the previous incorrect behavior, I'd just talk about the current/correct behavior under test.

neilconway · 2026-02-12T15:08:42Z

+                let _check_type = inner_expr.get_type(self.schema)?;
+                Ok(Transformed::no(expr))
+            }
+            Expr::SimilarTo(Like {


I wonder whether the handling of SIMILAR TO should be more similar to how we handle LIKE?

I suspect this is because the current implementation has focused more on optimizing LIKE. Many of these optimizations could likely be applied to SIMILAR TO as well, but they are probably just yet to be implemented.

For this issue, let's maintain consistency with the existing implementation. Maybe we should create separate issues to refine how SIMILAR TO is handled.

I wasn't thinking about optimizations; my concern was more whether the type coercion that is done for LIKE should also be done for IS SIMILAR TO for correctness reasons. Some digging suggests this might be the case:

-- works SELECT CAST('hello' AS BYTEA) LIKE 'hello%'; -- fails ("Cannot infer common argument type for regex operation Binary ~ Utf8") SELECT CAST('hello' AS BYTEA) SIMILAR TO 'hello%'; -- works SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') LIKE 'hello%'; -- fails ("Data type Dictionary(Int32, Utf8) not supported for regex_match_dyn") SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') SIMILAR TO 'hello%';

I filed #20342, I'll take a look at fixing shortly.

neilconway · 2026-02-13T17:54:43Z

Digging into this a bit further, this fix seems cleaner to me:

diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs
index c2121f45c..1a8218a1e 100644
--- a/datafusion/optimizer/src/analyzer/type_coercion.rs
+++ b/datafusion/optimizer/src/analyzer/type_coercion.rs
@@ -954,10 +954,6 @@ fn coerce_arguments_for_signature<F: UDFCoercionExt>(
     schema: &DFSchema,
     func: &F,
 ) -> Result<Vec<Expr>> {
-    if expressions.is_empty() {
-        return Ok(expressions);
-    }
-
     let current_fields = expressions
         .iter()
         .map(|e| e.to_field(schema).map(|(_, f)| f))

i.e., coerce_arguments_for_signature() -> fields_with_udf() is already checking for functions that require arguments but that are invoked with no arguments, but this is effectively dead code because we exit early before we get there. If we just remove the early-exit then the behavior seems right to me. What do you think?

alamb

Thanks @Acfboy and @neilconway

I think this looks good to me

alamb · 2026-02-13T18:32:26Z

+        Expr::ScalarFunction(ScalarFunction::new_udf(Arc::new(fun), vec![]))
+    }
+
+    #[test]


I agree with @neilconway here -- I do think the tests are redundant and I think we can just use the slt tests

alamb · 2026-02-13T18:37:18Z

+
+# https://github.com/apache/datafusion/issues/20201
+
+# IS NULL should reject invalid inner expression


I ran these tests without the code changes to verify they cover the issue and they fail like

Completed 403 test files in 8 seconds failure in type_coercion.slt for sql SELECT * FROM (SELECT 1) WHERE (STARTS_WITH() IS NULL); caused by External error: task 22188 panicked with message "index out of bounds: the len is 0 but the index is 1" Error: Execution("1 failures") error: test failed, to rerun pass `-p datafusion-sqllogictest --test sqllogictests`

Thanks for the review, @alamb and @neilconway. I have removed the redundant unit tests in type_coercion.rs and will rely on the .slt tests for coverage as suggested.

neilconway · 2026-02-13T18:43:36Z

Thanks @Acfboy and @neilconway

I think this looks good to me

Thanks @alamb ! I'm curious what you think of the alternative approach I suggested #20306 (comment)

alamb · 2026-02-13T20:28:36Z

If we just remove the early-exit then the behavior seems right to me. What do you think?

I think that sounds like a great change to me

Acfboy · 2026-02-14T02:16:21Z

i.e., coerce_arguments_for_signature() -> fields_with_udf() is already checking for functions that require arguments but that are invoked with no arguments, but this is effectively dead code because we exit early before we get there. If we just remove the early-exit then the behavior seems right to me. What do you think?

It seems this only works for cases where UDFs are called without arguments. However, there are other cases failing due to a lack of type-checking in IS NULL. For example, SELECT * FROM (SELECT 1) WHERE ((1.0 << 2.0) IS NULL); is expected to report that we can't run float << float, but it actually runs without error after constant folding by the optimizer.

neilconway · 2026-02-14T02:24:23Z

It seems this only works for cases where UDFs are called without arguments. However, there are other cases failing due to a lack of type-checking in IS NULL. For example, SELECT * FROM (SELECT 1) WHERE ((1.0 << 2.0) IS NULL); is expected to report that we can't run float << float, but it actually runs without error after constant folding by the optimizer.

Good point!

Can you add one or more example queries like this to the SLT tests?

Acfboy · 2026-02-14T08:41:23Z

@neilconway Thanks! While trying to add some tests, another bug occurs: #20355 . So I can't add something like SELECT * FROM (SELECT 1) WHERE ((-'abc') IS NULL); to slt tests now.

Also, my apologies for the oversight. Binary operators are indeed type-checked. The bitwise operation issue was a bug that's already been fixed in a merged PR. I've checked the other branches, and it appears that only unary negation and function calls still have this problem.

neilconway · 2026-02-15T16:05:49Z

Proposal:

Remove the early return from coerce_arguments_for_signature(), because that seems wrong in any case. (I'm happy to send a PR for this or you can, @Acfboy -- lmk what you prefer)
Fix The negation operator escapes type checking at logical planning phase #20355 and Incorrect type coercion for IS SIMILAR TO #20342 separately

My understanding is that if we do the above, we don't need the changes to TypeCoercionRewriter proposed in this PR -- is that correct?

…lidation

Acfboy · 2026-02-16T04:46:35Z

Thanks @neilconway ! You are right. I have changed the pr and re-request reivews.

neilconway

🎉🎉🎉

Can you update the PR description to match the current approach?

alamb

makes sense to me -- thank you @Acfboy and @neilconway

@neilconway

…che#20306) ## Which issue does this PR close?  - Closes apache#20201 . ## Rationale for this change Unlike `Projection`, expressions within a `Filter` node do not always have their types resolved during the initial LogicalPlan generation. Validation is often deferred until the type_coercion phase. When invoking f_up on the expression tree to perform type coercion, the check is bypassed for function nodes with empty arguments, leading to a panic during subsequent execution. For example, all statements below cause a panic: ``` SELECT * FROM (SELECT 1) WHERE (STARTS_WITH() IS NULL); SELECT * FROM (SELECT 1) WHERE (STARTS_WITH() IS NOT NULL); SELECT * FROM (SELECT 'a') WHERE (STARTS_WITH() SIMILAR TO 'abc%'); SELECT * FROM (SELECT 1) WHERE CAST(STARTS_WITH() AS STRING) = 'x'; SELECT * FROM (SELECT 1) WHERE TRY_CAST(STARTS_WITH() AS INT) IS NULL; ``` This pr aims to stop panic. It would be better if reject these invalid cases at planning. ## What changes are included in this PR?  ```diff fn coerce_arguments_for_signature<F: UDFCoercionExt>( schema: &DFSchema, func: &F, ) -> Result<Vec<Expr>> { - if expressions.is_empty() { - return Ok(expressions); - } ``` Deleted the early return. Thanks to @neilconway . ## Are these changes tested? All original tests passed. Added some unit tests and sqllogictests. ## Are there any user-facing changes?   No. --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

fix: panic when is null accept invalid input

7db880b

github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Feb 12, 2026

neilconway approved these changes Feb 12, 2026

View reviewed changes

neilconway mentioned this pull request Feb 13, 2026

Incorrect type coercion for IS SIMILAR TO #20342

Open

alamb approved these changes Feb 13, 2026

View reviewed changes

Acfboy force-pushed the fix-isnull-not-check-input branch 2 times, most recently from 137947b to 2c04325 Compare February 14, 2026 02:47

fix: remove early exit in coerce_arguments_for_signature to ensure va…

9f54b51

…lidation

Acfboy force-pushed the fix-isnull-not-check-input branch from 2c04325 to 9f54b51 Compare February 16, 2026 04:44

Acfboy requested review from alamb and neilconway February 16, 2026 04:44

neilconway approved these changes Feb 16, 2026

View reviewed changes

Acfboy changed the title ~~fix: IS NULL doesn't type-check its input and panic~~ fix: IS NULL panic with invalid function without input arguments Feb 17, 2026

alamb approved these changes Feb 24, 2026

View reviewed changes

Merge branch 'main' into fix-isnull-not-check-input

206430f

alamb added this pull request to the merge queue Feb 26, 2026

Merged via the queue into apache:main with commit e76f0ee Feb 26, 2026
32 checks passed

alamb mentioned this pull request Apr 4, 2026

Release DataFusion 52.5.0 (minor) Release (Apr 2026) #21078

Closed

11 tasks done


		# https://github.com/apache/datafusion/issues/20201

		# IS NULL should reject invalid inner expression

Conversation

Acfboy commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

neilconway left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neilconway commented Feb 13, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neilconway commented Feb 13, 2026

Uh oh!

alamb commented Feb 13, 2026

Uh oh!

Acfboy commented Feb 14, 2026

Uh oh!

neilconway commented Feb 14, 2026

Uh oh!

Acfboy commented Feb 14, 2026

Uh oh!

neilconway commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Acfboy commented Feb 16, 2026

Uh oh!

neilconway left a comment

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Acfboy commented Feb 12, 2026 •

edited

Loading

neilconway commented Feb 15, 2026 •

edited

Loading