diff --git a/Cargo.toml b/Cargo.toml index aa412cba5108..a476e3e256f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -188,3 +188,7 @@ large_futures = "warn" [workspace.lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ["cfg(tarpaulin)"] } unused_qualifications = "deny" + +# https://github.com/apache/datafusion-sqlparser-rs/commit/5da702fc19f9dc73559d9a6f0408729f1121444a +[patch.crates-io] +sqlparser = { git = "https://github.com/sqlparser-rs/sqlparser-rs.git", rev="5da702fc19f9dc73559d9a6f0408729f1121444a" } diff --git a/datafusion-cli/Cargo.lock b/datafusion-cli/Cargo.lock index b267b3fea485..be774bc38deb 100644 --- a/datafusion-cli/Cargo.lock +++ b/datafusion-cli/Cargo.lock @@ -3778,18 +3778,17 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "sqlparser" version = "0.53.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05a528114c392209b3264855ad491fcce534b94a38771b0a0b97a79379275ce8" +source = "git+https://github.com/sqlparser-rs/sqlparser-rs.git?rev=5da702fc19f9dc73559d9a6f0408729f1121444a#5da702fc19f9dc73559d9a6f0408729f1121444a" dependencies = [ "log", + "recursive", "sqlparser_derive", ] [[package]] name = "sqlparser_derive" version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da5fc6819faabb412da764b99d3b713bb55083c11e7e0c00144d386cd6a1939c" +source = "git+https://github.com/sqlparser-rs/sqlparser-rs.git?rev=5da702fc19f9dc73559d9a6f0408729f1121444a#5da702fc19f9dc73559d9a6f0408729f1121444a" dependencies = [ "proc-macro2", "quote", diff --git a/datafusion-cli/Cargo.toml b/datafusion-cli/Cargo.toml index b9d190ac07cc..50c5cea829e1 100644 --- a/datafusion-cli/Cargo.toml +++ b/datafusion-cli/Cargo.toml @@ -84,3 +84,7 @@ debug = false debug-assertions = false strip = "debuginfo" incremental = false + +# https://github.com/apache/datafusion-sqlparser-rs/commit/5da702fc19f9dc73559d9a6f0408729f1121444a +[patch.crates-io] +sqlparser = { git = "https://github.com/sqlparser-rs/sqlparser-rs.git", rev="5da702fc19f9dc73559d9a6f0408729f1121444a" } diff --git a/datafusion/common/src/column.rs b/datafusion/common/src/column.rs index fdde3d69eddb..ebb0164b2e98 100644 --- a/datafusion/common/src/column.rs +++ b/datafusion/common/src/column.rs @@ -71,7 +71,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"]` + pub fn from_idents(mut idents: Vec) -> Option { let (relation, name) = match idents.len() { 1 => (None, idents.remove(0)), 2 => ( @@ -109,7 +113,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, @@ -120,7 +124,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 93be04c27564..82acebee3de6 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 9adf14459081..07a3fad66e5c 100644 --- a/datafusion/sql/src/expr/identifier.rs +++ b/datafusion/sql/src/expr/identifier.rs @@ -83,7 +83,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 9b40ebdaf6a5..d913cc7fa10e 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; use datafusion_expr::expr::{InList, WildcardOptions}; @@ -236,14 +236,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) @@ -984,84 +984,141 @@ 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 }) => { + 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()), + }); + 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 f185d65fa194..7e39141720f5 100644 --- a/datafusion/sql/src/parser.rs +++ b/datafusion/sql/src/parser.rs @@ -563,7 +563,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()); @@ -666,7 +666,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)?) @@ -676,7 +676,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 628bcb2fbdcd..b76324771617 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -430,7 +430,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), @@ -566,7 +569,9 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { | SQLDataType::MediumText | SQLDataType::LongText | SQLDataType::Bit(_) - |SQLDataType::BitVarying(_) + | SQLDataType::BitVarying(_) + // BIG Query 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 75f39792bce1..bd04725853ab 100644 --- a/datafusion/sql/src/relation/join.rs +++ b/datafusion/sql/src/relation/join.rs @@ -16,7 +16,7 @@ // under the License. use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; -use datafusion_common::{not_impl_err, Column, Result}; +use datafusion_common::{not_impl_datafusion_err, not_impl_err, Column, Result}; use datafusion_expr::{JoinType, LogicalPlan, LogicalPlanBuilder}; use sqlparser::ast::{Join, JoinConstraint, JoinOperator, TableFactor, TableWithJoins}; use std::collections::HashSet; @@ -123,11 +123,21 @@ 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 idents = object_name + .0 + .into_iter() + .map(|id| self.ident_normalizer.normalize(id)) + .collect::>(); + Column::from_idents(idents).ok_or_else(|| { + not_impl_datafusion_err!("Invalid identifier in USING clause") + }) + }) + .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 75fdbd20e840..83406321347e 100644 --- a/datafusion/sql/src/set_expr.rs +++ b/datafusion/sql/src/set_expr.rs @@ -88,6 +88,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 dfd3a4fd76a2..83c91ecde69a 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 e320a4510e46..6d77c01ea888 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 5c318a96ef6c..50b1d9e4ca06 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; @@ -27,8 +28,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>; @@ -63,7 +62,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 @@ -511,7 +510,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, @@ -692,7 +691,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 7a110fd0785c..a82eba1fb5ca 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -523,9 +523,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 })], }) } @@ -1633,6 +1633,7 @@ mod tests { use datafusion_functions_nested::expr_fn::{array_element, make_array}; use datafusion_functions_nested::map::map; use datafusion_functions_window::row_number::row_number_udwf; + use sqlparser::ast::ExactNumberInfo; use crate::unparser::dialect::{ CharacterLengthStyle, CustomDialect, CustomDialectBuilder, DateFieldExtractStyle, @@ -2123,7 +2124,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 2e02baa4f7f5..a375f69841ca 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) { ( @@ -1102,14 +1102,18 @@ impl Unparser<'_> { name: right_name, }), ) 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 f6533dceca4f..2d1042d3130e 100644 --- a/datafusion/sql/tests/sql_integration.rs +++ b/datafusion/sql/tests/sql_integration.rs @@ -3671,7 +3671,7 @@ fn test_non_prepare_statement_should_infer_types() { #[test] #[should_panic( - expected = "value: SQL(ParserError(\"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 db453adf12ba..1ef3048ddc66 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