From 2ea74e043fae6d75dd7b4449e5a6e2b3b6ed7205 Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Sun, 19 Jan 2025 08:35:06 -0500
Subject: [PATCH 1/2] TEMP: Pin to upstream SQLParser version

---
 Cargo.toml                | 4 ++++
 datafusion-cli/Cargo.lock | 7 +++----
 datafusion-cli/Cargo.toml | 4 ++++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index aa412cba51087..a476e3e256f07 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 b267b3fea485f..be774bc38debd 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 b9d190ac07cc5..50c5cea829e15 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" }

From 5bb20ad20430e77f6cec53180833f430fda0b52f Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Sun, 19 Jan 2025 08:38:21 -0500
Subject: [PATCH 2/2] Update for API changes

---
 datafusion/common/src/column.rs               |  10 +-
 datafusion/expr/src/logical_plan/statement.rs |   1 +
 datafusion/sql/src/expr/identifier.rs         |   2 +-
 datafusion/sql/src/expr/mod.rs                | 219 +++++++++++-------
 datafusion/sql/src/parser.rs                  |   6 +-
 datafusion/sql/src/planner.rs                 |   9 +-
 datafusion/sql/src/relation/join.rs           |  20 +-
 datafusion/sql/src/set_expr.rs                |   3 +
 datafusion/sql/src/statement.rs               |  51 +++-
 datafusion/sql/src/unparser/ast.rs            |   1 +
 datafusion/sql/src/unparser/dialect.rs        |   9 +-
 datafusion/sql/src/unparser/expr.rs           |   9 +-
 datafusion/sql/src/unparser/plan.rs           |  10 +-
 datafusion/sql/tests/sql_integration.rs       |   2 +-
 .../sqllogictest/test_files/interval.slt      |  23 +-
 15 files changed, 240 insertions(+), 135 deletions(-)

diff --git a/datafusion/common/src/column.rs b/datafusion/common/src/column.rs
index fdde3d69eddba..ebb0164b2e983 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<String>) -> Option<Self> {
+    /// 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<String>) -> Option<Self> {
         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<String>) -> 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<String>) -> 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 9adf14459081e..07a3fad66e5cb 100644
--- a/datafusion/sql/src/expr/identifier.rs
+++ b/datafusion/sql/src/expr/identifier.rs
@@ -83,7 +83,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
         }
     }
 
-    pub(super) fn sql_compound_identifier_to_expr(
+    pub(crate) fn sql_compound_identifier_to_expr(
         &self,
         ids: Vec<Ident>,
         schema: &DFSchema,
diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs
index 9b40ebdaf6a5a..d913cc7fa10ec 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<S: ContextProvider> SqlToRel<'_, S> {
                 self.sql_identifier_to_expr(id, schema, planner_context)
             }
 
-            SQLExpr::MapAccess { .. } => {
-                not_impl_err!("Map Access")
-            }
-
             // <expr>["foo"], <expr>[4] or <expr>[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<S: ContextProvider> 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<Subscript>,
+        root: SQLExpr,
+        access_chain: Vec<AccessExpr>,
         schema: &DFSchema,
         planner_context: &mut PlannerContext,
     ) -> Result<Expr> {
-        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::<Result<Vec<_>>>()?;
 
-        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 f185d65fa1947..7e39141720f5c 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<ColumnDef, ParserError> {
-        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 628bcb2fbdcd9..b76324771617e 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 75f39792bce1d..bd04725853abd 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<S: ContextProvider> SqlToRel<'_, S> {
                     .join_on(right, join_type, Some(expr))?
                     .build()
             }
-            JoinConstraint::Using(idents) => {
-                let keys: Vec<Column> = 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::<Vec<_>>();
+                        Column::from_idents(idents).ok_or_else(|| {
+                            not_impl_datafusion_err!("Invalid identifier in USING clause")
+                        })
+                    })
+                    .collect::<Result<Vec<_>>>()?;
+
                 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 75fdbd20e8401..83406321347e9 100644
--- a/datafusion/sql/src/set_expr.rs
+++ b/datafusion/sql/src/set_expr.rs
@@ -88,6 +88,9 @@ impl<S: ContextProvider> 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<S: ContextProvider> SqlToRel<'_, S> {
                 if_not_exists,
                 temporary,
                 to,
+                params,
             } => {
                 if materialized {
                     return not_impl_err!("Materialized views not supported")?;
@@ -532,6 +533,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                     if_not_exists,
                     temporary,
                     to,
+                    params,
                 };
                 let sql = stmt.to_string();
                 let Statement::CreateView {
@@ -818,7 +820,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
             Statement::Insert(Insert {
                 or,
                 into,
-                table_name,
                 columns,
                 overwrite,
                 source,
@@ -832,7 +833,17 @@ impl<S: ContextProvider> 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<S: ContextProvider> 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<S: ContextProvider> 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<S: ContextProvider> 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<S: ContextProvider> 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<S: ContextProvider> 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 5c318a96ef6c9..50b1d9e4ca065 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<dyn Fn(&Unparser, &[Expr]) -> Result<Option<ast::Expr>> + 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 7a110fd0785ca..a82eba1fb5ca8 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 2e02baa4f7f58..a375f69841ca6 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<ast::JoinConstraint> {
-        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 f6533dceca4f4..2d1042d3130e7 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 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
