From 658827d3537e3b9f2f6132ce775a17a7350b1d55 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Thu, 30 Apr 2026 07:50:31 -0600 Subject: [PATCH 1/3] chore: pin sqlparser to apache/datafusion-sqlparser-rs main (9833c033) Bump from crates.io 0.61.0 to git main rev 9833c033bc44c... to gain access to SparkSqlDialect ahead of the next sqlparser-rs release. Notable upstream changes absorbed: - Lambda::params is now OneOrManyWithParens; reject typed lambda parameters (not_impl_err) since DataFusion does not model parameter data types. - Insert::columns is now Vec; reject multi-part column names in INSERT (not_impl_err). - ExcludeSelectItem::Single/Multiple now carry ObjectName; reject multi-part EXCLUDE identifiers (plan_err). - New SelectItem::ExprWithAliases, BeginTransactionKind::Tran, TableConstraint::PrimaryKeyUsingIndex/UniqueUsingIndex, TableObject::TableQuery variants -> not_impl_err. - WildcardAdditionalOptions gained opt_alias -> not_impl_err. - VALUES rows are now Vec>>; iterate via .content. - CREATE FUNCTION return_type is now Option (DataType vs SetOf); reject SETOF. - escape_char in Like/ILike/SimilarTo is now Option. - TableAlias gained `at: Option`; CreateTable, CreateView, Insert, Update, Delete, Select, TableFactor::Function gained several new fields with `not_impl_err!` for unsupported ones. Update higher_order.slt golden error messages to match new LambdaFunctionParameter Debug formatting. --- Cargo.lock | 110 +++-------------- Cargo.toml | 2 +- datafusion/expr/src/expr.rs | 4 +- datafusion/expr/src/utils.rs | 30 +++++ datafusion/sql/src/expr/function.rs | 15 ++- datafusion/sql/src/expr/mod.rs | 8 +- datafusion/sql/src/query.rs | 1 + datafusion/sql/src/select.rs | 6 + datafusion/sql/src/statement.rs | 116 ++++++++++++++++-- datafusion/sql/src/unparser/ast.rs | 4 +- datafusion/sql/src/unparser/expr.rs | 12 +- datafusion/sql/src/unparser/plan.rs | 3 + datafusion/sql/src/values.rs | 3 +- .../sqllogictest/test_files/higher_order.slt | 6 +- 14 files changed, 196 insertions(+), 124 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c18fd2012891c..7fb9abe0780f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,7 +112,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -123,7 +123,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -2713,7 +2713,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4119,7 +4119,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -5711,7 +5711,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -5742,8 +5742,7 @@ dependencies = [ [[package]] name = "sqlparser" version = "0.61.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbf5ea8d4d7c808e1af1cbabebca9a2abe603bcefc22294c5b95018d53200cb7" +source = "git+https://github.com/apache/datafusion-sqlparser-rs?rev=9833c033bc44c487b4425272cc8cea80971b5239#9833c033bc44c487b4425272cc8cea80971b5239" dependencies = [ "log", "recursive", @@ -5753,8 +5752,7 @@ dependencies = [ [[package]] name = "sqlparser_derive" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6dd45d8fc1c79299bfbb7190e42ccbbdf6a5f52e4a6ad98d92357ea965bd289" +source = "git+https://github.com/apache/datafusion-sqlparser-rs?rev=9833c033bc44c487b4425272cc8cea80971b5239#9833c033bc44c487b4425272cc8cea80971b5239" dependencies = [ "proc-macro2", "quote", @@ -5812,7 +5810,7 @@ dependencies = [ "cfg-if", "libc", "psm", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -7016,16 +7014,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.6", -] - -[[package]] -name = "windows-sys" -version = "0.60.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2f500e4d28234f72040990ec9d39e3a6b950f9f22d3dba18416c35882612bcb" -dependencies = [ - "windows-targets 0.53.5", + "windows-targets", ] [[package]] @@ -7043,31 +7032,14 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.6", - "windows_aarch64_msvc 0.52.6", - "windows_i686_gnu 0.52.6", - "windows_i686_gnullvm 0.52.6", - "windows_i686_msvc 0.52.6", - "windows_x86_64_gnu 0.52.6", - "windows_x86_64_gnullvm 0.52.6", - "windows_x86_64_msvc 0.52.6", -] - -[[package]] -name = "windows-targets" -version = "0.53.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4945f9f551b88e0d65f3db0bc25c33b8acea4d9e41163edf90dcd0b19f9069f3" -dependencies = [ - "windows-link", - "windows_aarch64_gnullvm 0.53.1", - "windows_aarch64_msvc 0.53.1", - "windows_i686_gnu 0.53.1", - "windows_i686_gnullvm 0.53.1", - "windows_i686_msvc 0.53.1", - "windows_x86_64_gnu 0.53.1", - "windows_x86_64_gnullvm 0.53.1", - "windows_x86_64_msvc 0.53.1", + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", ] [[package]] @@ -7085,96 +7057,48 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9d8416fa8b42f5c947f8482c43e7d89e73a173cead56d044f6a56104a6d1b53" - [[package]] name = "windows_aarch64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" -[[package]] -name = "windows_aarch64_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9d782e804c2f632e395708e99a94275910eb9100b2114651e04744e9b125006" - [[package]] name = "windows_i686_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" -[[package]] -name = "windows_i686_gnu" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "960e6da069d81e09becb0ca57a65220ddff016ff2d6af6a223cf372a506593a3" - [[package]] name = "windows_i686_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" -[[package]] -name = "windows_i686_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa7359d10048f68ab8b09fa71c3daccfb0e9b559aed648a8f95469c27057180c" - [[package]] name = "windows_i686_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" -[[package]] -name = "windows_i686_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e7ac75179f18232fe9c285163565a57ef8d3c89254a30685b57d83a38d326c2" - [[package]] name = "windows_x86_64_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" -[[package]] -name = "windows_x86_64_gnu" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c3842cdd74a865a8066ab39c8a7a473c0778a3f29370b5fd6b4b9aa7df4a499" - [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ffa179e2d07eee8ad8f57493436566c7cc30ac536a3379fdf008f47f6bb7ae1" - [[package]] name = "windows_x86_64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" -[[package]] -name = "windows_x86_64_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" - [[package]] name = "winnow" version = "1.0.2" diff --git a/Cargo.toml b/Cargo.toml index 37734211266ba..01b0c79215387 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -192,7 +192,7 @@ regex = "1.12" rstest = "0.26.1" serde_json = "1" sha2 = "^0.11.0" -sqlparser = { version = "0.61.0", default-features = false, features = ["std", "visitor"] } +sqlparser = { git = "https://github.com/apache/datafusion-sqlparser-rs", rev = "9833c033bc44c487b4425272cc8cea80971b5239", default-features = false, features = ["std", "visitor"] } strum = "0.28.0" strum_macros = "0.28.0" tempfile = "3" diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 582e8e41dd0cf..8ce8a26501cf9 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -4123,8 +4123,8 @@ mod test { wildcard_with_options(wildcard_options( None, Some(ExcludeSelectItem::Multiple(vec![ - Ident::from("c1"), - Ident::from("c2") + Ident::from("c1").into(), + Ident::from("c2").into() ])), None, None, diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index fdb4b6de7874a..f2fde0433ea06 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -339,7 +339,37 @@ fn get_excluded_columns( idents.push(&excepts.first_element); idents.extend(&excepts.additional_elements); } + #[cfg(feature = "sql")] + let exclude_owned: Vec; if let Some(exclude) = opt_exclude { + #[cfg(feature = "sql")] + { + let object_name_to_ident = + |name: &sqlparser::ast::ObjectName| -> Result { + if name.0.len() != 1 { + return plan_err!( + "EXCLUDE with multi-part identifiers is not supported: {name}" + ); + } + match &name.0[0] { + sqlparser::ast::ObjectNamePart::Identifier(ident) => { + Ok(ident.clone()) + } + other => plan_err!( + "EXCLUDE with non-identifier name part is not supported: {other}" + ), + } + }; + exclude_owned = match exclude { + ExcludeSelectItem::Single(name) => vec![object_name_to_ident(name)?], + ExcludeSelectItem::Multiple(names) => names + .iter() + .map(object_name_to_ident) + .collect::>>()?, + }; + idents.extend(exclude_owned.iter()); + } + #[cfg(not(feature = "sql"))] match exclude { ExcludeSelectItem::Single(ident) => idents.push(ident), ExcludeSelectItem::Multiple(idents_inner) => idents.extend(idents_inner), diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 1790e66a027bb..67abb8b822063 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -927,10 +927,15 @@ impl SqlToRel<'_, S> { ); } + if lambda.params.iter().any(|p| p.data_type.is_some()) { + return not_impl_err!( + "Lambda parameters with explicit data types are not supported" + ); + } let params = lambda .params .iter() - .map(|p| crate::utils::normalize_ident(p.clone())) + .map(|p| crate::utils::normalize_ident(p.name.clone())) .collect(); let lambda_parameters = std::iter::zip(lambda_params, ¶ms) @@ -1181,19 +1186,19 @@ impl SqlToRel<'_, S> { /// After normalization with [normalize_ident], check whether all params are unique /// /// [normalize_ident]: crate::utils::normalize_ident -fn all_unique(params: &[sqlparser::ast::Ident]) -> bool { +fn all_unique(params: &[sqlparser::ast::LambdaFunctionParameter]) -> bool { match params.len() { 0 | 1 => true, 2 => { - crate::utils::normalize_ident(params[0].clone()) - != crate::utils::normalize_ident(params[1].clone()) + crate::utils::normalize_ident(params[0].name.clone()) + != crate::utils::normalize_ident(params[1].name.clone()) } _ => { let mut set = HashSet::with_capacity(params.len()); params .iter() - .map(|p| crate::utils::normalize_ident(p.clone())) + .map(|p| crate::utils::normalize_ident(p.name.clone())) .all(|p| set.insert(p)) } } diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 5cbc1c84bdb4b..daf092ecd4cf9 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -900,7 +900,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, @@ -910,7 +910,7 @@ 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 = match escape_char { + let escape_char = match escape_char.map(|v| v.value) { Some(Value::SingleQuotedString(char)) if char.len() == 1 => { Some(char.chars().next().unwrap()) } @@ -935,7 +935,7 @@ impl SqlToRel<'_, S> { negated: bool, expr: SQLExpr, pattern: SQLExpr, - escape_char: Option, + escape_char: Option, schema: &DFSchema, planner_context: &mut PlannerContext, ) -> Result { @@ -944,7 +944,7 @@ impl SqlToRel<'_, S> { if pattern_type != DataType::Utf8 && pattern_type != DataType::Null { return plan_err!("Invalid pattern in SIMILAR TO expression"); } - let escape_char = match escape_char { + let escape_char = match escape_char.map(|v| v.value) { Some(Value::SingleQuotedString(char)) if char.len() == 1 => { Some(char.chars().next().unwrap()) } diff --git a/datafusion/sql/src/query.rs b/datafusion/sql/src/query.rs index e320d2ee6e9c1..76124cbc7eb59 100644 --- a/datafusion/sql/src/query.rs +++ b/datafusion/sql/src/query.rs @@ -171,6 +171,7 @@ impl SqlToRel<'_, S> { // Apply to all fields columns: vec![], explicit: true, + at: None, }, ), PipeOperator::Union { diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs index 09d8566c4a19e..b7f7d80e70815 100644 --- a/datafusion/sql/src/select.rs +++ b/datafusion/sql/src/select.rs @@ -839,6 +839,9 @@ impl SqlToRel<'_, S> { Ok(SelectExpr::Expression(expr)) } + SelectItem::ExprWithAliases { .. } => { + not_impl_err!("SELECT item with multiple aliases is not supported") + } SelectItem::Wildcard(options) => { Self::check_wildcard_options(&options)?; if empty_from { @@ -886,11 +889,14 @@ impl SqlToRel<'_, S> { opt_rename, opt_replace: _opt_replace, opt_ilike: _opt_ilike, + opt_alias, wildcard_token: _wildcard_token, } = options; if opt_rename.is_some() { not_impl_err!("wildcard * with RENAME not supported ") + } else if opt_alias.is_some() { + not_impl_err!("wildcard * with AS alias not supported") } else { Ok(()) } diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 587ed02d13188..a4a098d156b59 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -344,6 +344,12 @@ impl SqlToRel<'_, S> { require_user, partition_of, for_values, + snapshot, + with_storage_lifecycle_policy, + diststyle, + distkey, + sortkey, + backup, }) => { if temporary { return not_impl_err!("Temporary tables not supported"); @@ -497,6 +503,24 @@ impl SqlToRel<'_, S> { if for_values.is_some() { return not_impl_err!("PARTITION OF .. FOR VALUES .. not supported"); } + if snapshot { + return not_impl_err!("Snapshot tables not supported"); + } + if with_storage_lifecycle_policy.is_some() { + return not_impl_err!("WITH STORAGE LIFECYCLE POLICY not supported"); + } + if diststyle.is_some() { + return not_impl_err!("DISTSTYLE not supported"); + } + if distkey.is_some() { + return not_impl_err!("DISTKEY not supported"); + } + if sortkey.is_some() { + return not_impl_err!("SORTKEY not supported"); + } + if backup.is_some() { + return not_impl_err!("BACKUP not supported"); + } // Merge inline constraints and existing constraints let mut all_constraints = constraints; let inline_constraints = calc_inline_constraints_from_columns(&columns); @@ -604,6 +628,7 @@ impl SqlToRel<'_, S> { or_alter, secure, name_before_not_exists, + copy_grants, }) => { if materialized { return not_impl_err!("Materialized views not supported")?; @@ -623,6 +648,9 @@ impl SqlToRel<'_, S> { if to.is_some() { return not_impl_err!("To not supported")?; } + if copy_grants { + return not_impl_err!("COPY GRANTS not supported")?; + } // put the statement back together temporarily to get the SQL // string representation @@ -643,6 +671,7 @@ impl SqlToRel<'_, S> { or_alter, secure, name_before_not_exists, + copy_grants, }); let sql = stmt.to_string(); let Statement::CreateView(ast::CreateView { @@ -1000,7 +1029,12 @@ impl SqlToRel<'_, S> { settings, format_clause, insert_token: _, // record the location the `INSERT` token - optimizer_hint, + optimizer_hints, + output, + multi_table_insert_type, + multi_table_into_clauses, + multi_table_when_clauses, + multi_table_else_clause, }) => { let table_name = match table { TableObject::TableName(table_name) => table_name, @@ -1009,6 +1043,11 @@ impl SqlToRel<'_, S> { "INSERT INTO Table functions not supported" ); } + TableObject::TableQuery(_) => { + return not_impl_err!( + "INSERT INTO subquery target not supported" + ); + } }; if let Some(or) = or { match or { @@ -1056,9 +1095,19 @@ impl SqlToRel<'_, S> { if format_clause.is_some() { plan_err!("Inserts with format clause not supported")?; } - if optimizer_hint.is_some() { + if !optimizer_hints.is_empty() { plan_err!("Optimizer hints not supported")?; } + if output.is_some() { + plan_err!("Insert OUTPUT clause not supported")?; + } + if multi_table_insert_type.is_some() + || !multi_table_into_clauses.is_empty() + || !multi_table_when_clauses.is_empty() + || multi_table_else_clause.is_some() + { + plan_err!("Multi-table INSERT not supported")?; + } // optional keywords don't change behavior let _ = into; let _ = has_table_keyword; @@ -1073,7 +1122,9 @@ impl SqlToRel<'_, S> { or, limit, update_token: _, - optimizer_hint, + optimizer_hints, + output, + order_by, }) => { let from_clauses = from.map(|update_table_from_kind| match update_table_from_kind { @@ -1103,9 +1154,15 @@ impl SqlToRel<'_, S> { if limit.is_some() { return not_impl_err!("Update-limit clause not supported")?; } - if optimizer_hint.is_some() { + if !optimizer_hints.is_empty() { plan_err!("Optimizer hints not supported")?; } + if output.is_some() { + plan_err!("Update OUTPUT clause not supported")?; + } + if !order_by.is_empty() { + plan_err!("Update ORDER BY not supported")?; + } self.update_to_plan(table, &assignments, update_from, selection) } @@ -1118,7 +1175,8 @@ impl SqlToRel<'_, S> { order_by, limit, delete_token: _, - optimizer_hint, + optimizer_hints, + output, }) => { if !tables.is_empty() { plan_err!("DELETE not supported")?; @@ -1136,9 +1194,12 @@ impl SqlToRel<'_, S> { plan_err!("Delete-order-by clause not yet supported")?; } - if optimizer_hint.is_some() { + if !optimizer_hints.is_empty() { plan_err!("Optimizer hints not supported")?; } + if output.is_some() { + plan_err!("Delete OUTPUT clause not supported")?; + } let table_name = self.get_delete_target(from)?; self.delete_to_plan(&table_name, selection, limit) @@ -1260,7 +1321,14 @@ impl SqlToRel<'_, S> { .. }) => { let return_type = match return_type { - Some(t) => Some(self.convert_data_type_to_field(&t)?), + Some(ast::FunctionReturnType::DataType(t)) => { + Some(self.convert_data_type_to_field(&t)?) + } + Some(ast::FunctionReturnType::SetOf(_)) => { + return not_impl_err!( + "RETURNS SETOF in CREATE FUNCTION is not supported" + ); + } None => None, }; let mut planner_context = PlannerContext::new(); @@ -1882,6 +1950,16 @@ impl SqlToRel<'_, S> { TableConstraint::FulltextOrSpatial { .. } => { _plan_err!("Indexes are not currently supported") } + TableConstraint::PrimaryKeyUsingIndex(_) => { + _plan_err!( + "PRIMARY KEY USING INDEX constraints are not currently supported" + ) + } + TableConstraint::UniqueUsingIndex(_) => { + _plan_err!( + "UNIQUE USING INDEX constraints are not currently supported" + ) + } }) .collect::>>()?; Ok(Constraints::new_unverified(constraints)) @@ -2276,7 +2354,7 @@ impl SqlToRel<'_, S> { fn insert_to_plan( &self, table_name: ObjectName, - columns: Vec, + columns: Vec, source: Box, overwrite: bool, replace_into: bool, @@ -2286,6 +2364,24 @@ impl SqlToRel<'_, S> { let table_source = self.context_provider.get_table_source(table_name.clone())?; let table_schema = DFSchema::try_from(table_source.schema())?; + // Convert ObjectNames to Idents; reject multi-part column names + let columns: Vec = columns + .into_iter() + .map(|name| { + if name.0.len() != 1 { + return not_impl_err!( + "Multi-part column names in INSERT not supported: {name}" + ); + } + match name.0.into_iter().next().unwrap() { + ast::ObjectNamePart::Identifier(ident) => Ok(ident), + other => not_impl_err!( + "Non-identifier column name part in INSERT not supported: {other}" + ), + } + }) + .collect::>>()?; + // Get insert fields and target table's value indices // // If value_indices[i] = Some(j), it means that the value of the i-th target table's column is @@ -2329,7 +2425,7 @@ impl SqlToRel<'_, S> { let mut prepare_param_data_types = BTreeMap::new(); if let SetExpr::Values(ast::Values { rows, .. }) = (*source.body).clone() { for row in rows.iter() { - for (idx, val) in row.iter().enumerate() { + for (idx, val) in row.content.iter().enumerate() { if let SQLExpr::Value(ValueWithSpan { value: Value::Placeholder(name), span: _, @@ -2581,7 +2677,7 @@ ON p.function_name = r.routine_name None => Ok(()), // BEGIN TRANSACTION Some(BeginTransactionKind::Transaction) => Ok(()), - Some(BeginTransactionKind::Work) => { + Some(BeginTransactionKind::Work) | Some(BeginTransactionKind::Tran) => { not_impl_err!("Transaction kind not supported: {kind:?}") } } diff --git a/datafusion/sql/src/unparser/ast.rs b/datafusion/sql/src/unparser/ast.rs index d8d5ec9e409fc..c34dc0827b7a7 100644 --- a/datafusion/sql/src/unparser/ast.rs +++ b/datafusion/sql/src/unparser/ast.rs @@ -358,7 +358,7 @@ impl SelectBuilder { } pub fn build(&self) -> Result { Ok(ast::Select { - optimizer_hint: None, + optimizer_hints: vec![], distinct: self.distinct.clone(), select_modifiers: None, top_before_distinct: false, @@ -477,6 +477,7 @@ pub struct RelationBuilder { } #[derive(Clone)] +#[expect(clippy::large_enum_variant)] enum TableFactorBuilder { Table(TableRelationBuilder), Derived(DerivedRelationBuilder), @@ -794,6 +795,7 @@ impl FlattenRelationBuilder { lateral: true, name: ast::ObjectName::from(vec![ast::Ident::new("FLATTEN")]), args, + with_ordinality: false, alias: self.alias.clone(), }) } diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index e76aea5849492..5ec20a8826253 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -291,7 +291,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| SingleQuotedString(c.to_string())), + escape_char: escape_char + .map(|c| SingleQuotedString(c.to_string()).into()), any: false, }), Expr::Like(Like { @@ -307,7 +308,7 @@ impl Unparser<'_> { expr: Box::new(self.expr_to_sql_inner(expr)?), pattern: Box::new(self.expr_to_sql_inner(pattern)?), escape_char: escape_char - .map(|c| SingleQuotedString(c.to_string())), + .map(|c| SingleQuotedString(c.to_string()).into()), any: false, }) } else { @@ -316,7 +317,7 @@ impl Unparser<'_> { expr: Box::new(self.expr_to_sql_inner(expr)?), pattern: Box::new(self.expr_to_sql_inner(pattern)?), escape_char: escape_char - .map(|c| SingleQuotedString(c.to_string())), + .map(|c| SingleQuotedString(c.to_string()).into()), any: false, }) } @@ -572,7 +573,10 @@ impl Unparser<'_> { params: ast::OneOrManyWithParens::Many( params .iter() - .map(|param| self.new_ident_quoted_if_needs(param.clone())) + .map(|param| ast::LambdaFunctionParameter { + name: self.new_ident_quoted_if_needs(param.clone()), + data_type: None, + }) .collect(), ), body: Box::new(self.expr_to_sql_inner(body)?), diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 2c36fe0b2c98a..43d7ef49d9453 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -434,6 +434,7 @@ impl Unparser<'_> { name: Ident::with_quote('"', &flatten_alias_name), columns: vec![], explicit: true, + at: None, })); if !select.already_projected() { @@ -1208,6 +1209,7 @@ impl Unparser<'_> { name: Ident::with_quote('"', &alias), columns: vec![], explicit: true, + at: None, })); } relation.flatten(flatten_relation); @@ -1902,6 +1904,7 @@ impl Unparser<'_> { name: self.new_ident_quoted_if_needs(alias), columns, explicit: true, + at: None, } } diff --git a/datafusion/sql/src/values.rs b/datafusion/sql/src/values.rs index c8cdf1254f33f..a1df1fe1b18d1 100644 --- a/datafusion/sql/src/values.rs +++ b/datafusion/sql/src/values.rs @@ -43,7 +43,8 @@ impl SqlToRel<'_, S> { let values = rows .into_iter() .map(|row| { - row.into_iter() + row.content + .into_iter() .map(|v| self.sql_to_expr(v, &empty_schema, planner_context)) .collect::>>() }) diff --git a/datafusion/sqllogictest/test_files/higher_order.slt b/datafusion/sqllogictest/test_files/higher_order.slt index 86359194a5395..941c0a8a3af6f 100644 --- a/datafusion/sqllogictest/test_files/higher_order.slt +++ b/datafusion/sqllogictest/test_files/higher_order.slt @@ -261,13 +261,13 @@ SELECT array_transform([1, 2], (e, i, j) -> i); query error DataFusion error: Error during planning: lambda parameters names must be unique, got \(v, v\) SELECT array_transform([1], (v, v) -> v*2); -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,12\)\.\.Location\(1,13\)\) \}\), body: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,17\)\.\.Location\(1,18\)\) \}\), syntax: Arrow \}\) +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(LambdaFunctionParameter \{ name: Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,12\)\.\.Location\(1,13\)\) \}, data_type: None \}\), body: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,17\)\.\.Location\(1,18\)\) \}\), syntax: Arrow \}\) SELECT abs(v -> v); -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,8\)\.\.Location\(1,9\)\) \}\), body: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,13\)\.\.Location\(1,14\)\) \}\), syntax: Arrow \}\) +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(LambdaFunctionParameter \{ name: Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,8\)\.\.Location\(1,9\)\) \}, data_type: None \}\), body: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,13\)\.\.Location\(1,14\)\) \}\), syntax: Arrow \}\) SELECT v -> v; -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,34\)\.\.Location\(1,35\)\) \}\), body: BinaryOp \{ left: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,39\)\.\.Location\(1,40\)\) \}\), op: Plus, right: Value\(ValueWithSpan \{ value: Number\("1", false\), span: Span\(Location\(1,41\)\.\.Location\(1,42\)\) \}\) \}, syntax: Arrow \}\) +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Lambda\(LambdaFunction \{ params: One\(LambdaFunctionParameter \{ name: Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,34\)\.\.Location\(1,35\)\) \}, data_type: None \}\), body: BinaryOp \{ left: Identifier\(Ident \{ value: "v", quote_style: None, span: Span\(Location\(1,39\)\.\.Location\(1,40\)\) \}\), op: Plus, right: Value\(ValueWithSpan \{ value: Number\("1", false\), span: Span\(Location\(1,41\)\.\.Location\(1,42\)\) \}\) \}, syntax: Arrow \}\) SELECT array_transform([1], v -> v -> v+1); query error DataFusion error: SQL error: ParserError\("Expected: an expression, found: \) at Line: 1, Column: 30"\) From 5374947419f225384f7f14652c0c85ef9e8e8e82 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Thu, 30 Apr 2026 08:29:51 -0600 Subject: [PATCH 2/3] docs: explain exclude_owned scope and large_enum_variant suppression --- datafusion/expr/src/utils.rs | 2 ++ datafusion/sql/src/unparser/ast.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index f2fde0433ea06..080d18933fa41 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -339,6 +339,8 @@ fn get_excluded_columns( idents.push(&excepts.first_element); idents.extend(&excepts.additional_elements); } + // Declared outside the `if let` so `idents.extend(exclude_owned.iter())` + // below can borrow references that outlive the inner scope. #[cfg(feature = "sql")] let exclude_owned: Vec; if let Some(exclude) = opt_exclude { diff --git a/datafusion/sql/src/unparser/ast.rs b/datafusion/sql/src/unparser/ast.rs index c34dc0827b7a7..4b4e56c40cdc5 100644 --- a/datafusion/sql/src/unparser/ast.rs +++ b/datafusion/sql/src/unparser/ast.rs @@ -477,6 +477,8 @@ pub struct RelationBuilder { } #[derive(Clone)] +// Boxing variants would penalize the common builder path; this enum is +// constructed-then-consumed locally rather than stored at scale. #[expect(clippy::large_enum_variant)] enum TableFactorBuilder { Table(TableRelationBuilder), From abf1caceeb1a7369ac9f1c0b5331735091f76c27 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Thu, 30 Apr 2026 08:40:53 -0600 Subject: [PATCH 3/3] =?UTF-8?q?refactor:=20use=20ObjectNamePart::as=5Fiden?= =?UTF-8?q?t()=20for=20ObjectName=E2=86=92Ident=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- datafusion/expr/src/utils.rs | 15 +++++++-------- datafusion/sql/src/statement.rs | 14 +++++++------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index 080d18933fa41..e0e2330cd1ad8 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -353,14 +353,13 @@ fn get_excluded_columns( "EXCLUDE with multi-part identifiers is not supported: {name}" ); } - match &name.0[0] { - sqlparser::ast::ObjectNamePart::Identifier(ident) => { - Ok(ident.clone()) - } - other => plan_err!( - "EXCLUDE with non-identifier name part is not supported: {other}" - ), - } + let part = &name.0[0]; + let Some(ident) = part.as_ident() else { + return plan_err!( + "EXCLUDE with non-identifier name part is not supported: {part}" + ); + }; + Ok(ident.clone()) }; exclude_owned = match exclude { ExcludeSelectItem::Single(name) => vec![object_name_to_ident(name)?], diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index a4a098d156b59..e7088c8a3d6f1 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -2364,7 +2364,6 @@ impl SqlToRel<'_, S> { let table_source = self.context_provider.get_table_source(table_name.clone())?; let table_schema = DFSchema::try_from(table_source.schema())?; - // Convert ObjectNames to Idents; reject multi-part column names let columns: Vec = columns .into_iter() .map(|name| { @@ -2373,12 +2372,13 @@ impl SqlToRel<'_, S> { "Multi-part column names in INSERT not supported: {name}" ); } - match name.0.into_iter().next().unwrap() { - ast::ObjectNamePart::Identifier(ident) => Ok(ident), - other => not_impl_err!( - "Non-identifier column name part in INSERT not supported: {other}" - ), - } + let part = &name.0[0]; + let Some(ident) = part.as_ident() else { + return not_impl_err!( + "Non-identifier column name part in INSERT not supported: {part}" + ); + }; + Ok(ident.clone()) }) .collect::>>()?;