diff --git a/Cargo.lock b/Cargo.lock index 0746f0f2814e7..f670bd4c03519 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6106,9 +6106,9 @@ dependencies = [ [[package]] name = "sqlparser" -version = "0.55.0" +version = "0.58.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4521174166bac1ff04fe16ef4524c70144cd29682a45978978ca3d7f4e0be11" +checksum = "ec4b661c54b1e4b603b37873a18c59920e4c51ea8ea2cf527d925424dbd4437c" dependencies = [ "log", "recursive", diff --git a/Cargo.toml b/Cargo.toml index c9db705795066..5915035cd0bab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -173,7 +173,7 @@ recursive = "0.1.1" regex = "1.8" rstest = "0.25.0" serde_json = "1" -sqlparser = { version = "0.55.0", default-features = false, features = ["std", "visitor"] } +sqlparser = { version = "0.58.0", default-features = false, features = ["std", "visitor"] } tempfile = "3" testcontainers = { version = "0.24", features = ["default"] } testcontainers-modules = { version = "0.12" } diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index e92869873731f..ae4cddc61f548 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -254,6 +254,8 @@ impl SqlToRel<'_, S> { operand, conditions, else_result, + case_token: _, + end_token: _, } => self.sql_case_identifier_to_expr( operand, conditions, @@ -446,6 +448,7 @@ impl SqlToRel<'_, S> { substring_from, substring_for, special: _, + shorthand: _, } => self.sql_substring_to_expr( expr, substring_from, @@ -813,7 +816,7 @@ impl SqlToRel<'_, S> { negated: bool, expr: SQLExpr, pattern: SQLExpr, - escape_char: Option, + escape_char: Option, schema: &DFSchema, planner_context: &mut PlannerContext, case_insensitive: bool, @@ -823,13 +826,12 @@ impl SqlToRel<'_, S> { return not_impl_err!("ANY in LIKE expression"); } let pattern = self.sql_expr_to_logical_expr(pattern, schema, planner_context)?; - let escape_char = if let Some(char) = escape_char { - if char.len() != 1 { - return plan_err!("Invalid escape character in LIKE expression"); + let escape_char = match escape_char { + Some(Value::SingleQuotedString(char)) if char.len() == 1 => { + Some(char.chars().next().unwrap()) } - Some(char.chars().next().unwrap()) - } else { - None + Some(value) => return plan_err!("Invalid escape character in LIKE expression. Expected a single character wrapped with single quotes, got {value}"), + None => None, }; Ok(Expr::Like(Like::new( negated, @@ -845,7 +847,7 @@ impl SqlToRel<'_, S> { negated: bool, expr: SQLExpr, pattern: SQLExpr, - escape_char: Option, + escape_char: Option, schema: &DFSchema, planner_context: &mut PlannerContext, ) -> Result { @@ -854,13 +856,12 @@ impl SqlToRel<'_, S> { if pattern_type != DataType::Utf8 && pattern_type != DataType::Null { return plan_err!("Invalid pattern in SIMILAR TO expression"); } - let escape_char = if let Some(char) = escape_char { - if char.len() != 1 { - return plan_err!("Invalid escape character in SIMILAR TO expression"); + let escape_char = match escape_char { + Some(Value::SingleQuotedString(char)) if char.len() == 1 => { + Some(char.chars().next().unwrap()) } - Some(char.chars().next().unwrap()) - } else { - None + Some(value) => return plan_err!("Invalid escape character in SIMILAR TO expression. Expected a single character wrapped with single quotes, got {value}"), + None => None, }; Ok(Expr::SimilarTo(Like::new( negated, diff --git a/datafusion/sql/src/expr/subquery.rs b/datafusion/sql/src/expr/subquery.rs index 602d39233d587..24bb813634cc1 100644 --- a/datafusion/sql/src/expr/subquery.rs +++ b/datafusion/sql/src/expr/subquery.rs @@ -58,7 +58,7 @@ impl SqlToRel<'_, S> { planner_context.set_outer_query_schema(Some(input_schema.clone().into())); let mut spans = Spans::new(); - if let SetExpr::Select(select) = subquery.body.as_ref() { + if let SetExpr::Select(select) = &subquery.body.as_ref() { for item in &select.projection { if let SelectItem::UnnamedExpr(SQLExpr::Identifier(ident)) = item { if let Some(span) = Span::try_from_sqlparser_span(ident.span) { diff --git a/datafusion/sql/src/expr/substring.rs b/datafusion/sql/src/expr/substring.rs index 8f6e77e035c12..0cb47a770f9d1 100644 --- a/datafusion/sql/src/expr/substring.rs +++ b/datafusion/sql/src/expr/substring.rs @@ -18,8 +18,8 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; use datafusion_common::{not_impl_err, plan_err}; use datafusion_common::{DFSchema, Result, ScalarValue}; -use datafusion_expr::planner::PlannerResult; -use datafusion_expr::Expr; +use datafusion_expr::{planner::PlannerResult, Expr}; + use sqlparser::ast::Expr as SQLExpr; impl SqlToRel<'_, S> { @@ -62,12 +62,14 @@ impl SqlToRel<'_, S> { substring_from: None, substring_for: None, special: false, + shorthand: false, }; return plan_err!("Substring without for/from is not valid {orig_sql:?}"); } }; + // Try to plan the substring expression using one of the registered planners for planner in self.context_provider.get_expr_planners() { match planner.plan_substring(substring_args)? { PlannerResult::Planned(expr) => return Ok(expr), @@ -77,8 +79,6 @@ impl SqlToRel<'_, S> { } } - not_impl_err!( - "Substring not supported by UserDefinedExtensionPlanners: {substring_args:?}" - ) + not_impl_err!("Substring could not be planned by registered expr planner. Hint: enable the `unicode_expressions" ) } } diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs index 2cb1dbdcb4acd..fc678a8f87112 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -809,6 +809,15 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { | SQLDataType::AnyType | SQLDataType::Table(_) | SQLDataType::VarBit(_) + | SQLDataType::UTinyInt + | SQLDataType::USmallInt + | SQLDataType::HugeInt + | SQLDataType::UHugeInt + | SQLDataType::UBigInt + | SQLDataType::TimestampNtz + | SQLDataType::NamedTable { .. } + | SQLDataType::TsVector + | SQLDataType::TsQuery | SQLDataType::GeometricType(_) => { not_impl_err!("Unsupported SQL type {sql_type:?}") } diff --git a/datafusion/sql/src/query.rs b/datafusion/sql/src/query.rs index 2ea2299c1fcfd..c06147e08f87a 100644 --- a/datafusion/sql/src/query.rs +++ b/datafusion/sql/src/query.rs @@ -27,8 +27,8 @@ use datafusion_expr::{ CreateMemoryTable, DdlStatement, Distinct, Expr, LogicalPlan, LogicalPlanBuilder, }; use sqlparser::ast::{ - Expr as SQLExpr, Ident, Offset as SQLOffset, OrderBy, OrderByExpr, OrderByKind, - Query, SelectInto, SetExpr, + Expr as SQLExpr, Ident, LimitClause, OrderBy, OrderByExpr, OrderByKind, Query, + SelectInto, SetExpr, }; use sqlparser::tokenizer::Span; @@ -54,8 +54,7 @@ impl SqlToRel<'_, S> { let select_into = select.into.take(); let plan = self.select_to_plan(*select, query.order_by, planner_context)?; - let plan = - self.limit(plan, query.offset, query.limit, planner_context)?; + let plan = self.limit(plan, query.limit_clause, planner_context)?; // Process the `SELECT INTO` after `LIMIT`. self.select_into(plan, select_into) } @@ -77,7 +76,7 @@ impl SqlToRel<'_, S> { None, )?; let plan = self.order_by(plan, order_by_rex)?; - self.limit(plan, query.offset, query.limit, planner_context) + self.limit(plan, query.limit_clause, planner_context) } } } @@ -86,23 +85,53 @@ impl SqlToRel<'_, S> { fn limit( &self, input: LogicalPlan, - skip: Option, - fetch: Option, + limit_clause: Option, planner_context: &mut PlannerContext, ) -> Result { - if skip.is_none() && fetch.is_none() { + let Some(limit_clause) = limit_clause else { return Ok(input); - } + }; - // skip and fetch expressions are not allowed to reference columns from the input plan let empty_schema = DFSchema::empty(); - let skip = skip - .map(|o| self.sql_to_expr(o.value, &empty_schema, planner_context)) - .transpose()?; - let fetch = fetch - .map(|e| self.sql_to_expr(e, &empty_schema, planner_context)) - .transpose()?; + let (skip, fetch, limit_by_exprs) = match limit_clause { + LimitClause::LimitOffset { + limit, + offset, + limit_by, + } => { + let skip = offset + .map(|o| self.sql_to_expr(o.value, &empty_schema, planner_context)) + .transpose()?; + + let fetch = limit + .map(|e| self.sql_to_expr(e, &empty_schema, planner_context)) + .transpose()?; + + let limit_by_exprs = limit_by + .into_iter() + .map(|e| self.sql_to_expr(e, &empty_schema, planner_context)) + .collect::>>()?; + + (skip, fetch, limit_by_exprs) + } + LimitClause::OffsetCommaLimit { offset, limit } => { + let skip = + Some(self.sql_to_expr(offset, &empty_schema, planner_context)?); + let fetch = + Some(self.sql_to_expr(limit, &empty_schema, planner_context)?); + (skip, fetch, vec![]) + } + }; + + if !limit_by_exprs.is_empty() { + return not_impl_err!("LIMIT BY clause is not supported yet"); + } + + if skip.is_none() && fetch.is_none() { + return Ok(input); + } + LogicalPlanBuilder::from(input) .limit_by_expr(skip, fetch)? .build() diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 68be37f498387..2d9867b09927d 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -55,16 +55,16 @@ use datafusion_expr::{ Volatility, WriteOp, }; use sqlparser::ast::{ - self, BeginTransactionKind, NullsDistinctOption, ShowStatementIn, - ShowStatementOptions, SqliteOnConflict, TableObject, UpdateTableFromKind, - ValueWithSpan, + self, BeginTransactionKind, IndexColumn, IndexType, NullsDistinctOption, OrderByExpr, + OrderByOptions, Set, ShowStatementIn, ShowStatementOptions, SqliteOnConflict, + TableObject, UpdateTableFromKind, ValueWithSpan, }; use sqlparser::ast::{ Assignment, AssignmentTarget, ColumnDef, CreateIndex, CreateTable, CreateTableOptions, Delete, DescribeAlias, Expr as SQLExpr, FromTable, Ident, Insert, - ObjectName, ObjectType, OneOrManyWithParens, Query, SchemaName, SetExpr, - ShowCreateObject, ShowStatementFilter, Statement, TableConstraint, TableFactor, - TableWithJoins, TransactionMode, UnaryOperator, Value, + ObjectName, ObjectType, Query, SchemaName, SetExpr, ShowCreateObject, + ShowStatementFilter, Statement, TableConstraint, TableFactor, TableWithJoins, + TransactionMode, UnaryOperator, Value, }; use sqlparser::parser::ParserError::ParserError; @@ -111,7 +111,17 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec constraints.push(TableConstraint::Unique { name: name.clone(), - columns: vec![column.name.clone()], + columns: vec![IndexColumn { + column: OrderByExpr { + expr: SQLExpr::Identifier(column.name.clone()), + options: OrderByOptions { + asc: None, + nulls_first: None, + }, + with_fill: None, + }, + operator_class: None, + }], characteristics: *characteristics, index_name: None, index_type_display: ast::KeyOrIndexDisplay::None, @@ -124,7 +134,17 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec constraints.push(TableConstraint::PrimaryKey { name: name.clone(), - columns: vec![column.name.clone()], + columns: vec![IndexColumn { + column: OrderByExpr { + expr: SQLExpr::Identifier(column.name.clone()), + options: OrderByOptions { + asc: None, + nulls_first: None, + }, + with_fill: None, + }, + operator_class: None, + }], characteristics: *characteristics, index_name: None, index_type: None, @@ -144,11 +164,13 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { constraints.push(TableConstraint::Check { name: name.clone(), expr: Box::new(expr.clone()), + enforced: None, }) } // Other options are not constraint related. @@ -168,6 +190,7 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec {} } } @@ -233,13 +256,7 @@ impl SqlToRel<'_, S> { } Statement::Query(query) => self.query_to_plan(*query, planner_context), Statement::ShowVariable { variable } => self.show_variable_to_plan(&variable), - Statement::SetVariable { - local, - hivevar, - variables, - value, - } => self.set_variable_to_plan(local, hivevar, &variables, value), - + Statement::Set(statement) => self.set_statement_to_plan(statement), Statement::CreateTable(CreateTable { temporary, external, @@ -254,18 +271,12 @@ impl SqlToRel<'_, S> { name, columns, constraints, - table_properties, - with_options, if_not_exists, or_replace, without_rowid, like, clone, - engine, comment, - auto_increment_offset, - default_charset, - collation, on_commit, on_cluster, primary_key, @@ -273,7 +284,6 @@ impl SqlToRel<'_, S> { partition_by, cluster_by, clustered_by, - options, strict, copy_grants, enable_schema_evolution, @@ -290,7 +300,9 @@ impl SqlToRel<'_, S> { catalog, catalog_sync, storage_serialization_policy, - }) if table_properties.is_empty() && with_options.is_empty() => { + inherits, + table_options: CreateTableOptions::None, + }) => { if temporary { return not_impl_err!("Temporary tables not supported")?; } @@ -339,21 +351,9 @@ impl SqlToRel<'_, S> { if clone.is_some() { return not_impl_err!("Clone not supported")?; } - if engine.is_some() { - return not_impl_err!("Engine not supported")?; - } if comment.is_some() { return not_impl_err!("Comment not supported")?; } - if auto_increment_offset.is_some() { - return not_impl_err!("Auto increment offset not supported")?; - } - if default_charset.is_some() { - return not_impl_err!("Default charset not supported")?; - } - if collation.is_some() { - return not_impl_err!("Collation not supported")?; - } if on_commit.is_some() { return not_impl_err!("On commit not supported")?; } @@ -375,9 +375,6 @@ impl SqlToRel<'_, S> { if clustered_by.is_some() { return not_impl_err!("Clustered by not supported")?; } - if options.is_some() { - return not_impl_err!("Options not supported")?; - } if strict { return not_impl_err!("Strict not supported")?; } @@ -428,6 +425,9 @@ impl SqlToRel<'_, S> { if storage_serialization_policy.is_some() { return not_impl_err!("Storage serialization policy not supported")?; } + if inherits.is_some() { + return not_impl_err!("Table inheritance not supported")?; + } // Merge inline constraints and existing constraints let mut all_constraints = constraints; @@ -451,10 +451,10 @@ impl SqlToRel<'_, S> { let plan = if has_columns { if schema.fields().len() != input_schema.fields().len() { return plan_err!( - "Mismatch: {} columns specified, but result has {} columns", - schema.fields().len(), - input_schema.fields().len() - ); + "Mismatch: {} columns specified, but result has {} columns", + schema.fields().len(), + input_schema.fields().len() + ); } let input_fields = input_schema.fields(); let project_exprs = schema @@ -534,6 +534,7 @@ impl SqlToRel<'_, S> { temporary, to, params, + or_alter, } => { if materialized { return not_impl_err!("Materialized views not supported")?; @@ -570,6 +571,7 @@ impl SqlToRel<'_, S> { temporary, to, params, + or_alter, }; let sql = stmt.to_string(); let Statement::CreateView { @@ -617,6 +619,7 @@ impl SqlToRel<'_, S> { Statement::CreateSchema { schema_name, if_not_exists, + .. } => Ok(LogicalPlan::Ddl(DdlStatement::CreateCatalogSchema( CreateCatalogSchema { schema_name: get_schema_name(&schema_name), @@ -643,6 +646,7 @@ impl SqlToRel<'_, S> { restrict: _, purge: _, temporary: _, + table: _, } => { // We don't support cascade and purge for now. // nor do we support multiple object names @@ -738,6 +742,8 @@ impl SqlToRel<'_, S> { has_parentheses: _, immediate, into, + output, + default, } => { // `USING` is a MySQL-specific syntax and currently not supported. if !using.is_empty() { @@ -753,6 +759,16 @@ impl SqlToRel<'_, S> { if !into.is_empty() { return not_impl_err!("Execute statement with INTO is not supported"); } + if output { + return not_impl_err!( + "Execute statement with OUTPUT is not supported" + ); + } + if default { + return not_impl_err!( + "Execute statement with DEFAULT is not supported" + ); + } let empty_schema = DFSchema::empty(); let parameters = parameters .into_iter() @@ -1019,8 +1035,8 @@ impl SqlToRel<'_, S> { modifier, transaction, statements, - exception_statements, has_end_keyword, + exception, } => { if let Some(modifier) = modifier { return not_impl_err!( @@ -1032,7 +1048,7 @@ impl SqlToRel<'_, S> { "Transaction with multiple statements not supported" ); } - if exception_statements.is_some() { + if exception.is_some() { return not_impl_err!( "Transaction with exception statements not supported" ); @@ -1182,6 +1198,17 @@ impl SqlToRel<'_, S> { ast::CreateFunctionBody::AsBeforeOptions(expr) => expr, ast::CreateFunctionBody::AsAfterOptions(expr) => expr, ast::CreateFunctionBody::Return(expr) => expr, + ast::CreateFunctionBody::AsBeginEnd(_) => { + return not_impl_err!( + "BEGIN/END enclosed function body syntax is not supported" + )?; + } + ast::CreateFunctionBody::AsReturnExpr(_) + | ast::CreateFunctionBody::AsReturnSelect(_) => { + return not_impl_err!( + "AS RETURN function syntax is not supported" + )? + } }, &DFSchema::empty(), &mut planner_context, @@ -1251,9 +1278,15 @@ impl SqlToRel<'_, S> { .get_table_source(table.clone())? .schema() .to_dfschema_ref()?; - let using: Option = using.as_ref().map(ident_to_string); + let using: Option = + using.as_ref().map(|index_type| match index_type { + IndexType::Custom(ident) => ident_to_string(ident), + _ => index_type.to_string().to_ascii_lowercase(), + }); + let order_by_exprs: Vec = + columns.into_iter().map(|col| col.column).collect(); let columns = self.order_by_to_sort_expr( - columns, + order_by_exprs, &table_schema, planner_context, false, @@ -1537,13 +1570,21 @@ impl SqlToRel<'_, S> { fn get_constraint_column_indices( &self, df_schema: &DFSchemaRef, - columns: &[Ident], + columns: &[IndexColumn], constraint_name: &str, ) -> Result> { let field_names = df_schema.field_names(); columns .iter() - .map(|ident| { + .map(|index_column| { + let expr = &index_column.column.expr; + let ident = if let SQLExpr::Identifier(ident) = expr { + ident + } else { + return Err(plan_datafusion_err!( + "Column name for {constraint_name} must be an identifier: {expr}" + )); + }; let column = self.ident_normalizer.normalize(ident.clone()); field_names .iter() @@ -1741,64 +1782,58 @@ impl SqlToRel<'_, S> { self.statement_to_plan(rewrite.pop_front().unwrap()) } - fn set_variable_to_plan( - &self, - local: bool, - hivevar: bool, - variables: &OneOrManyWithParens, - value: Vec, - ) -> Result { - if local { - return not_impl_err!("LOCAL is not supported"); - } - - if hivevar { - return not_impl_err!("HIVEVAR is not supported"); - } + fn set_statement_to_plan(&self, statement: Set) -> Result { + match statement { + Set::SingleAssignment { + scope, + hivevar, + variable, + values, + } => { + if scope.is_some() { + return not_impl_err!("SET with scope modifiers is not supported"); + } - let variable = match variables { - OneOrManyWithParens::One(v) => object_name_to_string(v), - OneOrManyWithParens::Many(vs) => { - return not_impl_err!( - "SET only supports single variable assignment: {vs:?}" - ); - } - }; - let mut variable_lower = variable.to_lowercase(); + if hivevar { + return not_impl_err!("SET HIVEVAR is not supported"); + } - if variable_lower == "timezone" || variable_lower == "time.zone" { - // We could introduce alias in OptionDefinition if this string matching thing grows - variable_lower = "datafusion.execution.time_zone".to_string(); - } + let variable = object_name_to_string(&variable); + let mut variable_lower = variable.to_lowercase(); - // Parse value string from Expr - let value_string = match &value[0] { - SQLExpr::Identifier(i) => ident_to_string(i), - SQLExpr::Value(v) => match crate::utils::value_to_string(&v.value) { - None => { - return plan_err!("Unsupported Value {}", value[0]); + if variable_lower == "timezone" || variable_lower == "time.zone" { + variable_lower = "datafusion.execution.time_zone".to_string(); } - Some(v) => v, - }, - // For capture signed number e.g. +8, -8 - SQLExpr::UnaryOp { op, expr } => match op { - UnaryOperator::Plus => format!("+{expr}"), - UnaryOperator::Minus => format!("-{expr}"), - _ => { - return plan_err!("Unsupported Value {}", value[0]); + + if values.len() != 1 { + return plan_err!("SET only supports single value assignment"); } - }, - _ => { - return plan_err!("Unsupported Value {}", value[0]); - } - }; - let statement = PlanStatement::SetVariable(SetVariable { - variable: variable_lower, - value: value_string, - }); + let value_string = match &values[0] { + SQLExpr::Identifier(i) => ident_to_string(i), + SQLExpr::Value(v) => match crate::utils::value_to_string(&v.value) { + None => { + return plan_err!("Unsupported value {:?}", v.value); + } + Some(s) => s, + }, + SQLExpr::UnaryOp { op, expr } => match op { + UnaryOperator::Plus => format!("+{expr}"), + UnaryOperator::Minus => format!("-{expr}"), + _ => return plan_err!("Unsupported unary op {:?}", op), + }, + _ => return plan_err!("Unsupported expr {:?}", values[0]), + }; - Ok(LogicalPlan::Statement(statement)) + Ok(LogicalPlan::Statement(PlanStatement::SetVariable( + SetVariable { + variable: variable_lower, + value: value_string, + }, + ))) + } + other => not_impl_err!("SET variant not implemented yet: {other:?}"), + } } fn delete_to_plan( diff --git a/datafusion/sql/src/unparser/ast.rs b/datafusion/sql/src/unparser/ast.rs index d9ade822aa005..2cf26009ac0f2 100644 --- a/datafusion/sql/src/unparser/ast.rs +++ b/datafusion/sql/src/unparser/ast.rs @@ -19,7 +19,9 @@ use core::fmt; use std::ops::ControlFlow; use sqlparser::ast::helpers::attached_token::AttachedToken; -use sqlparser::ast::{self, visit_expressions_mut, OrderByKind, SelectFlavor}; +use sqlparser::ast::{ + self, visit_expressions_mut, LimitClause, OrderByKind, SelectFlavor, +}; #[derive(Clone)] pub struct QueryBuilder { @@ -100,14 +102,17 @@ impl QueryBuilder { None => return Err(Into::into(UninitializedFieldError::from("body"))), }, order_by, - limit: self.limit.clone(), - limit_by: self.limit_by.clone(), - offset: self.offset.clone(), + limit_clause: Some(LimitClause::LimitOffset { + limit: self.limit.clone(), + offset: self.offset.clone(), + limit_by: self.limit_by.clone(), + }), fetch: self.fetch.clone(), locks: self.locks.clone(), for_clause: self.for_clause.clone(), settings: None, format_clause: None, + pipe_operators: vec![], }) } fn create_empty() -> Self { @@ -143,7 +148,7 @@ pub struct SelectBuilder { group_by: Option, cluster_by: Vec, distribute_by: Vec, - sort_by: Vec, + sort_by: Vec, having: Option, named_window: Vec, qualify: Option, @@ -260,7 +265,7 @@ impl SelectBuilder { self.distribute_by = value; self } - pub fn sort_by(&mut self, value: Vec) -> &mut Self { + pub fn sort_by(&mut self, value: Vec) -> &mut Self { self.sort_by = value; self } @@ -315,6 +320,7 @@ impl SelectBuilder { Some(ref value) => value.clone(), None => return Err(Into::into(UninitializedFieldError::from("flavor"))), }, + exclude: None, }) } fn create_empty() -> Self { diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index ddfb9e0039c93..0501a4e049894 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -183,6 +183,8 @@ impl Unparser<'_> { operand, conditions, else_result, + case_token: AttachedToken::empty(), + end_token: AttachedToken::empty(), }) } Expr::Cast(Cast { expr, data_type }) => { @@ -281,7 +283,7 @@ impl Unparser<'_> { negated: *negated, expr: Box::new(self.expr_to_sql_inner(expr)?), pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), + escape_char: escape_char.map(|c| SingleQuotedString(c.to_string())), any: false, }), Expr::Like(Like { @@ -296,7 +298,8 @@ impl Unparser<'_> { negated: *negated, expr: Box::new(self.expr_to_sql_inner(expr)?), pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), + escape_char: escape_char + .map(|c| SingleQuotedString(c.to_string())), any: false, }) } else { @@ -304,7 +307,8 @@ impl Unparser<'_> { negated: *negated, expr: Box::new(self.expr_to_sql_inner(expr)?), pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), + escape_char: escape_char + .map(|c| SingleQuotedString(c.to_string())), any: false, }) } diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 4fb1e42d6028f..befea3fe28c18 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -466,6 +466,7 @@ impl Unparser<'_> { "Offset operator only valid in a statement context." ); }; + query.offset(Some(ast::Offset { rows: ast::OffsetRows::None, value: self.expr_to_sql(skip)?, @@ -799,7 +800,7 @@ impl Unparser<'_> { let projection = left_projection .into_iter() - .chain(right_projection.into_iter()) + .chain(right_projection) .collect(); select.projection(projection); } diff --git a/datafusion/sql/tests/cases/plan_to_sql.rs b/datafusion/sql/tests/cases/plan_to_sql.rs index fe6a98d5c2d04..cffb062974e51 100644 --- a/datafusion/sql/tests/cases/plan_to_sql.rs +++ b/datafusion/sql/tests/cases/plan_to_sql.rs @@ -51,6 +51,7 @@ use datafusion_expr::builder::{ project, subquery_alias, table_scan_with_filter_and_fetch, table_scan_with_filters, }; use datafusion_functions::core::planner::CoreFunctionPlanner; +use datafusion_functions::planner::UserDefinedFunctionPlanner; use datafusion_functions_nested::extract::array_element_udf; use datafusion_functions_nested::planner::{FieldAccessPlanner, NestedFunctionPlanner}; use datafusion_sql::unparser::ast::{ @@ -1340,6 +1341,7 @@ where .with_scalar_function(Arc::new(unicode::substr().as_ref().clone())) .with_scalar_function(make_array_udf()) .with_expr_planner(Arc::new(CoreFunctionPlanner::default())) + .with_expr_planner(Arc::new(UserDefinedFunctionPlanner)) .with_expr_planner(Arc::new(NestedFunctionPlanner)) .with_expr_planner(Arc::new(FieldAccessPlanner)), }; diff --git a/datafusion/sql/tests/sql_integration.rs b/datafusion/sql/tests/sql_integration.rs index fcd5115ae8bd1..28181771f1585 100644 --- a/datafusion/sql/tests/sql_integration.rs +++ b/datafusion/sql/tests/sql_integration.rs @@ -4564,6 +4564,30 @@ fn test_no_functions_registered() { ); } +#[test] +fn test_no_substring_registered() { + // substring requires an expression planner + let sql = "SELECT SUBSTRING(foo, bar, baz) FROM person"; + let err = logical_plan(sql).expect_err("query should have failed"); + + assert_snapshot!( + err.strip_backtrace(), + @"This feature is not implemented: Substring could not be planned by registered expr planner. Hint: enable the `unicode_expressions" + ); +} + +#[test] +fn test_no_substring_registered_alt_syntax() { + // Alternate syntax for substring + let sql = "SELECT SUBSTRING(foo FROM bar) FROM person"; + let err = logical_plan(sql).expect_err("query should have failed"); + + assert_snapshot!( + err.strip_backtrace(), + @"This feature is not implemented: Substring could not be planned by registered expr planner. Hint: enable the `unicode_expressions" + ); +} + #[test] fn test_custom_type_plan() -> Result<()> { let sql = "SELECT DATETIME '2001-01-01 18:00:00'"; diff --git a/datafusion/sqllogictest/test_files/errors.slt b/datafusion/sqllogictest/test_files/errors.slt index 1b4700c9bf4be..6b80f56bcf575 100644 --- a/datafusion/sqllogictest/test_files/errors.slt +++ b/datafusion/sqllogictest/test_files/errors.slt @@ -148,7 +148,7 @@ SELECT query error DataFusion error: Arrow error: Cast error: Cannot cast string 'foo' to value of Int64 type create table foo as values (1), ('foo'); -query error user-defined coercion failed +query error DataFusion error: Error during planning: Substring without for/from is not valid select 1 group by substr(''); # Error in filter should be reported diff --git a/datafusion/sqllogictest/test_files/order.slt b/datafusion/sqllogictest/test_files/order.slt index 14bc5fba3abd7..c5674d6998ddd 100644 --- a/datafusion/sqllogictest/test_files/order.slt +++ b/datafusion/sqllogictest/test_files/order.slt @@ -179,7 +179,7 @@ NULL three 2 two 1 one -statement error DataFusion error: Error during planning: Unsupported Value NULL +statement error DataFusion error: Error during planning: Unsupported value Null set datafusion.sql_parser.default_null_ordering = null; # reset to default null ordering