Skip to content

Commit 374806c

Browse files
fix(sql): return planner error for malformed typed literals (#21454)
## Which issue does this PR close? - Closes #21431. ## Rationale for this change Planning a malformed typed literal such as `time 17542368000000000` currently panics in `datafusion-sql` because the planner assumes every `TypedString` payload is string-backed and calls `into_string().unwrap()`. Invalid SQL should surface as a normal planner error rather than aborting the planning path. ## What changes are included in this PR? - Replace the `unwrap()` in the `SQLExpr::TypedString` planning path with explicit validation. - Return a planner error when the typed literal payload is not string-backed. - Add a regression test covering the invalid `TIME` typed literal reported in the issue. ## Are these changes tested? - `cargo test -p datafusion-sql` ## Are there any user-facing changes? Users now receive a planner error for malformed typed literals instead of a panic. Signed-off-by: Asish Kumar <officialasishkumar@gmail.com> Co-authored-by: Jonah Gao <jonahgao@msn.com>
1 parent 540d8ec commit 374806c

File tree

2 files changed

+35
-6
lines changed

2 files changed

+35
-6
lines changed

datafusion/sql/src/expr/mod.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,19 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
306306
data_type,
307307
value,
308308
uses_odbc_syntax: _,
309-
}) => Ok(Expr::Cast(Cast::new_from_field(
310-
Box::new(lit(value.into_string().unwrap())),
311-
self.convert_data_type_to_field(&data_type)?,
312-
))),
309+
}) => {
310+
let value = match value.into_string() {
311+
Some(value) => value,
312+
None => {
313+
return plan_err!("Typed literal requires a string payload");
314+
}
315+
};
316+
317+
Ok(Expr::Cast(Cast::new_from_field(
318+
Box::new(lit(value)),
319+
self.convert_data_type_to_field(&data_type)?,
320+
)))
321+
}
313322

314323
SQLExpr::IsNull(expr) => Ok(Expr::IsNull(Box::new(
315324
self.sql_expr_to_logical_expr(*expr, schema, planner_context)?,

datafusion/sql/tests/sql_integration.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use std::vec;
2626

2727
use arrow::datatypes::{TimeUnit::Nanosecond, *};
2828
use common::MockContextProvider;
29-
use datafusion_common::{DataFusionError, Result, assert_contains};
29+
use datafusion_common::{DFSchema, DataFusionError, Result, assert_contains};
3030
use datafusion_expr::{
3131
ColumnarValue, CreateIndex, DdlStatement, ScalarFunctionArgs, ScalarUDF,
3232
ScalarUDFImpl, Signature, Volatility, col, logical_plan::LogicalPlan,
@@ -35,7 +35,7 @@ use datafusion_expr::{
3535
use datafusion_functions::{string, unicode};
3636
use datafusion_sql::{
3737
parser::DFParser,
38-
planner::{NullOrdering, ParserOptions, SqlToRel},
38+
planner::{NullOrdering, ParserOptions, PlannerContext, SqlToRel},
3939
};
4040

4141
use crate::common::{CustomExprPlanner, CustomTypePlanner, MockSessionState};
@@ -52,6 +52,7 @@ use datafusion_functions_window::{rank::rank_udwf, row_number::row_number_udwf};
5252
use insta::{allow_duplicates, assert_snapshot};
5353
use rstest::rstest;
5454
use sqlparser::dialect::{Dialect, GenericDialect, HiveDialect, MySqlDialect};
55+
use sqlparser::parser::Parser;
5556

5657
mod cases;
5758
mod common;
@@ -254,6 +255,25 @@ fn within_group_rejected_for_non_ordered_set_udaf() {
254255
);
255256
}
256257

258+
#[test]
259+
fn typed_literal_without_string_payload_returns_error() {
260+
let sql_expr = Parser::new(&GenericDialect {})
261+
.try_with_sql("time 17542368000000000")
262+
.unwrap()
263+
.parse_expr()
264+
.unwrap();
265+
let context = MockContextProvider {
266+
state: MockSessionState::default(),
267+
};
268+
let sql_to_rel = SqlToRel::new(&context);
269+
270+
let err = sql_to_rel
271+
.sql_to_expr(sql_expr, &DFSchema::empty(), &mut PlannerContext::new())
272+
.expect_err("planning invalid typed literals should return an error");
273+
274+
assert_contains!(err.to_string(), "Typed literal requires a string payload");
275+
}
276+
257277
#[test]
258278
fn parse_ident_normalization_5() {
259279
let sql = "SELECT AGE FROM PERSON";

0 commit comments

Comments
 (0)