diff --git a/Cargo.toml b/Cargo.toml index 0952c17887461..8d25478ab5498 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -150,7 +150,7 @@ recursive = "0.1.1" regex = "1.8" rstest = "0.24.0" serde_json = "1" -sqlparser = { version = "0.53.0", features = ["visitor"] } +sqlparser = { version = "0.54.0", features = ["visitor"] } tempfile = "3" tokio = { version = "1.43", features = ["macros", "rt", "sync"] } url = "2.5.4" diff --git a/datafusion-cli/Cargo.lock b/datafusion-cli/Cargo.lock index d1107d2a7168b..544714d3731f3 100644 --- a/datafusion-cli/Cargo.lock +++ b/datafusion-cli/Cargo.lock @@ -3804,11 +3804,12 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "sqlparser" -version = "0.53.0" +version = "0.54.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05a528114c392209b3264855ad491fcce534b94a38771b0a0b97a79379275ce8" +checksum = "c66e3b7374ad4a6af849b08b3e7a6eda0edbd82f0fd59b57e22671bf16979899" dependencies = [ "log", + "recursive", "sqlparser_derive", ] diff --git a/datafusion/common/src/column.rs b/datafusion/common/src/column.rs index 9973a72e7bc0f..05e2dff0bd436 100644 --- a/datafusion/common/src/column.rs +++ b/datafusion/common/src/column.rs @@ -84,7 +84,11 @@ impl Column { } } - fn from_idents(idents: &mut Vec) -> Option { + /// Create a Column from multiple normalized identifiers + /// + /// For example, `foo.bar` would be represented as a two element vector + /// `["foo", "bar"]` + fn from_idents(mut idents: Vec) -> Option { let (relation, name) = match idents.len() { 1 => (None, idents.remove(0)), 2 => ( @@ -126,7 +130,7 @@ impl Column { /// where `"foo.BAR"` would be parsed to a reference to column named `foo.BAR` pub fn from_qualified_name(flat_name: impl Into) -> Self { let flat_name = flat_name.into(); - Self::from_idents(&mut parse_identifiers_normalized(&flat_name, false)).unwrap_or( + Self::from_idents(parse_identifiers_normalized(&flat_name, false)).unwrap_or( Self { relation: None, name: flat_name, @@ -138,7 +142,7 @@ impl Column { /// Deserialize a fully qualified name string into a column preserving column text case pub fn from_qualified_name_ignore_case(flat_name: impl Into) -> Self { let flat_name = flat_name.into(); - Self::from_idents(&mut parse_identifiers_normalized(&flat_name, true)).unwrap_or( + Self::from_idents(parse_identifiers_normalized(&flat_name, true)).unwrap_or( Self { relation: None, name: flat_name, diff --git a/datafusion/expr/src/logical_plan/statement.rs b/datafusion/expr/src/logical_plan/statement.rs index 93be04c275647..82acebee3de66 100644 --- a/datafusion/expr/src/logical_plan/statement.rs +++ b/datafusion/expr/src/logical_plan/statement.rs @@ -153,6 +153,7 @@ pub enum TransactionIsolationLevel { ReadCommitted, RepeatableRead, Serializable, + Snapshot, } /// Indicator that the following statements should be committed or rolled back atomically diff --git a/datafusion/sql/src/expr/identifier.rs b/datafusion/sql/src/expr/identifier.rs index 130cdf083da30..ab5c550691bd2 100644 --- a/datafusion/sql/src/expr/identifier.rs +++ b/datafusion/sql/src/expr/identifier.rs @@ -92,7 +92,7 @@ impl SqlToRel<'_, S> { } } - pub(super) fn sql_compound_identifier_to_expr( + pub(crate) fn sql_compound_identifier_to_expr( &self, ids: Vec, schema: &DFSchema, diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 951e81c1fdee0..de753da895d30 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -21,14 +21,14 @@ use datafusion_expr::planner::{ PlannerResult, RawBinaryExpr, RawDictionaryExpr, RawFieldAccessExpr, }; use sqlparser::ast::{ - BinaryOperator, CastFormat, CastKind, DataType as SQLDataType, DictionaryField, - Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, MapEntry, StructField, Subscript, - TrimWhereField, Value, + AccessExpr, BinaryOperator, CastFormat, CastKind, DataType as SQLDataType, + DictionaryField, Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, MapEntry, + StructField, Subscript, TrimWhereField, Value, }; use datafusion_common::{ - internal_datafusion_err, internal_err, not_impl_err, plan_err, DFSchema, Result, - ScalarValue, + internal_datafusion_err, internal_err, not_impl_err, plan_err, Column, DFSchema, + Result, ScalarValue, }; use datafusion_expr::expr::ScalarFunction; @@ -238,14 +238,14 @@ impl SqlToRel<'_, S> { self.sql_identifier_to_expr(id, schema, planner_context) } - SQLExpr::MapAccess { .. } => { - not_impl_err!("Map Access") - } - // ["foo"], [4] or [4:5] - SQLExpr::Subscript { expr, subscript } => { - self.sql_subscript_to_expr(*expr, subscript, schema, planner_context) - } + SQLExpr::CompoundFieldAccess { root, access_chain } => self + .sql_compound_field_access_to_expr( + *root, + access_chain, + schema, + planner_context, + ), SQLExpr::CompoundIdentifier(ids) => { self.sql_compound_identifier_to_expr(ids, schema, planner_context) @@ -982,84 +982,146 @@ impl SqlToRel<'_, S> { Ok(Expr::Cast(Cast::new(Box::new(expr), dt))) } - fn sql_subscript_to_expr( + fn sql_compound_field_access_to_expr( &self, - expr: SQLExpr, - subscript: Box, + root: SQLExpr, + access_chain: Vec, schema: &DFSchema, planner_context: &mut PlannerContext, ) -> Result { - let expr = self.sql_expr_to_logical_expr(expr, schema, planner_context)?; - - let field_access = match *subscript { - Subscript::Index { index } => { - // index can be a name, in which case it is a named field access - match index { - SQLExpr::Value( - Value::SingleQuotedString(s) | Value::DoubleQuotedString(s), - ) => GetFieldAccess::NamedStructField { - name: ScalarValue::from(s), - }, - SQLExpr::JsonAccess { .. } => { - return not_impl_err!("JsonAccess"); + let mut root = self.sql_expr_to_logical_expr(root, schema, planner_context)?; + let fields = access_chain + .into_iter() + .map(|field| match field { + AccessExpr::Subscript(subscript) => { + match subscript { + Subscript::Index { index } => { + // index can be a name, in which case it is a named field access + match index { + SQLExpr::Value( + Value::SingleQuotedString(s) + | Value::DoubleQuotedString(s), + ) => Ok(Some(GetFieldAccess::NamedStructField { + name: ScalarValue::from(s), + })), + SQLExpr::JsonAccess { .. } => { + not_impl_err!("JsonAccess") + } + // otherwise treat like a list index + _ => Ok(Some(GetFieldAccess::ListIndex { + key: Box::new(self.sql_expr_to_logical_expr( + index, + schema, + planner_context, + )?), + })), + } + } + Subscript::Slice { + lower_bound, + upper_bound, + stride, + } => { + // Means access like [:2] + let lower_bound = if let Some(lower_bound) = lower_bound { + self.sql_expr_to_logical_expr( + lower_bound, + schema, + planner_context, + ) + } else { + not_impl_err!("Slice subscript requires a lower bound") + }?; + + // means access like [2:] + let upper_bound = if let Some(upper_bound) = upper_bound { + self.sql_expr_to_logical_expr( + upper_bound, + schema, + planner_context, + ) + } else { + not_impl_err!("Slice subscript requires an upper bound") + }?; + + // stride, default to 1 + let stride = if let Some(stride) = stride { + self.sql_expr_to_logical_expr( + stride, + schema, + planner_context, + )? + } else { + lit(1i64) + }; + + Ok(Some(GetFieldAccess::ListRange { + start: Box::new(lower_bound), + stop: Box::new(upper_bound), + stride: Box::new(stride), + })) + } } - // otherwise treat like a list index - _ => GetFieldAccess::ListIndex { - key: Box::new(self.sql_expr_to_logical_expr( - index, - schema, - planner_context, - )?), - }, } - } - Subscript::Slice { - lower_bound, - upper_bound, - stride, - } => { - // Means access like [:2] - let lower_bound = if let Some(lower_bound) = lower_bound { - self.sql_expr_to_logical_expr(lower_bound, schema, planner_context) - } else { - not_impl_err!("Slice subscript requires a lower bound") - }?; - - // means access like [2:] - let upper_bound = if let Some(upper_bound) = upper_bound { - self.sql_expr_to_logical_expr(upper_bound, schema, planner_context) - } else { - not_impl_err!("Slice subscript requires an upper bound") - }?; - - // stride, default to 1 - let stride = if let Some(stride) = stride { - self.sql_expr_to_logical_expr(stride, schema, planner_context)? - } else { - lit(1i64) - }; - - GetFieldAccess::ListRange { - start: Box::new(lower_bound), - stop: Box::new(upper_bound), - stride: Box::new(stride), + AccessExpr::Dot(expr) => { + let expr = + self.sql_expr_to_logical_expr(expr, schema, planner_context)?; + match expr { + Expr::Column(Column { + name, + relation, + spans, + }) => { + if let Some(relation) = &relation { + // If the first part of the dot access is a column reference, we should + // check if the column is from the same table as the root expression. + // If it is, we should replace the root expression with the column reference. + // Otherwise, we should treat the dot access as a named field access. + if relation.table() == root.schema_name().to_string() { + root = Expr::Column(Column { + name, + relation: Some(relation.clone()), + spans, + }); + Ok(None) + } else { + plan_err!( + "table name mismatch: {} != {}", + relation.table(), + root.schema_name() + ) + } + } else { + Ok(Some(GetFieldAccess::NamedStructField { + name: ScalarValue::from(name), + })) + } + } + _ => not_impl_err!( + "Dot access not supported for non-column expr: {expr:?}" + ), + } } - } - }; + }) + .collect::>>()?; - let mut field_access_expr = RawFieldAccessExpr { expr, field_access }; - for planner in self.context_provider.get_expr_planners() { - match planner.plan_field_access(field_access_expr, schema)? { - PlannerResult::Planned(expr) => return Ok(expr), - PlannerResult::Original(expr) => { - field_access_expr = expr; + fields + .into_iter() + .flatten() + .try_fold(root, |expr, field_access| { + let mut field_access_expr = RawFieldAccessExpr { expr, field_access }; + for planner in self.context_provider.get_expr_planners() { + match planner.plan_field_access(field_access_expr, schema)? { + PlannerResult::Planned(expr) => return Ok(expr), + PlannerResult::Original(expr) => { + field_access_expr = expr; + } + } } - } - } - - not_impl_err!( - "GetFieldAccess not supported by ExprPlanner: {field_access_expr:?}" - ) + not_impl_err!( + "GetFieldAccess not supported by ExprPlanner: {field_access_expr:?}" + ) + }) } } diff --git a/datafusion/sql/src/parser.rs b/datafusion/sql/src/parser.rs index cc42ec1bf3113..9725166b8ae04 100644 --- a/datafusion/sql/src/parser.rs +++ b/datafusion/sql/src/parser.rs @@ -571,7 +571,7 @@ impl<'a> DFParser<'a> { loop { if let Token::Word(_) = self.parser.peek_token().token { - let identifier = self.parser.parse_identifier(false)?; + let identifier = self.parser.parse_identifier()?; partitions.push(identifier.to_string()); } else { return self.expected("partition name", self.parser.peek_token()); @@ -674,7 +674,7 @@ impl<'a> DFParser<'a> { } fn parse_column_def(&mut self) -> Result { - let name = self.parser.parse_identifier(false)?; + let name = self.parser.parse_identifier()?; let data_type = self.parser.parse_data_type()?; let collation = if self.parser.parse_keyword(Keyword::COLLATE) { Some(self.parser.parse_object_name(false)?) @@ -684,7 +684,7 @@ impl<'a> DFParser<'a> { let mut options = vec![]; loop { if self.parser.parse_keyword(Keyword::CONSTRAINT) { - let name = Some(self.parser.parse_identifier(false)?); + let name = Some(self.parser.parse_identifier()?); if let Some(option) = self.parser.parse_optional_column_option()? { options.push(ColumnOptionDef { name, option }); } else { diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs index f22ff6d94fc27..85d428cae84fc 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -451,7 +451,10 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { SQLDataType::UnsignedBigInt(_) | SQLDataType::UnsignedInt8(_) => Ok(DataType::UInt64), SQLDataType::Float(_) => Ok(DataType::Float32), SQLDataType::Real | SQLDataType::Float4 => Ok(DataType::Float32), - SQLDataType::Double | SQLDataType::DoublePrecision | SQLDataType::Float8 => Ok(DataType::Float64), + SQLDataType::Double(ExactNumberInfo::None) | SQLDataType::DoublePrecision | SQLDataType::Float8 => Ok(DataType::Float64), + SQLDataType::Double(ExactNumberInfo::Precision(_)|ExactNumberInfo::PrecisionAndScale(_, _)) => { + not_impl_err!("Unsupported SQL type (precision/scale not supported) {sql_type}") + } SQLDataType::Char(_) | SQLDataType::Text | SQLDataType::String(_) => Ok(DataType::Utf8), @@ -587,7 +590,9 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { | SQLDataType::MediumText | SQLDataType::LongText | SQLDataType::Bit(_) - |SQLDataType::BitVarying(_) + | SQLDataType::BitVarying(_) + // BigQuery UDFs + | SQLDataType::AnyType => not_impl_err!( "Unsupported SQL type {sql_type:?}" ), diff --git a/datafusion/sql/src/relation/join.rs b/datafusion/sql/src/relation/join.rs index 75f39792bce1d..88665401dc31f 100644 --- a/datafusion/sql/src/relation/join.rs +++ b/datafusion/sql/src/relation/join.rs @@ -18,7 +18,9 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; use datafusion_common::{not_impl_err, Column, Result}; use datafusion_expr::{JoinType, LogicalPlan, LogicalPlanBuilder}; -use sqlparser::ast::{Join, JoinConstraint, JoinOperator, TableFactor, TableWithJoins}; +use sqlparser::ast::{ + Join, JoinConstraint, JoinOperator, ObjectName, TableFactor, TableWithJoins, +}; use std::collections::HashSet; impl SqlToRel<'_, S> { @@ -123,11 +125,22 @@ impl SqlToRel<'_, S> { .join_on(right, join_type, Some(expr))? .build() } - JoinConstraint::Using(idents) => { - let keys: Vec = idents + JoinConstraint::Using(object_names) => { + let keys = object_names .into_iter() - .map(|x| Column::from_name(self.ident_normalizer.normalize(x))) - .collect(); + .map(|object_name| { + let ObjectName(mut object_names) = object_name; + if object_names.len() != 1 { + not_impl_err!( + "Invalid identifier in USING clause. Expected single identifier, got {}", ObjectName(object_names) + ) + } else { + let id = object_names.swap_remove(0); + Ok(self.ident_normalizer.normalize(id)) + } + }) + .collect::>>()?; + LogicalPlanBuilder::from(left) .join_using(right, join_type, keys)? .build() diff --git a/datafusion/sql/src/set_expr.rs b/datafusion/sql/src/set_expr.rs index 290c0174784a1..3ddbe373ecd3e 100644 --- a/datafusion/sql/src/set_expr.rs +++ b/datafusion/sql/src/set_expr.rs @@ -133,6 +133,9 @@ impl SqlToRel<'_, S> { (SetOperator::Except, false) => { LogicalPlanBuilder::except(left_plan, right_plan, false) } + (SetOperator::Minus, _) => { + not_impl_err!("MINUS Set Operator not implemented") + } } } } diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index dfd3a4fd76a2a..83c91ecde69a7 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -56,7 +56,7 @@ use datafusion_expr::{ }; use sqlparser::ast::{ self, BeginTransactionKind, NullsDistinctOption, ShowStatementIn, - ShowStatementOptions, SqliteOnConflict, + ShowStatementOptions, SqliteOnConflict, TableObject, UpdateTableFromKind, }; use sqlparser::ast::{ Assignment, AssignmentTarget, ColumnDef, CreateIndex, CreateTable, @@ -497,6 +497,7 @@ impl SqlToRel<'_, S> { if_not_exists, temporary, to, + params, } => { if materialized { return not_impl_err!("Materialized views not supported")?; @@ -532,6 +533,7 @@ impl SqlToRel<'_, S> { if_not_exists, temporary, to, + params, }; let sql = stmt.to_string(); let Statement::CreateView { @@ -818,7 +820,6 @@ impl SqlToRel<'_, S> { Statement::Insert(Insert { or, into, - table_name, columns, overwrite, source, @@ -832,7 +833,17 @@ impl SqlToRel<'_, S> { mut replace_into, priority, insert_alias, + assignments, + has_table_keyword, + settings, + format_clause, }) => { + let table_name = match table { + TableObject::TableName(table_name) => table_name, + TableObject::TableFunction(_) => { + return not_impl_err!("INSERT INTO Table functions not supported") + } + }; if let Some(or) = or { match or { SqliteOnConflict::Replace => replace_into = true, @@ -845,9 +856,6 @@ impl SqlToRel<'_, S> { if !after_columns.is_empty() { plan_err!("After-columns clause not supported")?; } - if table { - plan_err!("Table clause not supported")?; - } if on.is_some() { plan_err!("Insert-on clause not supported")?; } @@ -873,7 +881,18 @@ impl SqlToRel<'_, S> { if insert_alias.is_some() { plan_err!("Inserts with an alias not supported")?; } - let _ = into; // optional keyword doesn't change behavior + if !assignments.is_empty() { + plan_err!("Inserts with assignments not supported")?; + } + if settings.is_some() { + plan_err!("Inserts with settings not supported")?; + } + if format_clause.is_some() { + plan_err!("Inserts with format clause not supported")?; + } + // optional keywords don't change behavior + let _ = into; + let _ = has_table_keyword; self.insert_to_plan(table_name, columns, source, overwrite, replace_into) } Statement::Update { @@ -884,6 +903,11 @@ impl SqlToRel<'_, S> { returning, or, } => { + let from = + from.map(|update_table_from_kind| match update_table_from_kind { + UpdateTableFromKind::BeforeSet(from) => from, + UpdateTableFromKind::AfterSet(from) => from, + }); if returning.is_some() { plan_err!("Update-returning clause not yet supported")?; } @@ -969,6 +993,9 @@ impl SqlToRel<'_, S> { ast::TransactionIsolationLevel::Serializable => { TransactionIsolationLevel::Serializable } + ast::TransactionIsolationLevel::Snapshot => { + TransactionIsolationLevel::Snapshot + } }; let access_mode = match access_mode { ast::TransactionAccessMode::ReadOnly => { @@ -984,7 +1011,17 @@ impl SqlToRel<'_, S> { }); Ok(LogicalPlan::Statement(statement)) } - Statement::Commit { chain } => { + Statement::Commit { + chain, + end, + modifier, + } => { + if end { + return not_impl_err!("COMMIT AND END not supported"); + }; + if let Some(modifier) = modifier { + return not_impl_err!("COMMIT {modifier} not supported"); + }; let statement = PlanStatement::TransactionEnd(TransactionEnd { conclusion: TransactionConclusion::Commit, chain, diff --git a/datafusion/sql/src/unparser/ast.rs b/datafusion/sql/src/unparser/ast.rs index e320a4510e465..6d77c01ea8881 100644 --- a/datafusion/sql/src/unparser/ast.rs +++ b/datafusion/sql/src/unparser/ast.rs @@ -466,6 +466,7 @@ impl TableRelationBuilder { partitions: self.partitions.clone(), with_ordinality: false, json_path: None, + sample: None, }) } fn create_empty() -> Self { diff --git a/datafusion/sql/src/unparser/dialect.rs b/datafusion/sql/src/unparser/dialect.rs index 830435fd013cd..adfb7a0d0cd2c 100644 --- a/datafusion/sql/src/unparser/dialect.rs +++ b/datafusion/sql/src/unparser/dialect.rs @@ -17,6 +17,7 @@ use std::{collections::HashMap, sync::Arc}; +use super::{utils::character_length_to_sql, utils::date_part_to_sql, Unparser}; use arrow_schema::TimeUnit; use datafusion_common::Result; use datafusion_expr::Expr; @@ -29,8 +30,6 @@ use sqlparser::{ keywords::ALL_KEYWORDS, }; -use super::{utils::character_length_to_sql, utils::date_part_to_sql, Unparser}; - pub type ScalarFnToSqlHandler = Box Result> + Send + Sync>; @@ -65,7 +64,7 @@ pub trait Dialect: Send + Sync { /// Does the dialect use DOUBLE PRECISION to represent Float64 rather than DOUBLE? /// E.g. Postgres uses DOUBLE PRECISION instead of DOUBLE fn float64_ast_dtype(&self) -> ast::DataType { - ast::DataType::Double + ast::DataType::Double(ast::ExactNumberInfo::None) } /// The SQL type to use for Arrow Utf8 unparsing @@ -526,7 +525,7 @@ impl Default for CustomDialect { supports_nulls_first_in_sort: true, use_timestamp_for_date64: false, interval_style: IntervalStyle::SQLStandard, - float64_ast_dtype: ast::DataType::Double, + float64_ast_dtype: ast::DataType::Double(ast::ExactNumberInfo::None), utf8_cast_dtype: ast::DataType::Varchar(None), large_utf8_cast_dtype: ast::DataType::Text, date_field_extract_style: DateFieldExtractStyle::DatePart, @@ -718,7 +717,7 @@ impl CustomDialectBuilder { supports_nulls_first_in_sort: true, use_timestamp_for_date64: false, interval_style: IntervalStyle::PostgresVerbose, - float64_ast_dtype: ast::DataType::Double, + float64_ast_dtype: ast::DataType::Double(ast::ExactNumberInfo::None), utf8_cast_dtype: ast::DataType::Varchar(None), large_utf8_cast_dtype: ast::DataType::Text, date_field_extract_style: DateFieldExtractStyle::DatePart, diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index 5fad68cf46383..51ae73e03e29d 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -544,9 +544,9 @@ impl Unparser<'_> { } let array = self.expr_to_sql(&args[0])?; let index = self.expr_to_sql(&args[1])?; - Ok(ast::Expr::Subscript { - expr: Box::new(array), - subscript: Box::new(Subscript::Index { index }), + Ok(ast::Expr::CompoundFieldAccess { + root: Box::new(array), + access_chain: vec![ast::AccessExpr::Subscript(Subscript::Index { index })], }) } @@ -1667,6 +1667,7 @@ mod tests { use datafusion_functions_nested::map::map; use datafusion_functions_window::rank::rank_udwf; use datafusion_functions_window::row_number::row_number_udwf; + use sqlparser::ast::ExactNumberInfo; use crate::unparser::dialect::{ CharacterLengthStyle, CustomDialect, CustomDialectBuilder, DateFieldExtractStyle, @@ -2184,7 +2185,7 @@ mod tests { #[test] fn custom_dialect_float64_ast_dtype() -> Result<()> { for (float64_ast_dtype, identifier) in [ - (ast::DataType::Double, "DOUBLE"), + (ast::DataType::Double(ExactNumberInfo::None), "DOUBLE"), (ast::DataType::DoublePrecision, "DOUBLE PRECISION"), ] { let dialect = CustomDialectBuilder::new() diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 368131751e91d..0fa203c60b7ba 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -1089,7 +1089,7 @@ impl Unparser<'_> { &self, join_conditions: &[(Expr, Expr)], ) -> Option { - let mut idents = Vec::with_capacity(join_conditions.len()); + let mut object_names = Vec::with_capacity(join_conditions.len()); for (left, right) in join_conditions { match (left, right) { ( @@ -1104,14 +1104,18 @@ impl Unparser<'_> { spans: _, }), ) if left_name == right_name => { - idents.push(self.new_ident_quoted_if_needs(left_name.to_string())); + // For example, if the join condition `t1.id = t2.id` + // this is represented as two columns like `[t1.id, t2.id]` + // This code forms `id` (without relation name) + let ident = self.new_ident_quoted_if_needs(left_name.to_string()); + object_names.push(ast::ObjectName(vec![ident])); } // USING is only valid with matching column names; arbitrary expressions // are not allowed _ => return None, } } - Some(ast::JoinConstraint::Using(idents)) + Some(ast::JoinConstraint::Using(object_names)) } /// Convert a join constraint and associated conditions and filter to a SQL AST node diff --git a/datafusion/sql/tests/sql_integration.rs b/datafusion/sql/tests/sql_integration.rs index b9502c1520049..6a0db3888f836 100644 --- a/datafusion/sql/tests/sql_integration.rs +++ b/datafusion/sql/tests/sql_integration.rs @@ -3673,7 +3673,7 @@ fn test_non_prepare_statement_should_infer_types() { #[test] #[should_panic( - expected = "Expected: [NOT] NULL or TRUE|FALSE or [NOT] DISTINCT FROM after IS, found: $1" + expected = "Expected: [NOT] NULL | TRUE | FALSE | DISTINCT | [form] NORMALIZED FROM after IS, found: $1" )] fn test_prepare_statement_to_plan_panic_is_param() { let sql = "PREPARE my_plan(INT) AS SELECT id, age FROM person WHERE age is $1"; diff --git a/datafusion/sqllogictest/test_files/interval.slt b/datafusion/sqllogictest/test_files/interval.slt index db453adf12ba3..1ef3048ddc66a 100644 --- a/datafusion/sqllogictest/test_files/interval.slt +++ b/datafusion/sqllogictest/test_files/interval.slt @@ -25,27 +25,10 @@ select Interval(MonthDayNano) Interval(MonthDayNano) -## This is incredibly confusing but document it in tests: -# -# years is parsed as a column name -# year is parsed as part of the interval type. -# -# postgres=# select interval '5' year; -# interval -# ---------- -# 5 years -# (1 row) -# -# postgres=# select interval '5' years; -# years -# ---------- -# 00:00:05 -# (1 row) query ? select interval '5' years ---- -5.000000000 secs - +60 mons # check all different kinds of intervals query ? @@ -61,7 +44,7 @@ select interval '5' month query ? select interval '5' months ---- -5.000000000 secs +5 mons query ? select interval '5' week @@ -83,7 +66,7 @@ select interval '5' hour query ? select interval '5' hours ---- -5.000000000 secs +5 hours query ? select interval '5' minute diff --git a/datafusion/sqllogictest/test_files/joins.slt b/datafusion/sqllogictest/test_files/joins.slt index ac02aeb6fea4e..f16b2dbb2d0d5 100644 --- a/datafusion/sqllogictest/test_files/joins.slt +++ b/datafusion/sqllogictest/test_files/joins.slt @@ -2623,6 +2623,10 @@ SELECT t1.c1, t2.c2 FROM test_partition_table t1 JOIN test_partition_table t2 US 0 9 0 10 +# left_join_using_qualified (snowflake syntax) +query error DataFusion error: This feature is not implemented: Invalid identifier in USING clause\. Expected single identifier, got t2\.c2 +SELECT t1.c1, t2.c2 FROM test_partition_table t1 JOIN test_partition_table t2 USING (t2.c2) ORDER BY t2.c2; + # left_join_using_join_key_projection query III SELECT t1.c1, t1.c2, t2.c2 FROM test_partition_table t1 JOIN test_partition_table t2 USING (c2) ORDER BY t2.c2