This is an automated email from the ASF dual-hosted git repository.
github-bot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 93724b0b8e fix: validate wrapped negation during type coercion (#20965)
93724b0b8e is described below
commit 93724b0b8ea283e78f3ef9068bc0666c9af39e7e
Author: yaommen <[email protected]>
AuthorDate: Sun Mar 22 23:20:28 2026 +0800
fix: validate wrapped negation during type coercion (#20965)
## Which issue does this PR close?
- Related to #20988 .
## Rationale for this change
Invalid wrapped negation expressions such as `(-'a') IS NULL` could
still escape type validation during optimization.
This PR keeps the fix in the analyzer/type coercion path, which is where
most of DataFusion's type validation already happens, rather than adding
SQL-planner-specific validation.
## What changes are included in this PR?
- add type validation for `Expr::Negative` in `TypeCoercionRewriter`
- return an error for invalid negation operands during optimization
- add focused regression tests for wrapped negation
## Are these changes tested?
Yes.
Added:
- a unit test for `(-'a') IS NULL` in `type_coercion`
- an integration test that verifies the same case fails during
`into_optimized_plan()`
## Are there any user-facing changes?
Yes. Invalid wrapped negation expressions now fail during optimization
instead of escaping type validation until later phases.
---
datafusion/core/tests/sql/sql_api.rs | 17 ++++++++++++
datafusion/optimizer/src/analyzer/type_coercion.rs | 30 ++++++++++++++++++++--
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/datafusion/core/tests/sql/sql_api.rs
b/datafusion/core/tests/sql/sql_api.rs
index b87afd27dd..290aa737d2 100644
--- a/datafusion/core/tests/sql/sql_api.rs
+++ b/datafusion/core/tests/sql/sql_api.rs
@@ -16,6 +16,7 @@
// under the License.
use datafusion::prelude::*;
+use datafusion_common::assert_contains;
use tempfile::TempDir;
@@ -206,3 +207,19 @@ async fn ddl_can_not_be_planned_by_session_state() {
"This feature is not implemented: Unsupported logical plan: DropTable"
);
}
+
+#[tokio::test]
+async fn invalid_wrapped_negation_fails_during_optimization() {
+ let ctx = SessionContext::new();
+ let err = ctx
+ .sql("SELECT * FROM (SELECT 1) WHERE ((-'a') IS NULL)")
+ .await
+ .unwrap()
+ .into_optimized_plan()
+ .unwrap_err();
+
+ assert_contains!(
+ err.strip_backtrace(),
+ "Negation only supports numeric, interval and timestamp types"
+ );
+}
diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs
b/datafusion/optimizer/src/analyzer/type_coercion.rs
index efc9984acb..e21edeccdc 100644
--- a/datafusion/optimizer/src/analyzer/type_coercion.rs
+++ b/datafusion/optimizer/src/analyzer/type_coercion.rs
@@ -43,10 +43,12 @@ use datafusion_expr::expr_schema::cast_subquery;
use datafusion_expr::logical_plan::Subquery;
use datafusion_expr::type_coercion::binary::{comparison_coercion,
like_coercion};
use datafusion_expr::type_coercion::functions::{UDFCoercionExt,
fields_with_udf};
-use datafusion_expr::type_coercion::is_datetime;
use datafusion_expr::type_coercion::other::{
get_coerce_type_for_case_expression, get_coerce_type_for_list,
};
+use datafusion_expr::type_coercion::{
+ is_datetime, is_interval, is_signed_numeric, is_timestamp,
+};
use datafusion_expr::utils::merge_schema;
use datafusion_expr::{
Cast, Expr, ExprSchemable, Join, Limit, LogicalPlan, Operator, Projection,
Union,
@@ -559,6 +561,20 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
Expr::IsNotUnknown(expr) => Ok(Transformed::yes(is_not_unknown(
get_casted_expr_for_bool_op(*expr, self.schema)?,
))),
+ Expr::Negative(expr) => {
+ let data_type = expr.get_type(self.schema)?;
+ if data_type.is_null()
+ || is_signed_numeric(&data_type)
+ || is_interval(&data_type)
+ || is_timestamp(&data_type)
+ {
+ Ok(Transformed::no(Expr::Negative(expr)))
+ } else {
+ plan_err!(
+ "Negation only supports numeric, interval and
timestamp types"
+ )
+ }
+ }
Expr::Like(Like {
negated,
expr,
@@ -753,7 +769,6 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
| Expr::SimilarTo(_)
| Expr::IsNotNull(_)
| Expr::IsNull(_)
- | Expr::Negative(_)
| Expr::Cast(_)
| Expr::TryCast(_)
| Expr::Wildcard { .. }
@@ -1369,6 +1384,17 @@ mod test {
)
}
+ #[test]
+ fn negative_expr_wrapped_by_is_null_errors() -> Result<()> {
+ let predicate =
Expr::IsNull(Box::new(Expr::Negative(Box::new(lit("a")))));
+ let plan = LogicalPlan::Filter(Filter::try_new(predicate, empty())?);
+
+ assert_type_coercion_error(
+ plan,
+ "Negation only supports numeric, interval and timestamp types",
+ )
+ }
+
#[test]
fn test_coerce_union() -> Result<()> {
let left_plan = Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
