From c2dd55b3b5a97bf2b4f951325aebe313d2e4833f Mon Sep 17 00:00:00 2001 From: StandingMan Date: Wed, 7 Jan 2026 15:10:49 +0800 Subject: [PATCH 1/9] chore(deps): Update sqlparser to 0.60 Signed-off-by: StandingMan --- Cargo.lock | 8 +- Cargo.toml | 2 +- .../relation_planner/match_recognize.rs | 6 +- .../relation_planner/pivot_unpivot.rs | 10 +- .../examples/relation_planner/table_sample.rs | 18 ++- .../tests/user_defined/relation_planner.rs | 16 ++- datafusion/expr/src/expr.rs | 6 +- datafusion/expr/src/planner.rs | 4 +- datafusion/sql/src/planner.rs | 2 +- datafusion/sql/src/query.rs | 1 + datafusion/sql/src/relation/mod.rs | 8 +- datafusion/sql/src/statement.rs | 130 +++++------------- datafusion/sql/src/unparser/plan.rs | 1 + datafusion/sql/src/values.rs | 1 + 14 files changed, 90 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0c26e27b426b..a817f65a7ad4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5840,9 +5840,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", @@ -5851,9 +5851,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/expr/src/expr.rs b/datafusion/expr/src/expr.rs index c7d825ce1d52f..56fe293e121cb 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 @@ -1403,7 +1403,9 @@ pub struct PlannedReplaceSelectItem { impl Display for PlannedReplaceSelectItem { fn fmt(&self, f: &mut Formatter) -> fmt::Result { write!(f, "REPLACE")?; - write!(f, " ({})", display_comma_separated(&self.items))?; + for item in &self.items { + write!(f, " ({item})")?; + } Ok(()) } } 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..2ecdac6733d13 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: false, }, ), 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..22cba38a6aeb0 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -55,9 +55,9 @@ 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, IndexColumn, IndexType, OrderByExpr, Set, + ShowStatementIn, ShowStatementOptions, SqliteOnConflict, TableObject, Update, + UpdateTableFromKind, ValueWithSpan, }; use sqlparser::ast::{ Assignment, AssignmentTarget, ColumnDef, CreateIndex, CreateTable, @@ -102,78 +102,22 @@ 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 { + for ast::ColumnOptionDef { name: _, option } in &column.options { match option { - ast::ColumnOption::Unique { - is_primary: false, - characteristics, - } => constraints.push(TableConstraint::Unique { - 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()), - options: OrderByOptions { - asc: None, - nulls_first: None, - }, - with_fill: None, - }, - operator_class: None, - }], - characteristics: *characteristics, - index_name: None, - index_type: None, - index_options: vec![], - }), - ast::ColumnOption::ForeignKey { - 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::Unique(constraint) => { + constraints.push(TableConstraint::Unique(constraint.clone())) + } + ast::ColumnOption::PrimaryKey(constraint) => { + constraints.push(TableConstraint::PrimaryKey(constraint.clone())) + } + ast::ColumnOption::ForeignKey(constraint) => { + constraints.push(TableConstraint::ForeignKey(constraint.clone())) + } ast::ColumnOption::Check(expr) => { - constraints.push(TableConstraint::Check { - name: name.clone(), - expr: Box::new(expr.clone()), - enforced: None, - }) + constraints.push(TableConstraint::Check(expr.clone())) } - // Other options are not constraint related. ast::ColumnOption::Default(_) | ast::ColumnOption::Null | ast::ColumnOption::NotNull @@ -191,7 +135,8 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec {} + | ast::ColumnOption::Collation(_) + | ast::ColumnOption::Invisible => {} } } } @@ -557,7 +502,7 @@ impl SqlToRel<'_, S> { } } } - Statement::CreateView { + Statement::CreateView(ast::CreateView { or_replace, materialized, name, @@ -574,7 +519,7 @@ impl SqlToRel<'_, S> { or_alter, secure, name_before_not_exists, - } => { + }) => { if materialized { return not_impl_err!("Materialized views not supported")?; } @@ -596,7 +541,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 +558,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 +910,7 @@ impl SqlToRel<'_, S> { has_table_keyword, settings, format_clause, + .. }) => { let table_name = match table { TableObject::TableName(table_name) => table_name, @@ -1025,7 +971,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 +979,8 @@ impl SqlToRel<'_, S> { returning, or, limit, - } => { + .. + }) => { let from_clauses = from.map(|update_table_from_kind| match update_table_from_kind { UpdateTableFromKind::BeforeSet(from_clauses) => from_clauses, @@ -1064,6 +1011,7 @@ impl SqlToRel<'_, S> { from, order_by, limit, + .. }) => { if !tables.is_empty() { plan_err!("DELETE not supported")?; @@ -1295,7 +1243,7 @@ impl SqlToRel<'_, S> { let function_body = match function_body { Some(r) => Some(self.sql_to_expr( match r { - ast::CreateFunctionBody::AsBeforeOptions(expr) => expr, + ast::CreateFunctionBody::AsBeforeOptions{body: expr, ..} => expr, ast::CreateFunctionBody::AsAfterOptions(expr) => expr, ast::CreateFunctionBody::Return(expr) => expr, ast::CreateFunctionBody::AsBeginEnd(_) => { @@ -1338,14 +1286,10 @@ impl SqlToRel<'_, S> { Ok(LogicalPlan::Ddl(statement)) } - Statement::DropFunction { - if_exists, - func_desc, - .. - } => { + Statement::DropFunction(func) => { // According to postgresql documentation it can be only one function // specified in drop statement - if let Some(desc) = func_desc.first() { + if let Some(desc) = func.func_desc.first() { // At the moment functions can't be qualified `schema.name` let name = match &desc.name.0[..] { [] => exec_err!("Function should have name")?, @@ -1353,7 +1297,7 @@ impl SqlToRel<'_, S> { [..] => not_impl_err!("Qualified functions are not supported")?, }; let statement = DdlStatement::DropFunction(DropFunction { - if_exists, + if_exists: func.if_exists, name, schema: DFSchemaRef::new(DFSchema::empty()), }); @@ -1716,24 +1660,24 @@ impl SqlToRel<'_, S> { let constraints = constraints .iter() .map(|c: &TableConstraint| match c { - TableConstraint::Unique { name, columns, .. } => { - let constraint_name = match name { + TableConstraint::Unique(constraint) => { + let constraint_name = match &constraint.name { Some(name) => &format!("unique constraint with name '{name}'"), None => "unique constraint", }; // Get unique constraint indices in the schema let indices = self.get_constraint_column_indices( df_schema, - columns, + &constraint.columns, constraint_name, )?; Ok(Constraint::Unique(indices)) } - TableConstraint::PrimaryKey { columns, .. } => { + TableConstraint::PrimaryKey(constraint) => { // Get primary key indices in the schema let indices = self.get_constraint_column_indices( df_schema, - columns, + &constraint.columns, "primary key", )?; Ok(Constraint::PrimaryKey(indices)) diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 56bf887dbde43..28eb61465381e 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: false, } } diff --git a/datafusion/sql/src/values.rs b/datafusion/sql/src/values.rs index dd8957c95470d..61462db9d9ed9 100644 --- a/datafusion/sql/src/values.rs +++ b/datafusion/sql/src/values.rs @@ -31,6 +31,7 @@ impl SqlToRel<'_, S> { let SQLValues { explicit_row: _, rows, + value_keyword: _, } = values; let empty_schema = Arc::new(DFSchema::empty()); From 8b018d81572380cbf44149f46c98beebda03d827 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Wed, 7 Jan 2026 20:10:55 +0800 Subject: [PATCH 2/9] fix: fix the table alias and parameter on CreateTable Signed-off-by: StandingMan --- .../user_defined_scalar_functions.rs | 13 ++++------ datafusion/sql/src/query.rs | 2 +- datafusion/sql/src/statement.rs | 24 ++++++++++--------- datafusion/sql/src/unparser/plan.rs | 2 +- 4 files changed, 19 insertions(+), 22 deletions(-) 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/sql/src/query.rs b/datafusion/sql/src/query.rs index 2ecdac6733d13..1b7bb856a592b 100644 --- a/datafusion/sql/src/query.rs +++ b/datafusion/sql/src/query.rs @@ -170,7 +170,7 @@ impl SqlToRel<'_, S> { name: alias, // Apply to all fields columns: vec![], - explicit: false, + explicit: true, }, ), PipeOperator::Union { diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 22cba38a6aeb0..d2c53ef3716c7 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -115,8 +115,8 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { constraints.push(TableConstraint::ForeignKey(constraint.clone())) } - ast::ColumnOption::Check(expr) => { - constraints.push(TableConstraint::Check(expr.clone())) + ast::ColumnOption::Check(constraint) => { + constraints.push(TableConstraint::Check(constraint.clone())) } ast::ColumnOption::Default(_) | ast::ColumnOption::Null @@ -286,15 +286,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:?}" )?; diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 28eb61465381e..9f770f9f45e1d 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -1395,7 +1395,7 @@ impl Unparser<'_> { ast::TableAlias { name: self.new_ident_quoted_if_needs(alias), columns, - explicit: false, + explicit: true, } } From 563e8da386a66c3ea88f0c8bcd6eb11276461a15 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Wed, 7 Jan 2026 20:50:45 +0800 Subject: [PATCH 3/9] fix: fix the table constraints parser Signed-off-by: StandingMan --- datafusion/sql/src/statement.rs | 67 +++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index d2c53ef3716c7..8f6bb9d988553 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, OrderByExpr, Set, - ShowStatementIn, ShowStatementOptions, SqliteOnConflict, TableObject, Update, - 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, @@ -104,19 +105,69 @@ fn get_schema_name(schema_name: &SchemaName) -> String { fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { let mut constraints: Vec = vec![]; for column in columns { - for ast::ColumnOptionDef { name: _, option } in &column.options { + for ast::ColumnOptionDef { name, option } in &column.options { match option { ast::ColumnOption::Unique(constraint) => { - constraints.push(TableConstraint::Unique(constraint.clone())) + constraints.push(TableConstraint::Unique(UniqueConstraint { + name: name.clone(), + index_name: None, + index_type_display: ast::KeyOrIndexDisplay::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: constraint.characteristics, + nulls_distinct: NullsDistinctOption::None, + })) } ast::ColumnOption::PrimaryKey(constraint) => { - constraints.push(TableConstraint::PrimaryKey(constraint.clone())) + 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: constraint.characteristics, + })) } ast::ColumnOption::ForeignKey(constraint) => { - constraints.push(TableConstraint::ForeignKey(constraint.clone())) + constraints.push(TableConstraint::ForeignKey(ForeignKeyConstraint { + name: name.clone(), + index_name: None, + columns: vec![], + foreign_table: constraint.foreign_table.clone(), + referred_columns: constraint.referred_columns.clone(), + on_delete: constraint.on_delete, + on_update: constraint.on_update, + match_kind: None, + characteristics: constraint.characteristics, + })) } ast::ColumnOption::Check(constraint) => { - constraints.push(TableConstraint::Check(constraint.clone())) + constraints.push(TableConstraint::Check(CheckConstraint { + name: name.clone(), + expr: constraint.expr.clone(), + enforced: None, + })) } ast::ColumnOption::Default(_) | ast::ColumnOption::Null From b4a31895315ea21d6b1069c9ea165b7b80241e54 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Thu, 8 Jan 2026 13:51:25 +0800 Subject: [PATCH 4/9] chore: add comment for Insert Token Signed-off-by: StandingMan --- datafusion/sql/src/statement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 8f6bb9d988553..1beb7e3fae72b 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -963,7 +963,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, From 7f8cf1889d28e88693817f9ddbe70c1f306ef694 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Thu, 8 Jan 2026 16:18:00 +0800 Subject: [PATCH 5/9] fix: fix the DisPlay for Signed-off-by: StandingMan --- datafusion/expr/src/expr.rs | 10 +++++++--- datafusion/sql/src/statement.rs | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 56fe293e121cb..397a4d6313d71 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -1402,10 +1402,14 @@ pub struct PlannedReplaceSelectItem { impl Display for PlannedReplaceSelectItem { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - write!(f, "REPLACE")?; - for item in &self.items { - write!(f, " ({item})")?; + write!(f, "REPLACE (")?; + for (i, item) in self.items.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{item}")?; } + write!(f, ")")?; Ok(()) } } diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 1beb7e3fae72b..eebe4cedb7f50 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -1296,7 +1296,8 @@ impl SqlToRel<'_, S> { let function_body = match function_body { Some(r) => Some(self.sql_to_expr( match r { - ast::CreateFunctionBody::AsBeforeOptions{body: 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(_) => { From 5d0fed61bbfdc5b85ad8b62885d645a64d617827 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Fri, 9 Jan 2026 10:15:50 +0800 Subject: [PATCH 6/9] enhance: explicitly destructur tableconstraints Signed-off-by: StandingMan --- datafusion/sql/src/statement.rs | 72 +++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index eebe4cedb7f50..d74077778b117 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -107,29 +107,32 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { - constraints.push(TableConstraint::Unique(UniqueConstraint { - name: name.clone(), - index_name: None, - index_type_display: ast::KeyOrIndexDisplay::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, + ast::ColumnOption::Unique(UniqueConstraint { + characteristics, .. + }) => constraints.push(TableConstraint::Unique(UniqueConstraint { + name: name.clone(), + index_name: None, + index_type_display: ast::KeyOrIndexDisplay::None, + index_type: None, + columns: vec![IndexColumn { + column: OrderByExpr { + expr: SQLExpr::Identifier(column.name.clone()), + options: OrderByOptions { + asc: None, + nulls_first: None, }, - operator_class: None, - }], - index_options: vec![], - characteristics: constraint.characteristics, - nulls_distinct: NullsDistinctOption::None, - })) - } - ast::ColumnOption::PrimaryKey(constraint) => { + with_fill: None, + }, + operator_class: None, + }], + index_options: vec![], + characteristics: *characteristics, + nulls_distinct: NullsDistinctOption::None, + })), + ast::ColumnOption::PrimaryKey(PrimaryKeyConstraint { + characteristics, + .. + }) => { constraints.push(TableConstraint::PrimaryKey(PrimaryKeyConstraint { name: name.clone(), index_name: None, @@ -146,26 +149,33 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { + ast::ColumnOption::ForeignKey(ForeignKeyConstraint { + foreign_table, + referred_columns, + on_delete, + on_update, + characteristics, + .. + }) => { constraints.push(TableConstraint::ForeignKey(ForeignKeyConstraint { name: name.clone(), index_name: None, columns: vec![], - foreign_table: constraint.foreign_table.clone(), - referred_columns: constraint.referred_columns.clone(), - on_delete: constraint.on_delete, - on_update: constraint.on_update, + foreign_table: foreign_table.clone(), + referred_columns: referred_columns.clone(), + on_delete: on_delete.clone(), + on_update: on_update.clone(), match_kind: None, - characteristics: constraint.characteristics, + characteristics: *characteristics, })) } - ast::ColumnOption::Check(constraint) => { + ast::ColumnOption::Check(CheckConstraint { name, expr, .. }) => { constraints.push(TableConstraint::Check(CheckConstraint { name: name.clone(), - expr: constraint.expr.clone(), + expr: expr.clone(), enforced: None, })) } From 8f15b824830fae4a2f5ca516b186b16c608beef8 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Fri, 9 Jan 2026 15:56:21 +0800 Subject: [PATCH 7/9] enhance: explicitly destructur tableconstraints Signed-off-by: StandingMan --- datafusion/expr/src/expr.rs | 11 +---- datafusion/sql/src/statement.rs | 80 ++++++++++++++++++++++++--------- datafusion/sql/src/values.rs | 9 +++- 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 397a4d6313d71..8234247c92db6 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -1268,7 +1268,6 @@ impl Display for ExceptSelectItem { } } -#[cfg(not(feature = "sql"))] pub fn display_comma_separated(slice: &[T]) -> String where T: Display, @@ -1402,14 +1401,8 @@ pub struct PlannedReplaceSelectItem { impl Display for PlannedReplaceSelectItem { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - write!(f, "REPLACE (")?; - for (i, item) in self.items.iter().enumerate() { - if i > 0 { - write!(f, ", ")?; - } - write!(f, "{item}")?; - } - write!(f, ")")?; + write!(f, "REPLACE")?; + write!(f, " ({})", display_comma_separated(&self.items))?; Ok(()) } } diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index d74077778b117..fb3a950743aa6 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -108,7 +108,14 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec constraints.push(TableConstraint::Unique(UniqueConstraint { name: name.clone(), index_name: None, @@ -131,7 +138,11 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { constraints.push(TableConstraint::PrimaryKey(PrimaryKeyConstraint { name: name.clone(), @@ -158,7 +169,10 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { constraints.push(TableConstraint::ForeignKey(ForeignKeyConstraint { name: name.clone(), @@ -166,19 +180,21 @@ fn calc_inline_constraints_from_columns(columns: &[ColumnDef]) -> Vec { - constraints.push(TableConstraint::Check(CheckConstraint { - name: name.clone(), - expr: expr.clone(), - enforced: None, - })) - } + 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 @@ -1042,7 +1058,7 @@ impl SqlToRel<'_, S> { returning, or, limit, - .. + update_token: _, }) => { let from_clauses = from.map(|update_table_from_kind| match update_table_from_kind { @@ -1074,7 +1090,7 @@ impl SqlToRel<'_, S> { from, order_by, limit, - .. + delete_token: _, }) => { if !tables.is_empty() { plan_err!("DELETE
not supported")?; @@ -1350,10 +1366,14 @@ impl SqlToRel<'_, S> { Ok(LogicalPlan::Ddl(statement)) } - Statement::DropFunction(func) => { + 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.func_desc.first() { + if let Some(desc) = func_desc.first() { // At the moment functions can't be qualified `schema.name` let name = match &desc.name.0[..] { [] => exec_err!("Function should have name")?, @@ -1361,7 +1381,7 @@ impl SqlToRel<'_, S> { [..] => not_impl_err!("Qualified functions are not supported")?, }; let statement = DdlStatement::DropFunction(DropFunction { - if_exists: func.if_exists, + if_exists, name, schema: DFSchemaRef::new(DFSchema::empty()), }); @@ -1724,24 +1744,40 @@ impl SqlToRel<'_, S> { let constraints = constraints .iter() .map(|c: &TableConstraint| match c { - TableConstraint::Unique(constraint) => { - let constraint_name = match &constraint.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", }; // Get unique constraint indices in the schema let indices = self.get_constraint_column_indices( df_schema, - &constraint.columns, + &columns, constraint_name, )?; Ok(Constraint::Unique(indices)) } - TableConstraint::PrimaryKey(constraint) => { + 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, - &constraint.columns, + &columns, "primary key", )?; Ok(Constraint::PrimaryKey(indices)) diff --git a/datafusion/sql/src/values.rs b/datafusion/sql/src/values.rs index 61462db9d9ed9..b317f7f0e19a4 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,8 +31,13 @@ impl SqlToRel<'_, S> { let SQLValues { explicit_row: _, rows, - value_keyword: _, + value_keyword, } = values; + if value_keyword { + return not_impl_err!( + "`VALUE` keyword does not support inserting multiple values." + )?; + } let empty_schema = Arc::new(DFSchema::empty()); let values = rows From 7cf9df459ed40407750093061b44bb8e73e0013e Mon Sep 17 00:00:00 2001 From: StandingMan Date: Fri, 9 Jan 2026 16:09:30 +0800 Subject: [PATCH 8/9] chore: fix clippy error Signed-off-by: StandingMan --- datafusion/sql/src/statement.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index fb3a950743aa6..c962c25b51f50 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -1761,7 +1761,7 @@ impl SqlToRel<'_, S> { // Get unique constraint indices in the schema let indices = self.get_constraint_column_indices( df_schema, - &columns, + columns, constraint_name, )?; Ok(Constraint::Unique(indices)) @@ -1777,7 +1777,7 @@ impl SqlToRel<'_, S> { // Get primary key indices in the schema let indices = self.get_constraint_column_indices( df_schema, - &columns, + columns, "primary key", )?; Ok(Constraint::PrimaryKey(indices)) From 4acc2919f6e022d3afe5c1bb9d26639fda34e34e Mon Sep 17 00:00:00 2001 From: Alan Tang Date: Sat, 10 Jan 2026 07:33:13 +0800 Subject: [PATCH 9/9] Update datafusion/sql/src/values.rs Co-authored-by: Jeffrey Vo --- datafusion/sql/src/values.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/sql/src/values.rs b/datafusion/sql/src/values.rs index b317f7f0e19a4..c8cdf1254f33f 100644 --- a/datafusion/sql/src/values.rs +++ b/datafusion/sql/src/values.rs @@ -35,7 +35,7 @@ impl SqlToRel<'_, S> { } = values; if value_keyword { return not_impl_err!( - "`VALUE` keyword does not support inserting multiple values." + "`VALUE` keyword not supported. Did you mean `VALUES`?" )?; }