You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Acfboy (via GitHub)" <gi...@apache.org> on 2026/02/12 09:38:31 UTC

[PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Acfboy opened a new pull request, #20306:
URL: https://github.com/apache/datafusion/pull/20306

   ## Which issue does this PR close?
   
   <!--
   We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
   -->
   
   - Closes #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. While operators with specific type requirements (e.g., IS TRUE) trigger coercion and catch malformed expressions, type-neutral operators (e.g., IS NULL) bypass this check, allowing invalid expressions to reach the optimizer and cause a panic.
   
   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?
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   Add checks like this in `type_coercion.rs`:
   
   ```rust
   Expr::IsNotNull(ref inner_expr) | Expr::IsNull(ref inner_expr) => {
       let _check_type = inner_expr.get_type(self.schema)?;
       Ok(Transformed::no(expr))
   }
   ```
   
   ## Are these changes tested?
   All original tests passed. Added some unit tests and sqllogictests.
   
   
   ## Are there any user-facing changes?
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->
   No.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2802492157


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate

Review Comment:
   Thanks, modified.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3904739530

   Proposal:
   
   1. 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)
   2. Fix #20355 and #20342 separately
   
   My understanding is that if we do both of these, we don't need the changes to `TypeCoercionRewriter` proposed in this PR -- is that correct?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3899298174

   >  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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3898767489

   > 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 https://github.com/apache/datafusion/pull/20306#issuecomment-3898530434


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2804703038


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate
+            // their inner expressions during type coercion (previously they were in
+            // the catch-all branch and skipped validation, which caused panics at
+            // runtime for invalid inner expressions like zero-argument function calls).
+            // Detail: <https://github.com/apache/datafusion/issues/20201>
+            //
+            // TODO: reject the invalid cases at planning.
+            Expr::IsNotNull(ref inner_expr) | Expr::IsNull(ref inner_expr) => {
+                let _check_type = inner_expr.get_type(self.schema)?;
+                Ok(Transformed::no(expr))
+            }
+            Expr::SimilarTo(Like {

Review Comment:
   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%';
     ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3900480386

   > 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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2805547847


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -2742,4 +2771,69 @@ mod test {
         "
         )
     }
+
+    fn zero_arg_scalar_func() -> Expr {
+        let fun = ScalarUDF::new_from_impl(TestScalarUDF {
+            signature: Signature::uniform(1, vec![DataType::Float32], Volatility::Stable),
+        });
+        Expr::ScalarFunction(ScalarFunction::new_udf(Arc::new(fun), vec![]))
+    }
+
+    #[test]

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



##########
datafusion/sqllogictest/test_files/type_coercion.slt:
##########
@@ -281,3 +281,30 @@ SELECT true FROM t0 WHERE (REGEXP_MATCH(t0.v1, t0.v1)) NOT ILIKE [];
 
 statement ok
 DROP TABLE t0;
+
+###############################################################################
+## Test that IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate   ##
+## their inner expressions.                                                  ##
+###############################################################################
+
+# https://github.com/apache/datafusion/issues/20201
+
+# IS NULL should reject invalid inner expression

Review Comment:
   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`
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2799336096


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate

Review Comment:
   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).
   ```



##########
datafusion/sqllogictest/test_files/type_coercion.slt:
##########
@@ -281,3 +281,30 @@ SELECT true FROM t0 WHERE (REGEXP_MATCH(t0.v1, t0.v1)) NOT ILIKE [];
 
 statement ok
 DROP TABLE t0;
+
+###############################################################################
+## Test that IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate  ##
+## their inner expressions during type coercion (previously they were in     ##

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



##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -2742,4 +2771,69 @@ mod test {
         "
         )
     }
+
+    fn zero_arg_scalar_func() -> Expr {
+        let fun = ScalarUDF::new_from_impl(TestScalarUDF {
+            signature: Signature::uniform(1, vec![DataType::Float32], Volatility::Stable),
+        });
+        Expr::ScalarFunction(ScalarFunction::new_udf(Arc::new(fun), vec![]))
+    }
+
+    #[test]

Review Comment:
   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).



##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate
+            // their inner expressions during type coercion (previously they were in
+            // the catch-all branch and skipped validation, which caused panics at
+            // runtime for invalid inner expressions like zero-argument function calls).
+            // Detail: <https://github.com/apache/datafusion/issues/20201>
+            //
+            // TODO: reject the invalid cases at planning.
+            Expr::IsNotNull(ref inner_expr) | Expr::IsNull(ref inner_expr) => {
+                let _check_type = inner_expr.get_type(self.schema)?;
+                Ok(Transformed::no(expr))
+            }
+            Expr::SimilarTo(Like {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2805417152


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate
+            // their inner expressions during type coercion (previously they were in
+            // the catch-all branch and skipped validation, which caused panics at
+            // runtime for invalid inner expressions like zero-argument function calls).
+            // Detail: <https://github.com/apache/datafusion/issues/20201>
+            //
+            // TODO: reject the invalid cases at planning.
+            Expr::IsNotNull(ref inner_expr) | Expr::IsNull(ref inner_expr) => {
+                let _check_type = inner_expr.get_type(self.schema)?;
+                Ok(Transformed::no(expr))
+            }
+            Expr::SimilarTo(Like {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3900465808

   > 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2806808454


##########
datafusion/sqllogictest/test_files/type_coercion.slt:
##########
@@ -281,3 +281,30 @@ SELECT true FROM t0 WHERE (REGEXP_MATCH(t0.v1, t0.v1)) NOT ILIKE [];
 
 statement ok
 DROP TABLE t0;
+
+###############################################################################
+## Test that IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate   ##
+## their inner expressions.                                                  ##
+###############################################################################
+
+# https://github.com/apache/datafusion/issues/20201
+
+# IS NULL should reject invalid inner expression

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3906408699

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3898530434

   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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2802517231


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -744,18 +744,47 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
                 });
                 Ok(Transformed::yes(new_expr))
             }
+            // IS NULL / IS NOT NULL / SIMILAR TO / CAST / TRY_CAST validate
+            // their inner expressions during type coercion (previously they were in
+            // the catch-all branch and skipped validation, which caused panics at
+            // runtime for invalid inner expressions like zero-argument function calls).
+            // Detail: <https://github.com/apache/datafusion/issues/20201>
+            //
+            // TODO: reject the invalid cases at planning.
+            Expr::IsNotNull(ref inner_expr) | Expr::IsNull(ref inner_expr) => {
+                let _check_type = inner_expr.get_type(self.schema)?;
+                Ok(Transformed::no(expr))
+            }
+            Expr::SimilarTo(Like {

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on code in PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#discussion_r2802497284


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -2742,4 +2771,69 @@ mod test {
         "
         )
     }
+
+    fn zero_arg_scalar_func() -> Expr {
+        let fun = ScalarUDF::new_from_impl(TestScalarUDF {
+            signature: Signature::uniform(1, vec![DataType::Float32], Volatility::Stable),
+        });
+        Expr::ScalarFunction(ScalarFunction::new_udf(Arc::new(fun), vec![]))
+    }
+
+    #[test]

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: IS NULL doesn't type-check its input and panic [datafusion]

Posted by "Acfboy (via GitHub)" <gi...@apache.org>.
Acfboy commented on PR #20306:
URL: https://github.com/apache/datafusion/pull/20306#issuecomment-3901440291

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org