Skip to content

Commit 6c89207

Browse files
committed
chore: cargo fmt & remove unwrap
1 parent a916879 commit 6c89207

File tree

7 files changed

+138
-97
lines changed

7 files changed

+138
-97
lines changed

datafusion/sql/src/expr/function.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,15 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
222222
// (e.g. "foo.bar") for function names yet
223223
name.to_string()
224224
} else {
225-
crate::utils::normalize_ident(name.0[0].as_ident().unwrap().clone())
225+
match name.0[0].as_ident() {
226+
Some(ident) => crate::utils::normalize_ident(ident.clone()),
227+
None => {
228+
return plan_err!(
229+
"Expected an identifier in function name, but found {:?}",
230+
name.0[0]
231+
)
232+
}
233+
}
226234
};
227235

228236
if name.eq("make_map") {

datafusion/sql/src/expr/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,9 +1129,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
11291129
}
11301130
}
11311131
AccessExpr::Dot(expr) => match expr {
1132-
SQLExpr::Value(
1133-
Value::SingleQuotedString(s) | Value::DoubleQuotedString(s),
1134-
) => Ok(Some(GetFieldAccess::NamedStructField {
1132+
SQLExpr::Value(ValueWithSpan {
1133+
value: Value::SingleQuotedString(s) | Value::DoubleQuotedString(s),
1134+
span : _
1135+
}) => Ok(Some(GetFieldAccess::NamedStructField {
11351136
name: ScalarValue::from(s),
11361137
})),
11371138
_ => {

datafusion/sql/src/planner.rs

Lines changed: 112 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,17 @@ impl IdentNormalizer {
177177
}
178178
}
179179

180-
/// Struct to store the states used by the Planner. The Planner will leverage the states to resolve
181-
/// CTEs, Views, subqueries and PREPARE statements. The states include
180+
/// Struct to store the states used by the Planner. The Planner will leverage the states
181+
/// to resolve CTEs, Views, subqueries and PREPARE statements. The states include
182182
/// Common Table Expression (CTE) provided with WITH clause and
183183
/// Parameter Data Types provided with PREPARE statement and the query schema of the
184184
/// outer query plan.
185185
///
186186
/// # Cloning
187187
///
188-
/// Only the `ctes` are truly cloned when the `PlannerContext` is cloned. This helps resolve
189-
/// scoping issues of CTEs. By using cloning, a subquery can inherit CTEs from the outer query
188+
/// Only the `ctes` are truly cloned when the `PlannerContext` is cloned.
189+
/// This helps resolve scoping issues of CTEs.
190+
/// By using cloning, a subquery can inherit CTEs from the outer query
190191
/// and can also define its own private CTEs without affecting the outer query.
191192
///
192193
#[derive(Debug, Clone)]
@@ -329,7 +330,8 @@ impl PlannerContext {
329330
/// by subsequent passes.
330331
///
331332
/// Key interfaces are:
332-
/// * [`Self::sql_statement_to_plan`]: Convert a statement (e.g. `SELECT ...`) into a [`LogicalPlan`]
333+
/// * [`Self::sql_statement_to_plan`]: Convert a statement
334+
/// (e.g. `SELECT ...`) into a [`LogicalPlan`]
333335
/// * [`Self::sql_to_expr`]: Convert an expression (e.g. `1 + 2`) into an [`Expr`]
334336
pub struct SqlToRel<'a, S: ContextProvider> {
335337
pub(crate) context_provider: &'a S,
@@ -442,7 +444,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
442444
Ok(plan)
443445
} else if idents.len() != plan.schema().fields().len() {
444446
plan_err!(
445-
"Source table contains {} columns but only {} names given as column alias",
447+
"Source table contains {} columns but only {} \
448+
names given as column alias",
446449
plan.schema().fields().len(),
447450
idents.len()
448451
)
@@ -556,16 +559,21 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
556559
SQLDataType::Boolean | SQLDataType::Bool => Ok(DataType::Boolean),
557560
SQLDataType::TinyInt(_) => Ok(DataType::Int8),
558561
SQLDataType::SmallInt(_) | SQLDataType::Int2(_) => Ok(DataType::Int16),
559-
SQLDataType::Int(_) | SQLDataType::Integer(_) | SQLDataType::Int4(_) => Ok(DataType::Int32),
562+
SQLDataType::Int(_) | SQLDataType::Integer(_) | SQLDataType::Int4(_) => {
563+
Ok(DataType::Int32)
564+
}
560565
SQLDataType::BigInt(_) | SQLDataType::Int8(_) => Ok(DataType::Int64),
561566
SQLDataType::TinyIntUnsigned(_) => Ok(DataType::UInt8),
562-
SQLDataType::SmallIntUnsigned(_) | SQLDataType::Int2Unsigned(_) => Ok(DataType::UInt16),
563-
SQLDataType::IntUnsigned(_) | SQLDataType::IntegerUnsigned(_) | SQLDataType::Int4Unsigned(_) => {
564-
Ok(DataType::UInt32)
565-
}
567+
SQLDataType::SmallIntUnsigned(_) | SQLDataType::Int2Unsigned(_) => {
568+
Ok(DataType::UInt16)
569+
}
570+
SQLDataType::IntUnsigned(_)
571+
| SQLDataType::IntegerUnsigned(_)
572+
| SQLDataType::Int4Unsigned(_) => Ok(DataType::UInt32),
566573
SQLDataType::Varchar(length) => {
567574
match (length, self.options.support_varchar_with_length) {
568-
(Some(_), false) => plan_err!("does not support Varchar with length, please set `support_varchar_with_length` to be true"),
575+
(Some(_), false) => plan_err!("does not support Varchar with length, \
576+
please set `support_varchar_with_length` to be true"),
569577
_ => {
570578
if self.options.map_varchar_to_utf8view {
571579
Ok(DataType::Utf8View)
@@ -575,83 +583,90 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
575583
}
576584
}
577585
}
578-
SQLDataType::BigIntUnsigned(_) | SQLDataType::Int8Unsigned(_) => Ok(DataType::UInt64),
586+
SQLDataType::BigIntUnsigned(_) | SQLDataType::Int8Unsigned(_) => {
587+
Ok(DataType::UInt64)
588+
}
579589
SQLDataType::Float(_) => Ok(DataType::Float32),
580590
SQLDataType::Real | SQLDataType::Float4 => Ok(DataType::Float32),
581-
SQLDataType::Double(ExactNumberInfo::None) | SQLDataType::DoublePrecision | SQLDataType::Float8 => Ok(DataType::Float64),
582-
SQLDataType::Double(ExactNumberInfo::Precision(_)|ExactNumberInfo::PrecisionAndScale(_, _)) => {
583-
not_impl_err!("Unsupported SQL type (precision/scale not supported) {sql_type}")
584-
}
585-
SQLDataType::Char(_)
586-
| SQLDataType::Text
587-
| SQLDataType::String(_) => Ok(DataType::Utf8),
591+
SQLDataType::Double(ExactNumberInfo::None)
592+
| SQLDataType::DoublePrecision
593+
| SQLDataType::Float8 => Ok(DataType::Float64),
594+
SQLDataType::Double(
595+
ExactNumberInfo::Precision(_) | ExactNumberInfo::PrecisionAndScale(_, _),
596+
) => {
597+
not_impl_err!(
598+
"Unsupported SQL type (precision/scale not supported) {sql_type}"
599+
)
600+
}
601+
SQLDataType::Char(_) | SQLDataType::Text | SQLDataType::String(_) => {
602+
Ok(DataType::Utf8)
603+
}
588604
SQLDataType::Timestamp(precision, tz_info)
589-
if precision.is_none() || [0, 3, 6, 9].contains(&precision.unwrap()) => {
590-
let tz = if matches!(tz_info, TimezoneInfo::Tz)
591-
|| matches!(tz_info, TimezoneInfo::WithTimeZone)
592-
{
593-
// Timestamp With Time Zone
594-
// INPUT : [SQLDataType] TimestampTz + [Config] Time Zone
595-
// OUTPUT: [ArrowDataType] Timestamp<TimeUnit, Some(Time Zone)>
596-
self.context_provider.options().execution.time_zone.clone()
597-
} else {
598-
// Timestamp Without Time zone
599-
None
600-
};
601-
let precision = match precision {
602-
Some(0) => TimeUnit::Second,
603-
Some(3) => TimeUnit::Millisecond,
604-
Some(6) => TimeUnit::Microsecond,
605-
None | Some(9) => TimeUnit::Nanosecond,
606-
_ => unreachable!(),
607-
};
608-
Ok(DataType::Timestamp(precision, tz.map(Into::into)))
609-
}
605+
if precision.is_none() || [0, 3, 6, 9].contains(&precision.unwrap()) =>
606+
{
607+
let tz = if matches!(tz_info, TimezoneInfo::Tz)
608+
|| matches!(tz_info, TimezoneInfo::WithTimeZone)
609+
{
610+
// Timestamp With Time Zone
611+
// INPUT : [SQLDataType] TimestampTz + [Config] Time Zone
612+
// OUTPUT: [ArrowDataType] Timestamp<TimeUnit, Some(Time Zone)>
613+
self.context_provider.options().execution.time_zone.clone()
614+
} else {
615+
// Timestamp Without Time zone
616+
None
617+
};
618+
let precision = match precision {
619+
Some(0) => TimeUnit::Second,
620+
Some(3) => TimeUnit::Millisecond,
621+
Some(6) => TimeUnit::Microsecond,
622+
None | Some(9) => TimeUnit::Nanosecond,
623+
_ => unreachable!(),
624+
};
625+
Ok(DataType::Timestamp(precision, tz.map(Into::into)))
626+
}
610627
SQLDataType::Date => Ok(DataType::Date32),
611628
SQLDataType::Time(None, tz_info) => {
612-
if matches!(tz_info, TimezoneInfo::None)
613-
|| matches!(tz_info, TimezoneInfo::WithoutTimeZone)
614-
{
615-
Ok(DataType::Time64(TimeUnit::Nanosecond))
616-
} else {
617-
// We don't support TIMETZ and TIME WITH TIME ZONE for now
618-
not_impl_err!(
619-
"Unsupported SQL type {sql_type:?}"
620-
)
621-
}
622-
}
629+
if matches!(tz_info, TimezoneInfo::None)
630+
|| matches!(tz_info, TimezoneInfo::WithoutTimeZone)
631+
{
632+
Ok(DataType::Time64(TimeUnit::Nanosecond))
633+
} else {
634+
// We don't support TIMETZ and TIME WITH TIME ZONE for now
635+
not_impl_err!("Unsupported SQL type {sql_type:?}")
636+
}
637+
}
623638
SQLDataType::Numeric(exact_number_info)
624-
| SQLDataType::Decimal(exact_number_info) => {
625-
let (precision, scale) = match *exact_number_info {
626-
ExactNumberInfo::None => (None, None),
627-
ExactNumberInfo::Precision(precision) => (Some(precision), None),
628-
ExactNumberInfo::PrecisionAndScale(precision, scale) => {
629-
(Some(precision), Some(scale))
630-
}
631-
};
632-
make_decimal_type(precision, scale)
639+
| SQLDataType::Decimal(exact_number_info) => {
640+
let (precision, scale) = match *exact_number_info {
641+
ExactNumberInfo::None => (None, None),
642+
ExactNumberInfo::Precision(precision) => (Some(precision), None),
643+
ExactNumberInfo::PrecisionAndScale(precision, scale) => {
644+
(Some(precision), Some(scale))
633645
}
646+
};
647+
make_decimal_type(precision, scale)
648+
}
634649
SQLDataType::Bytea => Ok(DataType::Binary),
635650
SQLDataType::Interval => Ok(DataType::Interval(IntervalUnit::MonthDayNano)),
636651
SQLDataType::Struct(fields, _) => {
637-
let fields = fields
638-
.iter()
639-
.enumerate()
640-
.map(|(idx, field)| {
641-
let data_type = self.convert_data_type(&field.field_type)?;
642-
let field_name = match &field.field_name{
643-
Some(ident) => ident.clone(),
644-
None => Ident::new(format!("c{idx}"))
645-
};
646-
Ok(Arc::new(Field::new(
647-
self.ident_normalizer.normalize(field_name),
648-
data_type,
649-
true,
650-
)))
651-
})
652-
.collect::<Result<Vec<_>>>()?;
653-
Ok(DataType::Struct(Fields::from(fields)))
654-
}
652+
let fields = fields
653+
.iter()
654+
.enumerate()
655+
.map(|(idx, field)| {
656+
let data_type = self.convert_data_type(&field.field_type)?;
657+
let field_name = match &field.field_name {
658+
Some(ident) => ident.clone(),
659+
None => Ident::new(format!("c{idx}")),
660+
};
661+
Ok(Arc::new(Field::new(
662+
self.ident_normalizer.normalize(field_name),
663+
data_type,
664+
true,
665+
)))
666+
})
667+
.collect::<Result<Vec<_>>>()?;
668+
Ok(DataType::Struct(Fields::from(fields)))
669+
}
655670
SQLDataType::Nvarchar(_)
656671
| SQLDataType::JSON
657672
| SQLDataType::Uuid
@@ -843,7 +858,7 @@ pub(crate) fn idents_to_table_reference(
843858
pub fn object_name_to_qualifier(
844859
sql_table_name: &ObjectName,
845860
enable_normalization: bool,
846-
) -> String {
861+
) -> Result<String> {
847862
let columns = vec!["table_name", "table_schema", "table_catalog"].into_iter();
848863
let normalizer = IdentNormalizer::new(enable_normalization);
849864
sql_table_name
@@ -852,12 +867,22 @@ pub fn object_name_to_qualifier(
852867
.rev()
853868
.zip(columns)
854869
.map(|(object_name_part, column_name)| {
855-
format!(
856-
r#"{} = '{}'"#,
857-
column_name,
858-
normalizer.normalize(object_name_part.as_ident().unwrap().clone())
859-
)
870+
object_name_part
871+
.as_ident()
872+
.map(|ident| {
873+
format!(
874+
r#"{} = '{}'"#,
875+
column_name,
876+
normalizer.normalize(ident.clone())
877+
)
878+
})
879+
.ok_or_else(|| {
880+
DataFusionError::Plan(format!(
881+
"Expected identifier, but found: {:?}",
882+
object_name_part
883+
))
884+
})
860885
})
861-
.collect::<Vec<_>>()
862-
.join(" AND ")
886+
.collect::<Result<Vec<_>>>()
887+
.map(|parts| parts.join(" AND "))
863888
}

datafusion/sql/src/query.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
2222
use crate::stack::StackGuard;
2323
use datafusion_common::{not_impl_err, Constraints, DFSchema, Result};
2424
use datafusion_expr::expr::Sort;
25+
use datafusion_expr::select_expr::SelectExpr;
2526
use datafusion_expr::{
26-
CreateMemoryTable, DdlStatement, Distinct, Expr, LogicalPlan, LogicalPlanBuilder,
27+
CreateMemoryTable, DdlStatement, Distinct, LogicalPlan, LogicalPlanBuilder,
2728
};
2829
use sqlparser::ast::{
2930
Expr as SQLExpr, Offset as SQLOffset, OrderBy, OrderByExpr, OrderByKind, Query,
@@ -157,7 +158,7 @@ fn to_order_by_exprs(order_by: Option<OrderBy>) -> Result<Vec<OrderByExpr>> {
157158
/// Returns the order by expressions from the query with the select expressions.
158159
pub(crate) fn to_order_by_exprs_with_select(
159160
order_by: Option<OrderBy>,
160-
_select_exprs: Option<Vec<Expr>>, // TODO: ORDER BY ALL
161+
_select_exprs: Option<Vec<SelectExpr>>, // TODO: ORDER BY ALL
161162
) -> Result<Vec<OrderByExpr>> {
162163
let Some(OrderBy { kind, interpolate }) = order_by else {
163164
// If no order by, return an empty array.

datafusion/sql/src/relation/join.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
136136
)
137137
} else {
138138
let id = object_names.swap_remove(0);
139-
Ok(self.ident_normalizer.normalize(id.as_ident().unwrap().clone()))
139+
id.as_ident()
140+
.ok_or_else(|| {
141+
datafusion_common::DataFusionError::Plan(
142+
"Expected identifier in USING clause".to_string(),
143+
)
144+
})
145+
.map(|ident| self.ident_normalizer.normalize(ident.clone()))
140146
}
141147
})
142148
.collect::<Result<Vec<_>>>()?;

datafusion/sql/src/statement.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,7 +2087,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
20872087
let where_clause = object_name_to_qualifier(
20882088
&sql_table_name,
20892089
self.options.enable_ident_normalization,
2090-
);
2090+
)?;
20912091

20922092
if !self.has_table("information_schema", "columns") {
20932093
return plan_err!(
@@ -2212,7 +2212,7 @@ ON p.function_name = r.routine_name
22122212
let where_clause = object_name_to_qualifier(
22132213
&sql_table_name,
22142214
self.options.enable_ident_normalization,
2215-
);
2215+
)?;
22162216

22172217
// Do a table lookup to verify the table exists
22182218
let table_ref = self.object_name_to_table_reference(sql_table_name)?;

datafusion/sql/src/unparser/plan.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ impl Unparser<'_> {
668668
));
669669
}
670670
exists_select.projection(vec![ast::SelectItem::UnnamedExpr(
671-
ast::Expr::Value(ast::Value::Number("1".to_string(), false)),
671+
ast::Expr::value(ast::Value::Number("1".to_string(), false)),
672672
)]);
673673
query_builder.body(Box::new(SetExpr::Select(Box::new(
674674
exists_select.build()?,

0 commit comments

Comments
 (0)