Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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.44", features = ["macros", "rt", "sync"] }
url = "2.5.4"
Expand Down
6 changes: 3 additions & 3 deletions datafusion/datasource/src/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use arrow::{
compute::SortColumn,
row::{Row, Rows},
};
use datafusion_common::{plan_err, DataFusionError, Result};
use datafusion_common::{plan_datafusion_err, plan_err, DataFusionError, Result};
use datafusion_physical_expr::{expressions::Column, PhysicalSortExpr};
use datafusion_physical_expr_common::sort_expr::LexOrdering;

Expand Down Expand Up @@ -202,10 +202,10 @@ impl MinMaxStatistics {
.zip(max_values.column_by_name(column.name()))
}
.ok_or_else(|| {
DataFusionError::Plan(format!(
plan_datafusion_err!(
"missing column in MinMaxStatistics::new: '{}'",
column.name()
))
)
})
})
.collect::<Result<Vec<_>>>()?
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
14 changes: 11 additions & 3 deletions datafusion/sql/src/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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 @@ -216,13 +216,21 @@ 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())
match name.0[0].as_ident() {
Some(ident) => crate::utils::normalize_ident(ident.clone()),
None => {
return plan_err!(
"Expected an identifier in function name, but found {:?}",
name.0[0]
)
}
}
};

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
32 changes: 17 additions & 15 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 @@ -1062,10 +1061,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 Expand Up @@ -1128,9 +1129,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
}
}
AccessExpr::Dot(expr) => match expr {
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),
})),
_ => {
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 @@ -61,9 +61,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