diff --git a/Cargo.lock b/Cargo.lock index 2ee907b30cf02..964052b09811b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5677,9 +5677,9 @@ dependencies = [ [[package]] name = "sqlparser" -version = "0.59.0" +version = "0.60.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4591acadbcf52f0af60eafbb2c003232b2b4cd8de5f0e9437cb8b1b59046cc0f" +checksum = "505aa16b045c4c1375bf5f125cce3813d0176325bfe9ffc4a903f423de7774ff" dependencies = [ "log", "recursive", @@ -5688,9 +5688,9 @@ dependencies = [ [[package]] name = "sqlparser_derive" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da5fc6819faabb412da764b99d3b713bb55083c11e7e0c00144d386cd6a1939c" +checksum = "028e551d5e270b31b9f3ea271778d9d827148d4287a5d96167b6bb9787f5cc38" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index b9d8b1a69ef61..8b7e41d83b83e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -181,7 +181,7 @@ recursive = "0.1.1" regex = "1.12" rstest = "0.26.1" serde_json = "1" -sqlparser = { version = "0.59.0", default-features = false, features = ["std", "visitor"] } +sqlparser = { version = "0.60.0", default-features = false, features = ["std", "visitor"] } strum = "0.27.2" strum_macros = "0.27.2" tempfile = "3" diff --git a/datafusion-examples/examples/relation_planner/match_recognize.rs b/datafusion-examples/examples/relation_planner/match_recognize.rs index 60baf9bd61a62..c4b3d522efc17 100644 --- a/datafusion-examples/examples/relation_planner/match_recognize.rs +++ b/datafusion-examples/examples/relation_planner/match_recognize.rs @@ -362,7 +362,7 @@ impl RelationPlanner for MatchRecognizePlanner { .. } = relation else { - return Ok(RelationPlanning::Original(relation)); + return Ok(RelationPlanning::Original(Box::new(relation))); }; // Plan the input table @@ -401,6 +401,8 @@ impl RelationPlanner for MatchRecognizePlanner { node: Arc::new(node), }); - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } } diff --git a/datafusion-examples/examples/relation_planner/pivot_unpivot.rs b/datafusion-examples/examples/relation_planner/pivot_unpivot.rs index 86a6cb955500e..2e1696956bf62 100644 --- a/datafusion-examples/examples/relation_planner/pivot_unpivot.rs +++ b/datafusion-examples/examples/relation_planner/pivot_unpivot.rs @@ -339,7 +339,7 @@ impl RelationPlanner for PivotUnpivotPlanner { alias, ), - other => Ok(RelationPlanning::Original(other)), + other => Ok(RelationPlanning::Original(Box::new(other))), } } } @@ -459,7 +459,9 @@ fn plan_pivot( .aggregate(group_by_cols, pivot_exprs)? .build()?; - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } // ============================================================================ @@ -540,7 +542,9 @@ fn plan_unpivot( .build()?; } - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } // ============================================================================ diff --git a/datafusion-examples/examples/relation_planner/table_sample.rs b/datafusion-examples/examples/relation_planner/table_sample.rs index 362d35dcf4cac..657432ef31362 100644 --- a/datafusion-examples/examples/relation_planner/table_sample.rs +++ b/datafusion-examples/examples/relation_planner/table_sample.rs @@ -331,7 +331,7 @@ impl RelationPlanner for TableSamplePlanner { index_hints, } = relation else { - return Ok(RelationPlanning::Original(relation)); + return Ok(RelationPlanning::Original(Box::new(relation))); }; // Extract sample spec (handles both before/after alias positions) @@ -401,7 +401,9 @@ impl RelationPlanner for TableSamplePlanner { let fraction = bucket_num as f64 / total as f64; let plan = TableSamplePlanNode::new(input, fraction, seed).into_plan(); - return Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))); + return Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))); } // Handle quantity-based sampling @@ -422,7 +424,9 @@ impl RelationPlanner for TableSamplePlanner { let plan = LogicalPlanBuilder::from(input) .limit(0, Some(rows as usize))? .build()?; - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } // TABLESAMPLE (N PERCENT) - percentage sampling @@ -430,7 +434,9 @@ impl RelationPlanner for TableSamplePlanner { let percent: f64 = parse_literal::(&quantity_value_expr)?; let fraction = percent / 100.0; let plan = TableSamplePlanNode::new(input, fraction, seed).into_plan(); - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } // TABLESAMPLE (N) - fraction if <1.0, row limit if >=1.0 @@ -448,7 +454,9 @@ impl RelationPlanner for TableSamplePlanner { // Interpret as fraction TableSamplePlanNode::new(input, value, seed).into_plan() }; - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } } } diff --git a/datafusion/core/tests/user_defined/relation_planner.rs b/datafusion/core/tests/user_defined/relation_planner.rs index bda9b37ebea68..54af53ad858d4 100644 --- a/datafusion/core/tests/user_defined/relation_planner.rs +++ b/datafusion/core/tests/user_defined/relation_planner.rs @@ -68,9 +68,11 @@ fn plan_static_values_table( .project(vec![col("column1").alias(column_name)])? .build()?; - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } - other => Ok(RelationPlanning::Original(other)), + other => Ok(RelationPlanning::Original(Box::new(other))), } } @@ -176,9 +178,11 @@ impl RelationPlanner for SamplingJoinPlanner { .cross_join(right_sampled)? .build()?; - Ok(RelationPlanning::Planned(PlannedRelation::new(plan, alias))) + Ok(RelationPlanning::Planned(Box::new(PlannedRelation::new( + plan, alias, + )))) } - other => Ok(RelationPlanning::Original(other)), + other => Ok(RelationPlanning::Original(Box::new(other))), } } } @@ -195,7 +199,7 @@ impl RelationPlanner for PassThroughPlanner { _context: &mut dyn RelationPlannerContext, ) -> Result { // Never handles anything - always delegates - Ok(RelationPlanning::Original(relation)) + Ok(RelationPlanning::Original(Box::new(relation))) } } @@ -217,7 +221,7 @@ impl RelationPlanner for PremiumFeaturePlanner { to unlock advanced array operations." .to_string(), )), - other => Ok(RelationPlanning::Original(other)), + other => Ok(RelationPlanning::Original(Box::new(other))), } } } diff --git a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs index b86cd94a8a9b7..4e96aed530643 100644 --- a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs +++ b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs @@ -1306,19 +1306,14 @@ async fn create_scalar_function_from_sql_statement_default_arguments() -> Result "Error during planning: Non-default arguments cannot follow default arguments."; assert!(expected.starts_with(&err.strip_backtrace())); - // FIXME: The `DEFAULT` syntax does not work with positional params - let bad_expression_sql = r#" + let expression_sql = r#" CREATE FUNCTION bad_expression_fun(DOUBLE, DOUBLE DEFAULT 2.0) RETURNS DOUBLE RETURN $1 + $2 "#; - let err = ctx - .sql(bad_expression_sql) - .await - .expect_err("sqlparser error"); - let expected = - "SQL error: ParserError(\"Expected: ), found: 2.0 at Line: 2, Column: 63\")"; - assert!(expected.starts_with(&err.strip_backtrace())); + let result = ctx.sql(expression_sql).await; + + assert!(result.is_ok()); Ok(()) } diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index c7d825ce1d52f..8234247c92db6 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -42,7 +42,7 @@ use datafusion_functions_window_common::field::WindowUDFFieldArgs; #[cfg(feature = "sql")] use sqlparser::ast::{ ExceptSelectItem, ExcludeSelectItem, IlikeSelectItem, RenameSelectItem, - ReplaceSelectElement, display_comma_separated, + ReplaceSelectElement, }; // Moved in 51.0.0 to datafusion_common @@ -1268,7 +1268,6 @@ impl Display for ExceptSelectItem { } } -#[cfg(not(feature = "sql"))] pub fn display_comma_separated(slice: &[T]) -> String where T: Display, diff --git a/datafusion/expr/src/planner.rs b/datafusion/expr/src/planner.rs index 954f511651ced..76961a49a4c9c 100644 --- a/datafusion/expr/src/planner.rs +++ b/datafusion/expr/src/planner.rs @@ -369,9 +369,9 @@ impl PlannedRelation { #[derive(Debug)] pub enum RelationPlanning { /// The relation was successfully planned by an extension planner - Planned(PlannedRelation), + Planned(Box), /// No extension planner handled the relation, return it for default processing - Original(TableFactor), + Original(Box), } /// Customize planning SQL table factors to [`LogicalPlan`]s. diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs index eb798b71e4558..520a2d55ef6a2 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -823,7 +823,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { | SQLDataType::HugeInt | SQLDataType::UHugeInt | SQLDataType::UBigInt - | SQLDataType::TimestampNtz + | SQLDataType::TimestampNtz{..} | SQLDataType::NamedTable { .. } | SQLDataType::TsVector | SQLDataType::TsQuery diff --git a/datafusion/sql/src/query.rs b/datafusion/sql/src/query.rs index eba48a2401c38..1b7bb856a592b 100644 --- a/datafusion/sql/src/query.rs +++ b/datafusion/sql/src/query.rs @@ -170,6 +170,7 @@ impl SqlToRel<'_, S> { name: alias, // Apply to all fields columns: vec![], + explicit: true, }, ), PipeOperator::Union { diff --git a/datafusion/sql/src/relation/mod.rs b/datafusion/sql/src/relation/mod.rs index 3115d8dfffbd2..cef3726c62e40 100644 --- a/datafusion/sql/src/relation/mod.rs +++ b/datafusion/sql/src/relation/mod.rs @@ -93,7 +93,7 @@ impl SqlToRel<'_, S> { match self.create_extension_relation(relation, planner_context)? { RelationPlanning::Planned(planned) => planned, RelationPlanning::Original(original) => { - self.create_default_relation(original, planner_context)? + Box::new(self.create_default_relation(*original, planner_context)?) } }; @@ -112,7 +112,7 @@ impl SqlToRel<'_, S> { ) -> Result { let planners = self.context_provider.get_relation_planners(); if planners.is_empty() { - return Ok(RelationPlanning::Original(relation)); + return Ok(RelationPlanning::Original(Box::new(relation))); } let mut current_relation = relation; @@ -127,12 +127,12 @@ impl SqlToRel<'_, S> { return Ok(RelationPlanning::Planned(planned)); } RelationPlanning::Original(original) => { - current_relation = original; + current_relation = *original; } } } - Ok(RelationPlanning::Original(current_relation)) + Ok(RelationPlanning::Original(Box::new(current_relation))) } fn create_default_relation( diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 1acbcc92dfe19..c962c25b51f50 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -55,9 +55,10 @@ use datafusion_expr::{ TransactionIsolationLevel, TransactionStart, Volatility, WriteOp, cast, col, }; use sqlparser::ast::{ - self, BeginTransactionKind, IndexColumn, IndexType, NullsDistinctOption, OrderByExpr, - OrderByOptions, Set, ShowStatementIn, ShowStatementOptions, SqliteOnConflict, - TableObject, UpdateTableFromKind, ValueWithSpan, + self, BeginTransactionKind, CheckConstraint, ForeignKeyConstraint, IndexColumn, + IndexType, NullsDistinctOption, OrderByExpr, OrderByOptions, PrimaryKeyConstraint, + Set, ShowStatementIn, ShowStatementOptions, SqliteOnConflict, TableObject, + UniqueConstraint, Update, UpdateTableFromKind, ValueWithSpan, }; use sqlparser::ast::{ Assignment, AssignmentTarget, ColumnDef, CreateIndex, CreateTable, @@ -102,38 +103,24 @@ fn get_schema_name(schema_name: &SchemaName) -> String { /// Construct `TableConstraint`(s) for the given columns by iterating over /// `columns` and extracting individual inline constraint definitions. fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { - let mut constraints = vec![]; + let mut constraints: Vec = vec![]; for column in columns { for ast::ColumnOptionDef { name, option } in &column.options { match option { - ast::ColumnOption::Unique { - is_primary: false, + ast::ColumnOption::Unique(UniqueConstraint { characteristics, - } => constraints.push(TableConstraint::Unique { + name, + index_name: _index_name, + index_type_display: _index_type_display, + index_type: _index_type, + columns: _column, + index_options: _index_options, + nulls_distinct: _nulls_distinct, + }) => constraints.push(TableConstraint::Unique(UniqueConstraint { name: 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, index_type: None, - index_options: vec![], - nulls_distinct: NullsDistinctOption::None, - }), - ast::ColumnOption::Unique { - is_primary: true, - characteristics, - } => constraints.push(TableConstraint::PrimaryKey { - name: name.clone(), columns: vec![IndexColumn { column: OrderByExpr { expr: SQLExpr::Identifier(column.name.clone()), @@ -145,35 +132,69 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { + constraints.push(TableConstraint::PrimaryKey(PrimaryKeyConstraint { + name: name.clone(), + index_name: None, + index_type: None, + columns: vec![IndexColumn { + column: OrderByExpr { + expr: SQLExpr::Identifier(column.name.clone()), + options: OrderByOptions { + asc: None, + nulls_first: None, + }, + with_fill: None, + }, + operator_class: None, + }], + index_options: vec![], + characteristics: *characteristics, + })) + } + ast::ColumnOption::ForeignKey(ForeignKeyConstraint { foreign_table, referred_columns, on_delete, on_update, characteristics, - } => constraints.push(TableConstraint::ForeignKey { - name: name.clone(), - columns: vec![], - foreign_table: foreign_table.clone(), - referred_columns: referred_columns.to_vec(), - on_delete: *on_delete, - on_update: *on_update, - characteristics: *characteristics, - index_name: None, - }), - ast::ColumnOption::Check(expr) => { - constraints.push(TableConstraint::Check { + name: _name, + index_name: _index_name, + columns: _columns, + match_kind: _match_kind, + }) => { + constraints.push(TableConstraint::ForeignKey(ForeignKeyConstraint { name: name.clone(), - expr: Box::new(expr.clone()), - enforced: None, - }) - } - // Other options are not constraint related. + index_name: None, + columns: vec![], + foreign_table: foreign_table.clone(), + referred_columns: referred_columns.clone(), + on_delete: *on_delete, + on_update: *on_update, + match_kind: None, + characteristics: *characteristics, + })) + } + ast::ColumnOption::Check(CheckConstraint { + name, + expr, + enforced: _enforced, + }) => constraints.push(TableConstraint::Check(CheckConstraint { + name: name.clone(), + expr: expr.clone(), + enforced: None, + })), ast::ColumnOption::Default(_) | ast::ColumnOption::Null | ast::ColumnOption::NotNull @@ -191,7 +212,8 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec {} + | ast::ColumnOption::Collation(_) + | ast::ColumnOption::Invisible => {} } } } @@ -341,15 +363,17 @@ impl SqlToRel<'_, S> { "Hive distribution not supported: {hive_distribution:?}" )?; } - if !matches!( - hive_formats, - Some(ast::HiveFormat { - row_format: None, - serde_properties: None, - storage: None, - location: None, - }) - ) { + if hive_formats.is_some() + && !matches!( + hive_formats, + Some(ast::HiveFormat { + row_format: None, + serde_properties: None, + storage: None, + location: None, + }) + ) + { return not_impl_err!( "Hive formats not supported: {hive_formats:?}" )?; @@ -557,7 +581,7 @@ impl SqlToRel<'_, S> { } } } - Statement::CreateView { + Statement::CreateView(ast::CreateView { or_replace, materialized, name, @@ -574,7 +598,7 @@ impl SqlToRel<'_, S> { or_alter, secure, name_before_not_exists, - } => { + }) => { if materialized { return not_impl_err!("Materialized views not supported")?; } @@ -596,7 +620,7 @@ impl SqlToRel<'_, S> { // put the statement back together temporarily to get the SQL // string representation - let stmt = Statement::CreateView { + let stmt = Statement::CreateView(ast::CreateView { or_replace, materialized, name, @@ -613,16 +637,16 @@ impl SqlToRel<'_, S> { or_alter, secure, name_before_not_exists, - }; + }); let sql = stmt.to_string(); - let Statement::CreateView { + let Statement::CreateView(ast::CreateView { name, columns, query, or_replace, temporary, .. - } = stmt + }) = stmt else { return internal_err!("Unreachable code in create view"); }; @@ -965,6 +989,7 @@ impl SqlToRel<'_, S> { has_table_keyword, settings, format_clause, + insert_token: _insert_token, // record the location the `INSERT` token }) => { let table_name = match table { TableObject::TableName(table_name) => table_name, @@ -1025,7 +1050,7 @@ impl SqlToRel<'_, S> { let _ = has_table_keyword; self.insert_to_plan(table_name, columns, source, overwrite, replace_into) } - Statement::Update { + Statement::Update(Update { table, assignments, from, @@ -1033,7 +1058,8 @@ impl SqlToRel<'_, S> { returning, or, limit, - } => { + update_token: _, + }) => { let from_clauses = from.map(|update_table_from_kind| match update_table_from_kind { UpdateTableFromKind::BeforeSet(from_clauses) => from_clauses, @@ -1064,6 +1090,7 @@ impl SqlToRel<'_, S> { from, order_by, limit, + delete_token: _, }) => { if !tables.is_empty() { plan_err!("DELETE not supported")?; @@ -1295,7 +1322,8 @@ impl SqlToRel<'_, S> { let function_body = match function_body { Some(r) => Some(self.sql_to_expr( match r { - ast::CreateFunctionBody::AsBeforeOptions(expr) => expr, + // `link_symbol` indicates if the primary expression contains the name of shared library file. + ast::CreateFunctionBody::AsBeforeOptions{body: expr, link_symbol: _link_symbol} => expr, ast::CreateFunctionBody::AsAfterOptions(expr) => expr, ast::CreateFunctionBody::Return(expr) => expr, ast::CreateFunctionBody::AsBeginEnd(_) => { @@ -1338,11 +1366,11 @@ impl SqlToRel<'_, S> { Ok(LogicalPlan::Ddl(statement)) } - Statement::DropFunction { + Statement::DropFunction(ast::DropFunction { if_exists, func_desc, - .. - } => { + drop_behavior: _, + }) => { // According to postgresql documentation it can be only one function // specified in drop statement if let Some(desc) = func_desc.first() { @@ -1716,8 +1744,17 @@ impl SqlToRel<'_, S> { let constraints = constraints .iter() .map(|c: &TableConstraint| match c { - TableConstraint::Unique { name, columns, .. } => { - let constraint_name = match name { + TableConstraint::Unique(UniqueConstraint { + name, + index_name: _, + index_type_display: _, + index_type: _, + columns, + index_options: _, + characteristics: _, + nulls_distinct: _, + }) => { + let constraint_name = match &name { Some(name) => &format!("unique constraint with name '{name}'"), None => "unique constraint", }; @@ -1729,7 +1766,14 @@ impl SqlToRel<'_, S> { )?; Ok(Constraint::Unique(indices)) } - TableConstraint::PrimaryKey { columns, .. } => { + TableConstraint::PrimaryKey(PrimaryKeyConstraint { + name: _, + index_name: _, + index_type: _, + columns, + index_options: _, + characteristics: _, + }) => { // Get primary key indices in the schema let indices = self.get_constraint_column_indices( df_schema, diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 56bf887dbde43..9f770f9f45e1d 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -1395,6 +1395,7 @@ impl Unparser<'_> { ast::TableAlias { name: self.new_ident_quoted_if_needs(alias), columns, + explicit: true, } } diff --git a/datafusion/sql/src/values.rs b/datafusion/sql/src/values.rs index dd8957c95470d..c8cdf1254f33f 100644 --- a/datafusion/sql/src/values.rs +++ b/datafusion/sql/src/values.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; -use datafusion_common::{DFSchema, Result}; +use datafusion_common::{DFSchema, Result, not_impl_err}; use datafusion_expr::{LogicalPlan, LogicalPlanBuilder}; use sqlparser::ast::Values as SQLValues; @@ -31,7 +31,13 @@ impl SqlToRel<'_, S> { let SQLValues { explicit_row: _, rows, + value_keyword, } = values; + if value_keyword { + return not_impl_err!( + "`VALUE` keyword not supported. Did you mean `VALUES`?" + )?; + } let empty_schema = Arc::new(DFSchema::empty()); let values = rows