Summary
Planning an invalid SQL expression such as:
causes datafusion-sql to panic instead of returning a planner error.
The panic happens in the SQLExpr::TypedString branch inside SqlToRel::sql_expr_to_logical_expr_internal, where DataFusion assumes the typed literal payload is always a string and calls:
value.into_string().unwrap()
For this input, the parser produces a typed-string-like AST path with a non-string value, so into_string() returns None and the planner aborts with Option::unwrap().
Impact
- Invalid user SQL can crash the planning path.
- Embedders of DataFusion may see process-level panics instead of a recoverable SQL/planner error.
- This is especially confusing when the original SQL is just malformed and should be rejected normally.
Reproduction
#[test]
fn test_sql_to_expr_panics_on_time_typed_string_without_string_literal() {
let schema = DFSchema::empty();
let mut planner_context = PlannerContext::default();
let expr_str = "time 17542368000000000";
let dialect = GenericDialect {};
let mut parser = Parser::new(&dialect).try_with_sql(expr_str).unwrap();
let sql_expr = parser.parse_expr().unwrap();
let context_provider = TestContextProvider::new();
let sql_to_rel = SqlToRel::new(&context_provider);
let panic = catch_unwind(AssertUnwindSafe(|| {
sql_to_rel
.sql_expr_to_logical_expr(sql_expr, &schema, &mut planner_context)
.unwrap();
}))
.expect_err("planning invalid TIME typed string syntax should panic today");
let panic_message = if let Some(message) = panic.downcast_ref::<String>() {
message.as_str()
} else if let Some(message) = panic.downcast_ref::<&str>() {
message
} else {
""
};
assert!(
panic_message.contains("called `Option::unwrap()` on a `None` value"),
"unexpected panic payload: {panic_message}"
);
}
Expected behavior
DataFusion should return a normal Result::Err(...) describing invalid SQL or an unsupported typed literal payload.
Examples of acceptable outcomes:
- parser error
- planner error
not_impl_err! / plan_err!
Any of these would be preferable to a panic.
Why this happens
The problematic code path is effectively:
SQLExpr::TypedString(TypedString {
data_type,
value,
uses_odbc_syntax: _,
}) => Ok(Expr::Cast(Cast::new(
Box::new(lit(value.into_string().unwrap())),
self.convert_data_type_to_field(&data_type)?
.data_type()
.clone(),
))),
This assumes value is always string-backed. For malformed input such as time 17542368000000000, that assumption does not hold.
Suggested fix direction
Replace the unwrap() with explicit validation and return a planner error when the typed literal payload is not a string.
Pseudo-shape:
let value = value.into_string().ok_or_else(|| {
plan_datafusion_err!("TIME/typed string literal requires a string payload")
})?;
The exact error text can be adjusted to match DataFusion conventions.
Summary
Planning an invalid SQL expression such as:
causes
datafusion-sqlto panic instead of returning a planner error.The panic happens in the
SQLExpr::TypedStringbranch insideSqlToRel::sql_expr_to_logical_expr_internal, where DataFusion assumes the typed literal payload is always a string and calls:For this input, the parser produces a typed-string-like AST path with a non-string value, so
into_string()returnsNoneand the planner aborts withOption::unwrap().Impact
Reproduction
Expected behavior
DataFusion should return a normal
Result::Err(...)describing invalid SQL or an unsupported typed literal payload.Examples of acceptable outcomes:
not_impl_err!/plan_err!Any of these would be preferable to a panic.
Why this happens
The problematic code path is effectively:
This assumes
valueis always string-backed. For malformed input such astime 17542368000000000, that assumption does not hold.Suggested fix direction
Replace the
unwrap()with explicit validation and return a planner error when the typed literal payload is not a string.Pseudo-shape:
The exact error text can be adjusted to match DataFusion conventions.