Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ recursive = "0.1.1"
regex = "1.8"
rstest = "0.24.0"
serde_json = "1"
sqlparser = { version = "0.54.0", features = ["visitor"] }
sqlparser = { version = "0.55.0", features = ["visitor"] }
tempfile = "3"
tokio = { version = "1.43", features = ["macros", "rt", "sync"] }
url = "2.5.4"
Expand Down
27 changes: 13 additions & 14 deletions datafusion/expr/src/window_frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::fmt::{self, Formatter};
use std::hash::Hash;

use datafusion_common::{plan_err, sql_err, DataFusionError, Result, ScalarValue};
use sqlparser::ast;
use sqlparser::ast::{self, ValueWithSpan};
use sqlparser::parser::ParserError::ParserError;

/// The frame specification determines which output rows are read by an aggregate
Expand Down Expand Up @@ -368,7 +368,7 @@ fn convert_frame_bound_to_scalar_value(
match units {
// For ROWS and GROUPS we are sure that the ScalarValue must be a non-negative integer ...
ast::WindowFrameUnits::Rows | ast::WindowFrameUnits::Groups => match v {
ast::Expr::Value(ast::Value::Number(value, false)) => {
ast::Expr::Value(ValueWithSpan{value: ast::Value::Number(value, false), span: _}) => {
Ok(ScalarValue::try_from_string(value, &DataType::UInt64)?)
},
ast::Expr::Interval(ast::Interval {
Expand All @@ -379,7 +379,7 @@ fn convert_frame_bound_to_scalar_value(
fractional_seconds_precision: None,
}) => {
let value = match *value {
ast::Expr::Value(ast::Value::SingleQuotedString(item)) => item,
ast::Expr::Value(ValueWithSpan{value: ast::Value::SingleQuotedString(item), span: _}) => item,
e => {
return sql_err!(ParserError(format!(
"INTERVAL expression cannot be {e:?}"
Expand All @@ -395,14 +395,14 @@ fn convert_frame_bound_to_scalar_value(
// ... instead for RANGE it could be anything depending on the type of the ORDER BY clause,
// so we use a ScalarValue::Utf8.
ast::WindowFrameUnits::Range => Ok(ScalarValue::Utf8(Some(match v {
ast::Expr::Value(ast::Value::Number(value, false)) => value,
ast::Expr::Value(ValueWithSpan{value: ast::Value::Number(value, false), span: _}) => value,
ast::Expr::Interval(ast::Interval {
value,
leading_field,
..
}) => {
let result = match *value {
ast::Expr::Value(ast::Value::SingleQuotedString(item)) => item,
ast::Expr::Value(ValueWithSpan{value: ast::Value::SingleQuotedString(item), span: _}) => item,
e => {
return sql_err!(ParserError(format!(
"INTERVAL expression cannot be {e:?}"
Expand Down Expand Up @@ -514,10 +514,10 @@ mod tests {
let window_frame = ast::WindowFrame {
units: ast::WindowFrameUnits::Rows,
start_bound: ast::WindowFrameBound::Preceding(Some(Box::new(
ast::Expr::Value(ast::Value::Number("2".to_string(), false)),
ast::Expr::value(ast::Value::Number("2".to_string(), false)),
))),
end_bound: Some(ast::WindowFrameBound::Preceding(Some(Box::new(
ast::Expr::Value(ast::Value::Number("1".to_string(), false)),
ast::Expr::value(ast::Value::Number("1".to_string(), false)),
)))),
};

Expand Down Expand Up @@ -575,10 +575,9 @@ mod tests {
test_bound!(Range, None, ScalarValue::Null);

// Number
let number = Some(Box::new(ast::Expr::Value(ast::Value::Number(
"42".to_string(),
false,
))));
let number = Some(Box::new(ast::Expr::Value(
ast::Value::Number("42".to_string(), false).into(),
)));
test_bound!(Rows, number.clone(), ScalarValue::UInt64(Some(42)));
test_bound!(Groups, number.clone(), ScalarValue::UInt64(Some(42)));
test_bound!(
Expand All @@ -589,9 +588,9 @@ mod tests {

// Interval
let number = Some(Box::new(ast::Expr::Interval(ast::Interval {
value: Box::new(ast::Expr::Value(ast::Value::SingleQuotedString(
"1".to_string(),
))),
value: Box::new(ast::Expr::Value(
ast::Value::SingleQuotedString("1".to_string()).into(),
)),
leading_field: Some(ast::DateTimeField::Day),
fractional_seconds_precision: None,
last_field: None,
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sql/src/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use datafusion_expr::{
use sqlparser::ast::{
DuplicateTreatment, Expr as SQLExpr, Function as SQLFunction, FunctionArg,
FunctionArgExpr, FunctionArgumentClause, FunctionArgumentList, FunctionArguments,
NullTreatment, ObjectName, OrderByExpr, WindowType,
NullTreatment, ObjectName, OrderByExpr, Spanned, WindowType,
};

/// Suggest a valid function based on an invalid input function name
Expand Down Expand Up @@ -217,13 +217,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
// it shouldn't have ordering requirement as function argument
// required ordering should be defined in OVER clause.
let is_function_window = over.is_some();
let sql_parser_span = name.0[0].span;
let sql_parser_span = name.0[0].span();
let name = if name.0.len() > 1 {
// DF doesn't handle compound identifiers
// (e.g. "foo.bar") for function names yet
name.to_string()
} else {
crate::utils::normalize_ident(name.0[0].clone())
crate::utils::normalize_ident(name.0[0].as_ident().unwrap().clone())
};

if name.eq("make_map") {
Expand Down
36 changes: 18 additions & 18 deletions datafusion/sql/src/expr/identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use datafusion_common::{
};
use datafusion_expr::planner::PlannerResult;
use datafusion_expr::{Case, Expr};
use sqlparser::ast::{Expr as SQLExpr, Ident};
use sqlparser::ast::{CaseWhen, Expr as SQLExpr, Ident};

use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
use datafusion_expr::UNNAMED_TABLE;
Expand Down Expand Up @@ -216,8 +216,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
pub(super) fn sql_case_identifier_to_expr(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, why is it called case_identifier🤔
I'm not sure what the connection is between case and identifier.

&self,
operand: Option<Box<SQLExpr>>,
conditions: Vec<SQLExpr>,
results: Vec<SQLExpr>,
conditions: Vec<CaseWhen>,
else_result: Option<Box<SQLExpr>>,
schema: &DFSchema,
planner_context: &mut PlannerContext,
Expand All @@ -231,13 +230,22 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
} else {
None
};
let when_expr = conditions
let when_then_expr = conditions
.into_iter()
.map(|e| self.sql_expr_to_logical_expr(e, schema, planner_context))
.collect::<Result<Vec<_>>>()?;
let then_expr = results
.into_iter()
.map(|e| self.sql_expr_to_logical_expr(e, schema, planner_context))
.map(|e| {
Ok((
Box::new(self.sql_expr_to_logical_expr(
e.condition,
schema,
planner_context,
)?),
Box::new(self.sql_expr_to_logical_expr(
e.result,
schema,
planner_context,
)?),
))
})
.collect::<Result<Vec<_>>>()?;
let else_expr = if let Some(e) = else_result {
Some(Box::new(self.sql_expr_to_logical_expr(
Expand All @@ -249,15 +257,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
None
};

Ok(Expr::Case(Case::new(
expr,
when_expr
.iter()
.zip(then_expr.iter())
.map(|(w, t)| (Box::new(w.to_owned()), Box::new(t.to_owned())))
.collect(),
else_expr,
)))
Ok(Expr::Case(Case::new(expr, when_then_expr, else_expr)))
}
}

Expand Down
25 changes: 13 additions & 12 deletions datafusion/sql/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use datafusion_expr::planner::{
use sqlparser::ast::{
AccessExpr, BinaryOperator, CastFormat, CastKind, DataType as SQLDataType,
DictionaryField, Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, MapEntry,
StructField, Subscript, TrimWhereField, Value,
StructField, Subscript, TrimWhereField, Value, ValueWithSpan,
};

use datafusion_common::{
Expand Down Expand Up @@ -211,7 +211,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
// more context.
match sql {
SQLExpr::Value(value) => {
self.parse_value(value, planner_context.prepare_param_data_types())
self.parse_value(value.into(), planner_context.prepare_param_data_types())
}
SQLExpr::Extract { field, expr, .. } => {
let mut extract_args = vec![
Expand Down Expand Up @@ -253,12 +253,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
SQLExpr::Case {
operand,
conditions,
results,
else_result,
} => self.sql_case_identifier_to_expr(
operand,
conditions,
results,
else_result,
schema,
planner_context,
Expand Down Expand Up @@ -292,7 +290,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
}

SQLExpr::TypedString { data_type, value } => Ok(Expr::Cast(Cast::new(
Box::new(lit(value)),
Box::new(lit(value.into_string().unwrap())),
self.convert_data_type(&data_type)?,
))),

Expand Down Expand Up @@ -544,9 +542,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
planner_context,
)?),
match *time_zone {
SQLExpr::Value(Value::SingleQuotedString(s)) => {
DataType::Timestamp(TimeUnit::Nanosecond, Some(s.into()))
}
SQLExpr::Value(ValueWithSpan {
value: Value::SingleQuotedString(s),
span: _,
}) => DataType::Timestamp(TimeUnit::Nanosecond, Some(s.into())),
_ => {
return not_impl_err!(
"Unsupported ast node in sqltorel: {time_zone:?}"
Expand Down Expand Up @@ -999,10 +998,12 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
Subscript::Index { index } => {
// index can be a name, in which case it is a named field access
match index {
SQLExpr::Value(
Value::SingleQuotedString(s)
| Value::DoubleQuotedString(s),
) => Ok(Some(GetFieldAccess::NamedStructField {
SQLExpr::Value(ValueWithSpan {
value:
Value::SingleQuotedString(s)
| Value::DoubleQuotedString(s),
span: _,
}) => Ok(Some(GetFieldAccess::NamedStructField {
name: ScalarValue::from(s),
})),
SQLExpr::JsonAccess { .. } => {
Expand Down
12 changes: 8 additions & 4 deletions datafusion/sql/src/expr/order_by.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use datafusion_common::{
};
use datafusion_expr::expr::Sort;
use datafusion_expr::{Expr, SortExpr};
use sqlparser::ast::{Expr as SQLExpr, OrderByExpr, Value};
use sqlparser::ast::{
Expr as SQLExpr, OrderByExpr, OrderByOptions, Value, ValueWithSpan,
};

impl<S: ContextProvider> SqlToRel<'_, S> {
/// Convert sql [OrderByExpr] to `Vec<Expr>`.
Expand Down Expand Up @@ -62,9 +64,8 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
let mut expr_vec = vec![];
for e in exprs {
let OrderByExpr {
asc,
expr,
nulls_first,
options: OrderByOptions { asc, nulls_first },
with_fill,
} = e;

Expand All @@ -73,7 +74,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
}

let expr = match expr {
SQLExpr::Value(Value::Number(v, _)) if literal_to_column => {
SQLExpr::Value(ValueWithSpan {
value: Value::Number(v, _),
span: _,
}) if literal_to_column => {
let field_index = v
.parse::<usize>()
.map_err(|err| plan_datafusion_err!("{}", err))?;
Expand Down
9 changes: 5 additions & 4 deletions datafusion/sql/src/expr/unary_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use datafusion_expr::{
type_coercion::{is_interval, is_timestamp},
Expr, ExprSchemable,
};
use sqlparser::ast::{Expr as SQLExpr, UnaryOperator, Value};
use sqlparser::ast::{Expr as SQLExpr, UnaryOperator, Value, ValueWithSpan};

impl<S: ContextProvider> SqlToRel<'_, S> {
pub(crate) fn parse_sql_unary_op(
Expand Down Expand Up @@ -52,9 +52,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
match expr {
// Optimization: if it's a number literal, we apply the negative operator
// here directly to calculate the new literal.
SQLExpr::Value(Value::Number(n, _)) => {
self.parse_sql_number(&n, true)
}
SQLExpr::Value(ValueWithSpan {
value: Value::Number(n, _),
span: _,
}) => self.parse_sql_number(&n, true),
SQLExpr::Interval(interval) => {
self.sql_interval_to_expr(true, interval)
}
Expand Down
14 changes: 11 additions & 3 deletions datafusion/sql/src/expr/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ use datafusion_expr::expr::{BinaryExpr, Placeholder};
use datafusion_expr::planner::PlannerResult;
use datafusion_expr::{lit, Expr, Operator};
use log::debug;
use sqlparser::ast::{BinaryOperator, Expr as SQLExpr, Interval, UnaryOperator, Value};
use sqlparser::ast::{
BinaryOperator, Expr as SQLExpr, Interval, UnaryOperator, Value, ValueWithSpan,
};
use sqlparser::parser::ParserError::ParserError;
use std::borrow::Cow;
use std::cmp::Ordering;
Expand Down Expand Up @@ -254,8 +256,14 @@ impl<S: ContextProvider> SqlToRel<'_, S> {

fn interval_literal(interval_value: SQLExpr, negative: bool) -> Result<String> {
let s = match interval_value {
SQLExpr::Value(Value::SingleQuotedString(s) | Value::DoubleQuotedString(s)) => s,
SQLExpr::Value(Value::Number(ref v, long)) => {
SQLExpr::Value(ValueWithSpan {
value: Value::SingleQuotedString(s) | Value::DoubleQuotedString(s),
span: _,
}) => s,
SQLExpr::Value(ValueWithSpan {
value: Value::Number(ref v, long),
span: _,
}) => {
if long {
return not_impl_err!(
"Unsupported interval argument. Long number not supported: {interval_value:?}"
Expand Down
Loading