From f70d90a6410d66540b1acab54df08eede6fe3596 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Fri, 23 Aug 2024 14:55:58 +0100
Subject: [PATCH 01/14] refactor "INTERVAL" parsing

---
 src/dialect/duckdb.rs       |   4 +
 src/dialect/mod.rs          |  17 +++++
 src/dialect/postgresql.rs   |   4 +
 src/dialect/redshift.rs     |   4 +
 src/dialect/snowflake.rs    |   4 +
 src/parser/mod.rs           | 140 ++++++++++++++++-------------------
 tests/sqlparser_bigquery.rs |  18 ++---
 tests/sqlparser_common.rs   | 141 +++++++++++++++++++++++++-----------
 tests/sqlparser_postgres.rs |   5 +-
 9 files changed, 207 insertions(+), 130 deletions(-)

diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
index 1fc211685a..e43f407cf6 100644
--- a/src/dialect/duckdb.rs
+++ b/src/dialect/duckdb.rs
@@ -55,4 +55,8 @@ impl Dialect for DuckDbDialect {
     fn support_map_literal_syntax(&self) -> bool {
         true
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index df3022adcb..fd95f63e43 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -499,6 +499,23 @@ pub trait Dialect: Debug + Any {
     fn describe_requires_table_keyword(&self) -> bool {
         false
     }
+
+    /// Whether or not units are required with interval expressions.
+    ///
+    /// When `true`:
+    /// * `INTERVAL '1' DAY` is VALID
+    /// * `INTERVAL 1 + 1 DAY` is VALID
+    /// * `INTERVAL '1' + '1' DAY` is VALID
+    /// * `INTERVAL '1'` is INVALID
+    ///
+    /// When `false`:
+    /// * `INTERVAL '1' DAY` is VALID
+    /// * `INTERVAL '1'` is VALID
+    /// * `INTERVAL '1 second'` is VALID
+    /// * `INTERVAL 1 + 1 DAY` is INVALID
+    fn require_interval_units(&self) -> bool {
+        true
+    }
 }
 
 /// This represents the operators for which precedence must be defined
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index c25a80f674..4a3eeaf33b 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -154,6 +154,10 @@ impl Dialect for PostgreSqlDialect {
             Precedence::Or => OR_PREC,
         }
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
 
 pub fn parse_comment(parser: &mut Parser) -> Result<Statement, ParserError> {
diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs
index bd4dc817ce..b6cce26e07 100644
--- a/src/dialect/redshift.rs
+++ b/src/dialect/redshift.rs
@@ -63,4 +63,8 @@ impl Dialect for RedshiftSqlDialect {
     fn supports_connect_by(&self) -> bool {
         true
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index 89508c3bbb..cd70c21167 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -158,6 +158,10 @@ impl Dialect for SnowflakeDialect {
     fn describe_requires_table_keyword(&self) -> bool {
         true
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
 
 /// Parse snowflake create table statement.
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 1eff8f7d5e..e4364b1309 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -56,15 +56,6 @@ macro_rules! parser_err {
     };
 }
 
-// Returns a successful result if the optional expression is some
-macro_rules! return_ok_if_some {
-    ($e:expr) => {{
-        if let Some(v) = $e {
-            return Ok(v);
-        }
-    }};
-}
-
 #[cfg(feature = "std")]
 /// Implementation [`RecursionCounter`] if std is available
 mod recursion {
@@ -896,33 +887,61 @@ impl<'a> Parser<'a> {
         Ok(expr)
     }
 
-    pub fn parse_interval_expr(&mut self) -> Result<Expr, ParserError> {
-        let precedence = self.dialect.prec_unknown();
+    fn parse_interval_expr(&mut self) -> Result<(Expr, bool), ParserError> {
         let mut expr = self.parse_prefix()?;
 
-        loop {
-            let next_precedence = self.get_next_interval_precedence()?;
-
-            if precedence >= next_precedence {
-                break;
-            }
-
-            expr = self.parse_infix(expr, next_precedence)?;
-        }
-
-        Ok(expr)
-    }
-
-    /// Get the precedence of the next token, with AND, OR, and XOR.
-    pub fn get_next_interval_precedence(&self) -> Result<u8, ParserError> {
-        let token = self.peek_token();
-
-        match token.token {
-            Token::Word(w) if w.keyword == Keyword::AND => Ok(self.dialect.prec_unknown()),
-            Token::Word(w) if w.keyword == Keyword::OR => Ok(self.dialect.prec_unknown()),
-            Token::Word(w) if w.keyword == Keyword::XOR => Ok(self.dialect.prec_unknown()),
-            _ => self.get_next_precedence(),
-        }
+        if self.dialect.require_interval_units() {
+            // if require_interval_units is true, continue parsing expressions until a unit is foudn
+            loop {
+                if self.next_token_is_unit() {
+                    return Ok((expr, true));
+                } else {
+                    expr = self.parse_infix(expr, self.dialect.prec_unknown())?;
+                }
+            }
+        } else {
+            // otherwise, check if the next token is a unit, but don't iterate
+            Ok((expr, self.next_token_is_unit()))
+        }
+    }
+
+    pub fn next_token_is_unit(&mut self) -> bool {
+        let token_loc = self.peek_token();
+        if let Token::Word(word) = token_loc.token {
+            if matches!(
+                word.keyword,
+                Keyword::YEAR
+                    | Keyword::MONTH
+                    | Keyword::WEEK
+                    | Keyword::DAY
+                    | Keyword::HOUR
+                    | Keyword::MINUTE
+                    | Keyword::SECOND
+                    | Keyword::CENTURY
+                    | Keyword::DECADE
+                    | Keyword::DOW
+                    | Keyword::DOY
+                    | Keyword::EPOCH
+                    | Keyword::ISODOW
+                    | Keyword::ISOYEAR
+                    | Keyword::JULIAN
+                    | Keyword::MICROSECOND
+                    | Keyword::MICROSECONDS
+                    | Keyword::MILLENIUM
+                    | Keyword::MILLENNIUM
+                    | Keyword::MILLISECOND
+                    | Keyword::MILLISECONDS
+                    | Keyword::NANOSECOND
+                    | Keyword::NANOSECONDS
+                    | Keyword::QUARTER
+                    | Keyword::TIMEZONE
+                    | Keyword::TIMEZONE_HOUR
+                    | Keyword::TIMEZONE_MINUTE
+            ) {
+                return true;
+            }
+        }
+        return false;
     }
 
     pub fn parse_assert(&mut self) -> Result<Statement, ParserError> {
@@ -972,7 +991,7 @@ impl<'a> Parser<'a> {
         // name is not followed by a string literal, but in fact in PostgreSQL it is a valid
         // expression that should parse as the column name "date".
         let loc = self.peek_token().location;
-        return_ok_if_some!(self.maybe_parse(|parser| {
+        let opt_expr = self.maybe_parse(|parser| {
             match parser.parse_data_type()? {
                 DataType::Interval => parser.parse_interval(),
                 // PostgreSQL allows almost any identifier to be used as custom data type name,
@@ -988,7 +1007,11 @@ impl<'a> Parser<'a> {
                     value: parser.parse_literal_string()?,
                 }),
             }
-        }));
+        });
+
+        if let Some(expr) = opt_expr {
+            return Ok(expr);
+        }
 
         let next_token = self.next_token();
         let expr = match next_token.token {
@@ -2079,52 +2102,17 @@ impl<'a> Parser<'a> {
         // don't currently try to parse it. (The sign can instead be included
         // inside the value string.)
 
-        // The first token in an interval is a string literal which specifies
-        // the duration of the interval.
-        let value = self.parse_interval_expr()?;
+        let (value, has_units) = self.parse_interval_expr()?;
 
         // Following the string literal is a qualifier which indicates the units
         // of the duration specified in the string literal.
         //
         // Note that PostgreSQL allows omitting the qualifier, so we provide
         // this more general implementation.
-        let leading_field = match self.peek_token().token {
-            Token::Word(kw)
-                if [
-                    Keyword::YEAR,
-                    Keyword::MONTH,
-                    Keyword::WEEK,
-                    Keyword::DAY,
-                    Keyword::HOUR,
-                    Keyword::MINUTE,
-                    Keyword::SECOND,
-                    Keyword::CENTURY,
-                    Keyword::DECADE,
-                    Keyword::DOW,
-                    Keyword::DOY,
-                    Keyword::EPOCH,
-                    Keyword::ISODOW,
-                    Keyword::ISOYEAR,
-                    Keyword::JULIAN,
-                    Keyword::MICROSECOND,
-                    Keyword::MICROSECONDS,
-                    Keyword::MILLENIUM,
-                    Keyword::MILLENNIUM,
-                    Keyword::MILLISECOND,
-                    Keyword::MILLISECONDS,
-                    Keyword::NANOSECOND,
-                    Keyword::NANOSECONDS,
-                    Keyword::QUARTER,
-                    Keyword::TIMEZONE,
-                    Keyword::TIMEZONE_HOUR,
-                    Keyword::TIMEZONE_MINUTE,
-                ]
-                .iter()
-                .any(|d| kw.keyword == *d) =>
-            {
-                Some(self.parse_date_time_field()?)
-            }
-            _ => None,
+        let leading_field = if has_units {
+            Some(self.parse_date_time_field()?)
+        } else {
+            None
         };
 
         let (leading_precision, last_field, fsec_precision) =
diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs
index 57cf9d7fd4..ac71c482f9 100644
--- a/tests/sqlparser_bigquery.rs
+++ b/tests/sqlparser_bigquery.rs
@@ -830,16 +830,14 @@ fn parse_typed_struct_syntax_bigquery() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1' DAY), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
             values: vec![Expr::Interval(ast::Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString(
-                    "1-2 3 4:5:6.789999".to_string()
-                ))),
-                leading_field: None,
+                value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
+                leading_field: Some(DateTimeField::Day),
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
@@ -1141,16 +1139,14 @@ fn parse_typed_struct_syntax_bigquery_and_generic() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '2' MONTH), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery_and_generic().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
-            values: vec![Expr::Interval(ast::Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString(
-                    "1-2 3 4:5:6.789999".to_string()
-                ))),
-                leading_field: None,
+            values: vec![Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))),
+                leading_field: Some(DateTimeField::Month),
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 517e978b4f..111e79b017 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4464,9 +4464,9 @@ fn parse_window_functions() {
                avg(bar) OVER (ORDER BY a \
                RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING), \
                sum(bar) OVER (ORDER BY a \
-               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1 MONTH' FOLLOWING), \
+               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' MONTH FOLLOWING), \
                COUNT(*) OVER (ORDER BY a \
-               RANGE BETWEEN INTERVAL '1 DAY' PRECEDING AND INTERVAL '1 DAY' FOLLOWING), \
+               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' DAY FOLLOWING), \
                max(baz) OVER (ORDER BY a \
                ROWS UNBOUNDED PRECEDING), \
                sum(qux) OVER (ORDER BY a \
@@ -4886,9 +4886,11 @@ fn parse_literal_timestamp_with_time_zone() {
 }
 
 #[test]
-fn parse_interval() {
+fn parse_interval_unit_required() {
+    let dialects_unit_required = all_dialects_where(|d| d.require_interval_units());
+
     let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))),
@@ -4901,7 +4903,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
@@ -4916,7 +4918,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL '1' SECOND (5, 4)";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1")))),
@@ -4929,7 +4931,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL '10' HOUR";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))),
@@ -4942,7 +4944,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL 5 DAY";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(number("5"))),
@@ -4955,7 +4957,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL 1 + 1 DAY";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::BinaryOp {
@@ -4972,7 +4974,7 @@ fn parse_interval() {
     );
 
     let sql = "SELECT INTERVAL '10' HOUR (1)";
-    let select = verified_only_select(sql);
+    let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))),
@@ -4984,8 +4986,39 @@ fn parse_interval() {
         expr_from_projection(only(&select.projection)),
     );
 
+    let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
+    assert_eq!(
+        ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
+        result.unwrap_err(),
+    );
+
+    let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)");
+    assert_eq!(
+        ParserError::ParserError("Expected: end of statement, found: (".to_string()),
+        result.unwrap_err(),
+    );
+
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MONTH");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' SECOND");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
+    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
+}
+
+#[test]
+fn parse_interval_unit_not_required() {
+    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+
     let sql = "SELECT INTERVAL '1 DAY'";
-    let select = verified_only_select(sql);
+    let select = dialect_no_unit.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
@@ -4999,37 +5032,60 @@ fn parse_interval() {
         expr_from_projection(only(&select.projection)),
     );
 
-    let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
-    assert_eq!(
-        ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
-        result.unwrap_err(),
+    dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'");
+    dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year");
+    dialect_no_unit.one_statement_parses_to(
+        "SELECT INTERVAL '1 YEAR' one_year",
+        "SELECT INTERVAL '1 YEAR' AS one_year",
     );
+}
 
-    let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)");
+#[test]
+fn interval_unit_not_required_gt() {
+    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
-        ParserError::ParserError("Expected: end of statement, found: (".to_string()),
-        result.unwrap_err(),
-    );
+        expr,
+        Expr::BinaryOp {
+            left: Box::new(Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString(
+                    "1 second".to_string()
+                ))),
+                leading_field: None,
+                leading_precision: None,
+                last_field: None,
+                fractional_seconds_precision: None,
+            },)),
+            op: BinaryOperator::Gt,
+            right: Box::new(Expr::Identifier(Ident {
+                value: "x".to_string(),
+                quote_style: None,
+            })),
+        }
+    )
+}
 
-    verified_only_select("SELECT INTERVAL '1' YEAR");
-    verified_only_select("SELECT INTERVAL '1' MONTH");
-    verified_only_select("SELECT INTERVAL '1' DAY");
-    verified_only_select("SELECT INTERVAL '1' HOUR");
-    verified_only_select("SELECT INTERVAL '1' MINUTE");
-    verified_only_select("SELECT INTERVAL '1' SECOND");
-    verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
-    verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
-    verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
-    verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
-    verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
-    verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
-    verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
-    verified_only_select("SELECT INTERVAL '1 YEAR'");
-    verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year");
-    one_statement_parses_to(
-        "SELECT INTERVAL '1 YEAR' one_year",
-        "SELECT INTERVAL '1 YEAR' AS one_year",
-    );
+#[test]
+fn interval_unit_not_required_double_colon() {
+    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT");
+    assert_eq!(
+        expr,
+        Expr::Cast {
+            kind: CastKind::DoubleColon,
+            expr: Box::new(Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString(
+                    "1 second".to_string()
+                ))),
+                leading_field: None,
+                leading_precision: None,
+                last_field: None,
+                fractional_seconds_precision: None,
+            })),
+            data_type: DataType::Text,
+            format: None,
+        }
+    )
 }
 
 #[test]
@@ -5038,7 +5094,7 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let actual_ast = Parser::parse_sql(&GenericDialect {}, sql).unwrap();
+    let actual_ast = Parser::parse_sql(&PostgreSqlDialect {}, sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
         with: None,
@@ -5140,19 +5196,20 @@ fn parse_interval_and_or_xor() {
 
     assert_eq!(actual_ast, expected_ast);
 
-    verified_stmt(
+    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    dialect_no_unit.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'",
     );
 
-    verified_stmt(
+    dialect_no_unit.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         OR d2_date > d1_date + INTERVAL '3 days'",
     );
 
-    verified_stmt(
+    dialect_no_unit.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         XOR d2_date > d1_date + INTERVAL '3 days'",
diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs
index 2f9fe86c9e..4ba338dc0f 100644
--- a/tests/sqlparser_postgres.rs
+++ b/tests/sqlparser_postgres.rs
@@ -4353,7 +4353,10 @@ fn parse_mat_cte() {
 fn parse_at_time_zone() {
     pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE tz");
     pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE ('America/' || 'Los_Angeles')");
+}
 
+#[test]
+fn parse_at_time_zone_interval() {
     // check precedence
     let expr = Expr::BinaryOp {
         left: Box::new(Expr::AtTimeZone {
@@ -4382,7 +4385,7 @@ fn parse_at_time_zone() {
         })),
     };
     pretty_assertions::assert_eq!(
-        pg_and_generic().verified_expr(
+        pg().verified_expr(
             "TIMESTAMP '2001-09-28 01:00' AT TIME ZONE 'America/Los_Angeles'::TEXT + INTERVAL '23 hours'",
         ),
         expr

From f643d44ca7d810f12e3b763184f48ad69e07e636 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Fri, 23 Aug 2024 15:06:36 +0100
Subject: [PATCH 02/14] rearrange Parser code, split out some tests

---
 src/parser/mod.rs         | 121 ++++++++++++++++++++------------------
 tests/sqlparser_common.rs |  38 +++++++-----
 2 files changed, 87 insertions(+), 72 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index e4364b1309..dc5f9f19b8 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -887,63 +887,6 @@ impl<'a> Parser<'a> {
         Ok(expr)
     }
 
-    fn parse_interval_expr(&mut self) -> Result<(Expr, bool), ParserError> {
-        let mut expr = self.parse_prefix()?;
-
-        if self.dialect.require_interval_units() {
-            // if require_interval_units is true, continue parsing expressions until a unit is foudn
-            loop {
-                if self.next_token_is_unit() {
-                    return Ok((expr, true));
-                } else {
-                    expr = self.parse_infix(expr, self.dialect.prec_unknown())?;
-                }
-            }
-        } else {
-            // otherwise, check if the next token is a unit, but don't iterate
-            Ok((expr, self.next_token_is_unit()))
-        }
-    }
-
-    pub fn next_token_is_unit(&mut self) -> bool {
-        let token_loc = self.peek_token();
-        if let Token::Word(word) = token_loc.token {
-            if matches!(
-                word.keyword,
-                Keyword::YEAR
-                    | Keyword::MONTH
-                    | Keyword::WEEK
-                    | Keyword::DAY
-                    | Keyword::HOUR
-                    | Keyword::MINUTE
-                    | Keyword::SECOND
-                    | Keyword::CENTURY
-                    | Keyword::DECADE
-                    | Keyword::DOW
-                    | Keyword::DOY
-                    | Keyword::EPOCH
-                    | Keyword::ISODOW
-                    | Keyword::ISOYEAR
-                    | Keyword::JULIAN
-                    | Keyword::MICROSECOND
-                    | Keyword::MICROSECONDS
-                    | Keyword::MILLENIUM
-                    | Keyword::MILLENNIUM
-                    | Keyword::MILLISECOND
-                    | Keyword::MILLISECONDS
-                    | Keyword::NANOSECOND
-                    | Keyword::NANOSECONDS
-                    | Keyword::QUARTER
-                    | Keyword::TIMEZONE
-                    | Keyword::TIMEZONE_HOUR
-                    | Keyword::TIMEZONE_MINUTE
-            ) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     pub fn parse_assert(&mut self) -> Result<Statement, ParserError> {
         let condition = self.parse_expr()?;
         let message = if self.parse_keyword(Keyword::AS) {
@@ -2102,7 +2045,11 @@ impl<'a> Parser<'a> {
         // don't currently try to parse it. (The sign can instead be included
         // inside the value string.)
 
-        let (value, has_units) = self.parse_interval_expr()?;
+        let (value, has_units) = if self.dialect.require_interval_units() {
+            self.parse_interval_expr_units_required()?
+        } else {
+            self.parse_interval_expr_units_not_require()?
+        };
 
         // Following the string literal is a qualifier which indicates the units
         // of the duration specified in the string literal.
@@ -2149,6 +2096,64 @@ impl<'a> Parser<'a> {
         }))
     }
 
+    /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found
+    pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> {
+        let mut expr = self.parse_prefix()?;
+
+        loop {
+            if self.next_token_is_unit() {
+                return Ok((expr, true));
+            } else {
+                expr = self.parse_infix(expr, self.dialect.prec_unknown())?;
+            }
+        }
+    }
+
+    /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit
+    pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> {
+        self.parse_prefix()
+            .map(|expr| (expr, self.next_token_is_unit()))
+    }
+
+    pub fn next_token_is_unit(&mut self) -> bool {
+        let token_loc = self.peek_token();
+        if let Token::Word(word) = token_loc.token {
+            if matches!(
+                word.keyword,
+                Keyword::YEAR
+                    | Keyword::MONTH
+                    | Keyword::WEEK
+                    | Keyword::DAY
+                    | Keyword::HOUR
+                    | Keyword::MINUTE
+                    | Keyword::SECOND
+                    | Keyword::CENTURY
+                    | Keyword::DECADE
+                    | Keyword::DOW
+                    | Keyword::DOY
+                    | Keyword::EPOCH
+                    | Keyword::ISODOW
+                    | Keyword::ISOYEAR
+                    | Keyword::JULIAN
+                    | Keyword::MICROSECOND
+                    | Keyword::MICROSECONDS
+                    | Keyword::MILLENIUM
+                    | Keyword::MILLENNIUM
+                    | Keyword::MILLISECOND
+                    | Keyword::MILLISECONDS
+                    | Keyword::NANOSECOND
+                    | Keyword::NANOSECONDS
+                    | Keyword::QUARTER
+                    | Keyword::TIMEZONE
+                    | Keyword::TIMEZONE_HOUR
+                    | Keyword::TIMEZONE_MINUTE
+            ) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /// Bigquery specific: Parse a struct literal
     /// Syntax
     /// ```sql
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 111e79b017..9852e05f50 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4885,6 +4885,30 @@ fn parse_literal_timestamp_with_time_zone() {
     one_statement_parses_to("SELECT TIMESTAMPTZ '1999-01-01 01:23:34Z'", sql);
 }
 
+#[test]
+fn parse_interval_all() {
+    // these intervals expressions should work with both variants of INTERVAL
+    verified_only_select("SELECT INTERVAL '1' YEAR");
+    verified_only_select("SELECT INTERVAL '1' MONTH");
+    verified_only_select("SELECT INTERVAL '1' DAY");
+    verified_only_select("SELECT INTERVAL '1' HOUR");
+    verified_only_select("SELECT INTERVAL '1' MINUTE");
+    verified_only_select("SELECT INTERVAL '1' SECOND");
+    verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
+    verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
+    verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
+    verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
+    verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
+    verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
+    verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
+    verified_only_select("SELECT INTERVAL 1 YEAR");
+    verified_only_select("SELECT INTERVAL 1 MONTH");
+    verified_only_select("SELECT INTERVAL 1 DAY");
+    verified_only_select("SELECT INTERVAL 1 HOUR");
+    verified_only_select("SELECT INTERVAL 1 MINUTE");
+    verified_only_select("SELECT INTERVAL 1 SECOND");
+}
+
 #[test]
 fn parse_interval_unit_required() {
     let dialects_unit_required = all_dialects_where(|d| d.require_interval_units());
@@ -4997,20 +5021,6 @@ fn parse_interval_unit_required() {
         ParserError::ParserError("Expected: end of statement, found: (".to_string()),
         result.unwrap_err(),
     );
-
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MONTH");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' SECOND");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
-    dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
 }
 
 #[test]

From d61d73e97a163868cce0f90ad806dc14f3cd1a5c Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Fri, 23 Aug 2024 15:16:48 +0100
Subject: [PATCH 03/14] cleanup next_token_is_unit

---
 src/parser/mod.rs | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index dc5f9f19b8..75866a29ea 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2116,9 +2116,8 @@ impl<'a> Parser<'a> {
     }
 
     pub fn next_token_is_unit(&mut self) -> bool {
-        let token_loc = self.peek_token();
-        if let Token::Word(word) = token_loc.token {
-            if matches!(
+        if let Token::Word(word) = self.peek_token().token {
+            matches!(
                 word.keyword,
                 Keyword::YEAR
                     | Keyword::MONTH
@@ -2147,11 +2146,10 @@ impl<'a> Parser<'a> {
                     | Keyword::TIMEZONE
                     | Keyword::TIMEZONE_HOUR
                     | Keyword::TIMEZONE_MINUTE
-            ) {
-                return true;
-            }
+            )
+        } else {
+            false
         }
-        return false;
     }
 
     /// Bigquery specific: Parse a struct literal

From 83e93982381147a5abc79348480c6c758371da26 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Fri, 23 Aug 2024 15:29:45 +0100
Subject: [PATCH 04/14] more comments and improve tests

---
 src/parser/mod.rs         |   8 +++
 tests/sqlparser_common.rs | 121 ++++++++++++++++++++++++--------------
 2 files changed, 84 insertions(+), 45 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 75866a29ea..0e21eebabe 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2097,6 +2097,10 @@ impl<'a> Parser<'a> {
     }
 
     /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found
+    ///
+    /// # Returns
+    ///
+    /// A tuple of (interval expression, whether a unit is found)
     pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> {
         let mut expr = self.parse_prefix()?;
 
@@ -2110,6 +2114,10 @@ impl<'a> Parser<'a> {
     }
 
     /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit
+    ///
+    /// # Returns
+    ///
+    /// A tuple of (interval expression, whether a unit is found)
     pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> {
         self.parse_prefix()
             .map(|expr| (expr, self.next_token_is_unit()))
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 9852e05f50..a75f9dcb0c 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4887,34 +4887,23 @@ fn parse_literal_timestamp_with_time_zone() {
 
 #[test]
 fn parse_interval_all() {
-    // these intervals expressions should work with both variants of INTERVAL
-    verified_only_select("SELECT INTERVAL '1' YEAR");
-    verified_only_select("SELECT INTERVAL '1' MONTH");
-    verified_only_select("SELECT INTERVAL '1' DAY");
-    verified_only_select("SELECT INTERVAL '1' HOUR");
-    verified_only_select("SELECT INTERVAL '1' MINUTE");
-    verified_only_select("SELECT INTERVAL '1' SECOND");
-    verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
-    verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
-    verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
-    verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
-    verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
-    verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
-    verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
-    verified_only_select("SELECT INTERVAL 1 YEAR");
-    verified_only_select("SELECT INTERVAL 1 MONTH");
-    verified_only_select("SELECT INTERVAL 1 DAY");
-    verified_only_select("SELECT INTERVAL 1 HOUR");
-    verified_only_select("SELECT INTERVAL 1 MINUTE");
-    verified_only_select("SELECT INTERVAL 1 SECOND");
-}
+    // these intervals expressions all work with both variants of INTERVAL
 
-#[test]
-fn parse_interval_unit_required() {
-    let dialects_unit_required = all_dialects_where(|d| d.require_interval_units());
+    let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH";
+    let select = verified_only_select(sql);
+    assert_eq!(
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))),
+            leading_field: Some(DateTimeField::Year),
+            leading_precision: None,
+            last_field: Some(DateTimeField::Month),
+            fractional_seconds_precision: None,
+        }),
+        expr_from_projection(only(&select.projection)),
+    );
 
     let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))),
@@ -4927,7 +4916,7 @@ fn parse_interval_unit_required() {
     );
 
     let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
@@ -4942,7 +4931,7 @@ fn parse_interval_unit_required() {
     );
 
     let sql = "SELECT INTERVAL '1' SECOND (5, 4)";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1")))),
@@ -4955,7 +4944,7 @@ fn parse_interval_unit_required() {
     );
 
     let sql = "SELECT INTERVAL '10' HOUR";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))),
@@ -4968,7 +4957,7 @@ fn parse_interval_unit_required() {
     );
 
     let sql = "SELECT INTERVAL 5 DAY";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(number("5"))),
@@ -4980,6 +4969,56 @@ fn parse_interval_unit_required() {
         expr_from_projection(only(&select.projection)),
     );
 
+    let sql = "SELECT INTERVAL '10' HOUR (1)";
+    let select = verified_only_select(sql);
+    assert_eq!(
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))),
+            leading_field: Some(DateTimeField::Hour),
+            leading_precision: Some(1),
+            last_field: None,
+            fractional_seconds_precision: None,
+        }),
+        expr_from_projection(only(&select.projection)),
+    );
+
+    let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
+    assert_eq!(
+        ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
+        result.unwrap_err(),
+    );
+
+    let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)");
+    assert_eq!(
+        ParserError::ParserError("Expected: end of statement, found: (".to_string()),
+        result.unwrap_err(),
+    );
+
+    verified_only_select("SELECT INTERVAL '1' YEAR");
+    verified_only_select("SELECT INTERVAL '1' MONTH");
+    verified_only_select("SELECT INTERVAL '1' DAY");
+    verified_only_select("SELECT INTERVAL '1' HOUR");
+    verified_only_select("SELECT INTERVAL '1' MINUTE");
+    verified_only_select("SELECT INTERVAL '1' SECOND");
+    verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH");
+    verified_only_select("SELECT INTERVAL '1' DAY TO HOUR");
+    verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE");
+    verified_only_select("SELECT INTERVAL '1' DAY TO SECOND");
+    verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE");
+    verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND");
+    verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND");
+    verified_only_select("SELECT INTERVAL 1 YEAR");
+    verified_only_select("SELECT INTERVAL 1 MONTH");
+    verified_only_select("SELECT INTERVAL 1 DAY");
+    verified_only_select("SELECT INTERVAL 1 HOUR");
+    verified_only_select("SELECT INTERVAL 1 MINUTE");
+    verified_only_select("SELECT INTERVAL 1 SECOND");
+}
+
+#[test]
+fn parse_interval_unit_required() {
+    let dialects_unit_required = all_dialects_where(|d| d.require_interval_units());
+
     let sql = "SELECT INTERVAL 1 + 1 DAY";
     let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
@@ -4997,30 +5036,22 @@ fn parse_interval_unit_required() {
         expr_from_projection(only(&select.projection)),
     );
 
-    let sql = "SELECT INTERVAL '10' HOUR (1)";
+    let sql = "SELECT INTERVAL '1' + '1' DAY";
     let select = dialects_unit_required.verified_only_select(sql);
     assert_eq!(
         &Expr::Interval(Interval {
-            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))),
-            leading_field: Some(DateTimeField::Hour),
-            leading_precision: Some(1),
+            value: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
+                op: BinaryOperator::Plus,
+                right: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
+            }),
+            leading_field: Some(DateTimeField::Day),
+            leading_precision: None,
             last_field: None,
             fractional_seconds_precision: None,
         }),
         expr_from_projection(only(&select.projection)),
     );
-
-    let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
-    assert_eq!(
-        ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
-        result.unwrap_err(),
-    );
-
-    let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)");
-    assert_eq!(
-        ParserError::ParserError("Expected: end of statement, found: (".to_string()),
-        result.unwrap_err(),
-    );
 }
 
 #[test]

From fa8e807d6c0ede5dfc892bc81e514012e951f79f Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Fri, 23 Aug 2024 15:34:04 +0100
Subject: [PATCH 05/14] improve tests, more

---
 tests/sqlparser_common.rs | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index a75f9dcb0c..e4d3391a6c 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4902,19 +4902,6 @@ fn parse_interval_all() {
         expr_from_projection(only(&select.projection)),
     );
 
-    let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH";
-    let select = verified_only_select(sql);
-    assert_eq!(
-        &Expr::Interval(Interval {
-            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))),
-            leading_field: Some(DateTimeField::Year),
-            leading_precision: None,
-            last_field: Some(DateTimeField::Month),
-            fractional_seconds_precision: None,
-        }),
-        expr_from_projection(only(&select.projection)),
-    );
-
     let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)";
     let select = verified_only_select(sql);
     assert_eq!(
@@ -5135,7 +5122,8 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let actual_ast = Parser::parse_sql(&PostgreSqlDialect {}, sql).unwrap();
+    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
         with: None,

From 2c14b9599d1df45093eaa8b4d7d654490049ac5d Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 28 Aug 2024 21:23:32 +0100
Subject: [PATCH 06/14] more backwards compatible and fix binary expressions

---
 src/dialect/duckdb.rs       |   2 +-
 src/dialect/mod.rs          |   5 +-
 src/dialect/postgresql.rs   |   2 +-
 src/dialect/redshift.rs     |   2 +-
 src/dialect/snowflake.rs    |   2 +-
 src/parser/mod.rs           |  38 +++---------
 tests/sqlparser_bigquery.rs |  16 +++--
 tests/sqlparser_common.rs   | 115 +++++++++++++++++++++++++++++++-----
 8 files changed, 124 insertions(+), 58 deletions(-)

diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
index e43f407cf6..3160688440 100644
--- a/src/dialect/duckdb.rs
+++ b/src/dialect/duckdb.rs
@@ -56,7 +56,7 @@ impl Dialect for DuckDbDialect {
         true
     }
 
-    fn require_interval_units(&self) -> bool {
+    fn prefer_interval_units(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index b5b532b42a..0d232eac0e 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -517,13 +517,16 @@ pub trait Dialect: Debug + Any {
     /// * `INTERVAL 1 + 1 DAY` is VALID
     /// * `INTERVAL '1' + '1' DAY` is VALID
     /// * `INTERVAL '1'` is INVALID
+    /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but
+    ///  `INTERVAL '2' > INTERVAL '1'` is not!
     ///
     /// When `false`:
     /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL '1'` is VALID
     /// * `INTERVAL '1 second'` is VALID
     /// * `INTERVAL 1 + 1 DAY` is INVALID
-    fn require_interval_units(&self) -> bool {
+    /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly
+    fn prefer_interval_units(&self) -> bool {
         true
     }
 }
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index c68f6b7923..d663570fb3 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -163,7 +163,7 @@ impl Dialect for PostgreSqlDialect {
         true
     }
 
-    fn require_interval_units(&self) -> bool {
+    fn prefer_interval_units(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs
index b6cce26e07..d554d62855 100644
--- a/src/dialect/redshift.rs
+++ b/src/dialect/redshift.rs
@@ -64,7 +64,7 @@ impl Dialect for RedshiftSqlDialect {
         true
     }
 
-    fn require_interval_units(&self) -> bool {
+    fn prefer_interval_units(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index cdb972602c..a712fd0f8c 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -167,7 +167,7 @@ impl Dialect for SnowflakeDialect {
         true
     }
 
-    fn require_interval_units(&self) -> bool {
+    fn prefer_interval_units(&self) -> bool {
         false
     }
 }
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 64b875095f..a898585477 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2044,10 +2044,13 @@ impl<'a> Parser<'a> {
         // don't currently try to parse it. (The sign can instead be included
         // inside the value string.)
 
-        let (value, has_units) = if self.dialect.require_interval_units() {
-            self.parse_interval_expr_units_required()?
+        let value = if self.dialect.prefer_interval_units() {
+            // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid
+            self.parse_expr()?
         } else {
-            self.parse_interval_expr_units_not_require()?
+            // parse a prefix expression so `INTERVAL 1 DAY` is valid, but `INTERVAL 1 + 1 DAY` is not
+            // this also means that `INTERVAL '5 days' > INTERVAL '1 day'` treated properly
+            self.parse_prefix()?
         };
 
         // Following the string literal is a qualifier which indicates the units
@@ -2055,7 +2058,7 @@ impl<'a> Parser<'a> {
         //
         // Note that PostgreSQL allows omitting the qualifier, so we provide
         // this more general implementation.
-        let leading_field = if has_units {
+        let leading_field = if self.next_token_is_unit() {
             Some(self.parse_date_time_field()?)
         } else {
             None
@@ -2095,33 +2098,6 @@ impl<'a> Parser<'a> {
         }))
     }
 
-    /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found
-    ///
-    /// # Returns
-    ///
-    /// A tuple of (interval expression, whether a unit is found)
-    pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> {
-        let mut expr = self.parse_prefix()?;
-
-        loop {
-            if self.next_token_is_unit() {
-                return Ok((expr, true));
-            } else {
-                expr = self.parse_infix(expr, self.dialect.prec_unknown())?;
-            }
-        }
-    }
-
-    /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit
-    ///
-    /// # Returns
-    ///
-    /// A tuple of (interval expression, whether a unit is found)
-    pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> {
-        self.parse_prefix()
-            .map(|expr| (expr, self.next_token_is_unit()))
-    }
-
     pub fn next_token_is_unit(&mut self) -> bool {
         if let Token::Word(word) = self.peek_token().token {
             matches!(
diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs
index ac71c482f9..22d086d8a3 100644
--- a/tests/sqlparser_bigquery.rs
+++ b/tests/sqlparser_bigquery.rs
@@ -830,14 +830,16 @@ fn parse_typed_struct_syntax_bigquery() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1' DAY), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
             values: vec![Expr::Interval(ast::Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
-                leading_field: Some(DateTimeField::Day),
+                value: Box::new(Expr::Value(Value::SingleQuotedString(
+                    "1-2 3 4:5:6.789999".to_string()
+                ))),
+                leading_field: None,
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
@@ -1139,14 +1141,16 @@ fn parse_typed_struct_syntax_bigquery_and_generic() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '2' MONTH), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery_and_generic().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
             values: vec![Expr::Interval(Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))),
-                leading_field: Some(DateTimeField::Month),
+                value: Box::new(Expr::Value(Value::SingleQuotedString(
+                    "1-2 3 4:5:6.789999".to_string()
+                ))),
+                leading_field: None,
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 0a7121dbb7..17f4bc6eac 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4464,9 +4464,9 @@ fn parse_window_functions() {
                avg(bar) OVER (ORDER BY a \
                RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING), \
                sum(bar) OVER (ORDER BY a \
-               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' MONTH FOLLOWING), \
+               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1 MONTH' FOLLOWING), \
                COUNT(*) OVER (ORDER BY a \
-               RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' DAY FOLLOWING), \
+               RANGE BETWEEN INTERVAL '1 DAY' PRECEDING AND INTERVAL '1 DAY' FOLLOWING), \
                max(baz) OVER (ORDER BY a \
                ROWS UNBOUNDED PRECEDING), \
                sum(qux) OVER (ORDER BY a \
@@ -5000,15 +5000,23 @@ fn parse_interval_all() {
     verified_only_select("SELECT INTERVAL 1 HOUR");
     verified_only_select("SELECT INTERVAL 1 MINUTE");
     verified_only_select("SELECT INTERVAL 1 SECOND");
+    // these are allow with both variants for backwards compatibility
+    verified_only_select("SELECT INTERVAL '1 YEAR'");
+    verified_only_select("SELECT INTERVAL '1 MONTH'");
+    verified_only_select("SELECT INTERVAL '1 DAY'");
+    verified_only_select("SELECT INTERVAL '1 HOUR'");
+    verified_only_select("SELECT INTERVAL '1 MINUTE'");
+    verified_only_select("SELECT INTERVAL '1 SECOND'");
 }
 
 #[test]
-fn parse_interval_unit_required() {
-    let dialects_unit_required = all_dialects_where(|d| d.require_interval_units());
+fn parse_interval_unit_preferred() {
+    let dialects_unit_preferred = all_dialects_where(|d| d.prefer_interval_units());
 
     let sql = "SELECT INTERVAL 1 + 1 DAY";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = dialects_unit_preferred.verified_only_select(sql);
     assert_eq!(
+        expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
             value: Box::new(Expr::BinaryOp {
                 left: Box::new(Expr::Value(number("1"))),
@@ -5020,12 +5028,12 @@ fn parse_interval_unit_required() {
             last_field: None,
             fractional_seconds_precision: None,
         }),
-        expr_from_projection(only(&select.projection)),
     );
 
     let sql = "SELECT INTERVAL '1' + '1' DAY";
-    let select = dialects_unit_required.verified_only_select(sql);
+    let select = dialects_unit_preferred.verified_only_select(sql);
     assert_eq!(
+        expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
             value: Box::new(Expr::BinaryOp {
                 left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
@@ -5037,17 +5045,66 @@ fn parse_interval_unit_required() {
             last_field: None,
             fractional_seconds_precision: None,
         }),
+    );
+
+    let sql = "SELECT INTERVAL '1' + '2' - '3' DAY";
+    let select = dialects_unit_preferred.verified_only_select(sql);
+    assert_eq!(
         expr_from_projection(only(&select.projection)),
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::BinaryOp {
+                    left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
+                    op: BinaryOperator::Plus,
+                    right: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))),
+                }),
+                op: BinaryOperator::Minus,
+                right: Box::new(Expr::Value(Value::SingleQuotedString("3".to_string()))),
+            }),
+            leading_field: Some(DateTimeField::Day),
+            leading_precision: None,
+            last_field: None,
+            fractional_seconds_precision: None,
+        }),
+    );
+
+    // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false
+    let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
+    let select = dialects_unit_preferred.verified_only_select(sql);
+    assert_eq!(
+        expr_from_projection(only(&select.projection)),
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                    "1 DAY"
+                )))),
+                op: BinaryOperator::Gt,
+                right: Box::new(Expr::Interval(Interval {
+                    value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                        "1 SECOND"
+                    )))),
+                    leading_field: None,
+                    leading_precision: None,
+                    last_field: None,
+                    fractional_seconds_precision: None,
+                }))
+            }),
+            leading_field: None,
+            leading_precision: None,
+            last_field: None,
+            fractional_seconds_precision: None,
+        }),
     );
 }
 
 #[test]
-fn parse_interval_unit_not_required() {
-    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+fn parse_interval_unit_not_preferred() {
+    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let select = dialect_no_unit.verified_only_select(sql);
     assert_eq!(
+        expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
             value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
                 "1 DAY"
@@ -5057,7 +5114,6 @@ fn parse_interval_unit_not_required() {
             last_field: None,
             fractional_seconds_precision: None,
         }),
-        expr_from_projection(only(&select.projection)),
     );
 
     dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'");
@@ -5066,11 +5122,38 @@ fn parse_interval_unit_not_required() {
         "SELECT INTERVAL '1 YEAR' one_year",
         "SELECT INTERVAL '1 YEAR' AS one_year",
     );
+
+    let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
+    let select = dialect_no_unit.verified_only_select(sql);
+    assert_eq!(
+        expr_from_projection(only(&select.projection)),
+        &Expr::BinaryOp {
+            left: Box::new(Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                    "1 DAY"
+                )))),
+                leading_field: None,
+                leading_precision: None,
+                last_field: None,
+                fractional_seconds_precision: None,
+            })),
+            op: BinaryOperator::Gt,
+            right: Box::new(Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                    "1 SECOND"
+                )))),
+                leading_field: None,
+                leading_precision: None,
+                last_field: None,
+                fractional_seconds_precision: None,
+            }))
+        }
+    );
 }
 
 #[test]
-fn interval_unit_not_required_gt() {
-    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+fn interval_unit_not_preferred_gt() {
+    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
     let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
         expr,
@@ -5094,8 +5177,8 @@ fn interval_unit_not_required_gt() {
 }
 
 #[test]
-fn interval_unit_not_required_double_colon() {
-    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+fn interval_unit_not_preferred_double_colon() {
+    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
     let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT");
     assert_eq!(
         expr,
@@ -5122,7 +5205,7 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
     let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
@@ -5225,7 +5308,7 @@ fn parse_interval_and_or_xor() {
 
     assert_eq!(actual_ast, expected_ast);
 
-    let dialect_no_unit = all_dialects_except(|d| d.require_interval_units());
+    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
     dialect_no_unit.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \

From b6dcbf22e5301f4b3ef75f649cbc396b5d18a2e8 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 28 Aug 2024 21:35:07 +0100
Subject: [PATCH 07/14] remove unnecessary changes

---
 tests/sqlparser_bigquery.rs |  2 +-
 tests/sqlparser_common.rs   | 17 ++++++++++++++++-
 tests/sqlparser_postgres.rs |  5 +----
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs
index 22d086d8a3..57cf9d7fd4 100644
--- a/tests/sqlparser_bigquery.rs
+++ b/tests/sqlparser_bigquery.rs
@@ -1146,7 +1146,7 @@ fn parse_typed_struct_syntax_bigquery_and_generic() {
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
-            values: vec![Expr::Interval(Interval {
+            values: vec![Expr::Interval(ast::Interval {
                 value: Box::new(Expr::Value(Value::SingleQuotedString(
                     "1-2 3 4:5:6.789999".to_string()
                 ))),
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 17f4bc6eac..e762eb2160 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4969,6 +4969,21 @@ fn parse_interval_all() {
         expr_from_projection(only(&select.projection)),
     );
 
+    let sql = "SELECT INTERVAL '1 DAY'";
+    let select = verified_only_select(sql);
+    assert_eq!(
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                "1 DAY"
+            )))),
+            leading_field: None,
+            leading_precision: None,
+            last_field: None,
+            fractional_seconds_precision: None,
+        }),
+        expr_from_projection(only(&select.projection)),
+    );
+
     let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
     assert_eq!(
         ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
@@ -5000,7 +5015,7 @@ fn parse_interval_all() {
     verified_only_select("SELECT INTERVAL 1 HOUR");
     verified_only_select("SELECT INTERVAL 1 MINUTE");
     verified_only_select("SELECT INTERVAL 1 SECOND");
-    // these are allow with both variants for backwards compatibility
+    // these are allowed with both variants for backwards compatibility
     verified_only_select("SELECT INTERVAL '1 YEAR'");
     verified_only_select("SELECT INTERVAL '1 MONTH'");
     verified_only_select("SELECT INTERVAL '1 DAY'");
diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs
index 4ba338dc0f..2f9fe86c9e 100644
--- a/tests/sqlparser_postgres.rs
+++ b/tests/sqlparser_postgres.rs
@@ -4353,10 +4353,7 @@ fn parse_mat_cte() {
 fn parse_at_time_zone() {
     pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE tz");
     pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE ('America/' || 'Los_Angeles')");
-}
 
-#[test]
-fn parse_at_time_zone_interval() {
     // check precedence
     let expr = Expr::BinaryOp {
         left: Box::new(Expr::AtTimeZone {
@@ -4385,7 +4382,7 @@ fn parse_at_time_zone_interval() {
         })),
     };
     pretty_assertions::assert_eq!(
-        pg().verified_expr(
+        pg_and_generic().verified_expr(
             "TIMESTAMP '2001-09-28 01:00' AT TIME ZONE 'America/Los_Angeles'::TEXT + INTERVAL '23 hours'",
         ),
         expr

From 6c2a9717897d7986c655c24bfcbcd7e814fd3d40 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 28 Aug 2024 21:38:20 +0100
Subject: [PATCH 08/14] clippy

---
 src/dialect/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index 0d232eac0e..b30789d7b5 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -518,7 +518,7 @@ pub trait Dialect: Debug + Any {
     /// * `INTERVAL '1' + '1' DAY` is VALID
     /// * `INTERVAL '1'` is INVALID
     /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but
-    ///  `INTERVAL '2' > INTERVAL '1'` is not!
+    /// * `INTERVAL '2' > INTERVAL '1'` is not!
     ///
     /// When `false`:
     /// * `INTERVAL '1' DAY` is VALID

From 522f981b7ae032c1e66d44a0c717f86574b1141d Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 28 Aug 2024 22:19:11 +0100
Subject: [PATCH 09/14] prefer_interval_units -> allow_interval_expressions

---
 src/dialect/duckdb.rs     |  2 +-
 src/dialect/mod.rs        |  8 ++++---
 src/dialect/postgresql.rs |  2 +-
 src/dialect/redshift.rs   |  2 +-
 src/dialect/snowflake.rs  |  2 +-
 src/parser/mod.rs         |  2 +-
 tests/sqlparser_common.rs | 44 +++++++++++++++++++--------------------
 7 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
index 3160688440..3457dafe45 100644
--- a/src/dialect/duckdb.rs
+++ b/src/dialect/duckdb.rs
@@ -56,7 +56,7 @@ impl Dialect for DuckDbDialect {
         true
     }
 
-    fn prefer_interval_units(&self) -> bool {
+    fn allow_interval_expressions(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index b30789d7b5..037d03f423 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -510,13 +510,15 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Whether or not units are required with interval expressions.
+    /// Whether expressions (like `1 + 1`) are allowed in `INTERVAL` expressions.
+    ///
+    /// See <https://github.com/sqlparser-rs/sqlparser-rs/pull/1398> for more information.
     ///
     /// When `true`:
     /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL 1 + 1 DAY` is VALID
     /// * `INTERVAL '1' + '1' DAY` is VALID
-    /// * `INTERVAL '1'` is INVALID
+    /// * `INTERVAL '1'` is VALID
     /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but
     /// * `INTERVAL '2' > INTERVAL '1'` is not!
     ///
@@ -526,7 +528,7 @@ pub trait Dialect: Debug + Any {
     /// * `INTERVAL '1 second'` is VALID
     /// * `INTERVAL 1 + 1 DAY` is INVALID
     /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly
-    fn prefer_interval_units(&self) -> bool {
+    fn allow_interval_expressions(&self) -> bool {
         true
     }
 }
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index d663570fb3..d9026943a3 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -163,7 +163,7 @@ impl Dialect for PostgreSqlDialect {
         true
     }
 
-    fn prefer_interval_units(&self) -> bool {
+    fn allow_interval_expressions(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs
index d554d62855..c0b6371fea 100644
--- a/src/dialect/redshift.rs
+++ b/src/dialect/redshift.rs
@@ -64,7 +64,7 @@ impl Dialect for RedshiftSqlDialect {
         true
     }
 
-    fn prefer_interval_units(&self) -> bool {
+    fn allow_interval_expressions(&self) -> bool {
         false
     }
 }
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index a712fd0f8c..43b9906bc6 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -167,7 +167,7 @@ impl Dialect for SnowflakeDialect {
         true
     }
 
-    fn prefer_interval_units(&self) -> bool {
+    fn allow_interval_expressions(&self) -> bool {
         false
     }
 }
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index a898585477..a4af153956 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2044,7 +2044,7 @@ impl<'a> Parser<'a> {
         // don't currently try to parse it. (The sign can instead be included
         // inside the value string.)
 
-        let value = if self.dialect.prefer_interval_units() {
+        let value = if self.dialect.allow_interval_expressions() {
             // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid
             self.parse_expr()?
         } else {
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index e762eb2160..505e994352 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -5025,11 +5025,11 @@ fn parse_interval_all() {
 }
 
 #[test]
-fn parse_interval_unit_preferred() {
-    let dialects_unit_preferred = all_dialects_where(|d| d.prefer_interval_units());
+fn parse_interval_allow_interval_expr() {
+    let dialects = all_dialects_where(|d| d.allow_interval_expressions());
 
     let sql = "SELECT INTERVAL 1 + 1 DAY";
-    let select = dialects_unit_preferred.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5046,7 +5046,7 @@ fn parse_interval_unit_preferred() {
     );
 
     let sql = "SELECT INTERVAL '1' + '1' DAY";
-    let select = dialects_unit_preferred.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5063,7 +5063,7 @@ fn parse_interval_unit_preferred() {
     );
 
     let sql = "SELECT INTERVAL '1' + '2' - '3' DAY";
-    let select = dialects_unit_preferred.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5085,7 +5085,7 @@ fn parse_interval_unit_preferred() {
 
     // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false
     let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
-    let select = dialects_unit_preferred.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5113,11 +5113,11 @@ fn parse_interval_unit_preferred() {
 }
 
 #[test]
-fn parse_interval_unit_not_preferred() {
-    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
+fn parse_interval_disallow_interval_expr() {
+    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
 
     let sql = "SELECT INTERVAL '1 DAY'";
-    let select = dialect_no_unit.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5131,15 +5131,15 @@ fn parse_interval_unit_not_preferred() {
         }),
     );
 
-    dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'");
-    dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year");
-    dialect_no_unit.one_statement_parses_to(
+    dialects.verified_only_select("SELECT INTERVAL '1 YEAR'");
+    dialects.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year");
+    dialects.one_statement_parses_to(
         "SELECT INTERVAL '1 YEAR' one_year",
         "SELECT INTERVAL '1 YEAR' AS one_year",
     );
 
     let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
-    let select = dialect_no_unit.verified_only_select(sql);
+    let select = dialects.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::BinaryOp {
@@ -5167,9 +5167,9 @@ fn parse_interval_unit_not_preferred() {
 }
 
 #[test]
-fn interval_unit_not_preferred_gt() {
-    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
-    let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x");
+fn interval_disallow_interval_expr_gt() {
+    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let expr = dialects.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
         expr,
         Expr::BinaryOp {
@@ -5192,9 +5192,9 @@ fn interval_unit_not_preferred_gt() {
 }
 
 #[test]
-fn interval_unit_not_preferred_double_colon() {
-    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
-    let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT");
+fn interval_disallow_interval_expr_double_colon() {
+    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT");
     assert_eq!(
         expr,
         Expr::Cast {
@@ -5220,8 +5220,8 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
-    let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap();
+    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let actual_ast = dialects.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
         with: None,
@@ -5323,7 +5323,7 @@ fn parse_interval_and_or_xor() {
 
     assert_eq!(actual_ast, expected_ast);
 
-    let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units());
+    let dialect_no_unit = all_dialects_except(|d| d.allow_interval_expressions());
     dialect_no_unit.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \

From aebf8580444c0c99ca26c7d74c87589180e5a906 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Thu, 29 Aug 2024 09:42:01 +0100
Subject: [PATCH 10/14] make generic more lax by not MySQL etc.

---
 src/dialect/generic.rs      |  4 ++
 src/dialect/mod.rs          | 16 +++++++-
 src/dialect/postgresql.rs   |  4 ++
 src/dialect/redshift.rs     |  4 ++
 src/dialect/snowflake.rs    |  4 ++
 src/parser/mod.rs           |  4 ++
 tests/sqlparser_bigquery.rs | 20 ++++------
 tests/sqlparser_common.rs   | 76 +++++++++++++++++++++----------------
 8 files changed, 87 insertions(+), 45 deletions(-)

diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs
index 211abd9767..0ed1ada0de 100644
--- a/src/dialect/generic.rs
+++ b/src/dialect/generic.rs
@@ -86,4 +86,8 @@ impl Dialect for GenericDialect {
     fn allow_extract_single_quotes(&self) -> bool {
         true
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index 037d03f423..2a12d87000 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -525,12 +525,26 @@ pub trait Dialect: Debug + Any {
     /// When `false`:
     /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL '1'` is VALID
-    /// * `INTERVAL '1 second'` is VALID
     /// * `INTERVAL 1 + 1 DAY` is INVALID
     /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly
     fn allow_interval_expressions(&self) -> bool {
         true
     }
+
+    /// Whether the `INTERVAL` keyword requires units to be specified, e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`.
+    ///
+    /// See <https://github.com/sqlparser-rs/sqlparser-rs/pull/1398> for more information.
+    ///
+    /// When `true`:
+    /// * `INTERVAL '1' DAY` is VALID
+    /// * `INTERVAL '1'` is INVALID
+    ///
+    /// When `false`:
+    /// * `INTERVAL '1' DAY` is VALID
+    /// * `INTERVAL '1'` is VALID
+    fn require_interval_units(&self) -> bool {
+        true
+    }
 }
 
 /// This represents the operators for which precedence must be defined
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index d9026943a3..7276ea16c4 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -166,6 +166,10 @@ impl Dialect for PostgreSqlDialect {
     fn allow_interval_expressions(&self) -> bool {
         false
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
 
 pub fn parse_comment(parser: &mut Parser) -> Result<Statement, ParserError> {
diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs
index c0b6371fea..1ae407f3f6 100644
--- a/src/dialect/redshift.rs
+++ b/src/dialect/redshift.rs
@@ -67,4 +67,8 @@ impl Dialect for RedshiftSqlDialect {
     fn allow_interval_expressions(&self) -> bool {
         false
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index 43b9906bc6..07a5092da8 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -170,6 +170,10 @@ impl Dialect for SnowflakeDialect {
     fn allow_interval_expressions(&self) -> bool {
         false
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
 
 /// Parse snowflake create table statement.
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index a4af153956..2117fc8be9 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2060,6 +2060,10 @@ impl<'a> Parser<'a> {
         // this more general implementation.
         let leading_field = if self.next_token_is_unit() {
             Some(self.parse_date_time_field()?)
+        } else if self.dialect.require_interval_units() {
+            return Err(ParserError::ParserError(
+                "INTERVAL requires a unit like DAY after the literal value".to_string(),
+            ));
         } else {
             None
         };
diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs
index 57cf9d7fd4..b17f1ae78a 100644
--- a/tests/sqlparser_bigquery.rs
+++ b/tests/sqlparser_bigquery.rs
@@ -830,16 +830,14 @@ fn parse_typed_struct_syntax_bigquery() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '2' HOUR), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
-            values: vec![Expr::Interval(ast::Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString(
-                    "1-2 3 4:5:6.789999".to_string()
-                ))),
-                leading_field: None,
+            values: vec![Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))),
+                leading_field: Some(DateTimeField::Hour),
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
@@ -1141,16 +1139,14 @@ fn parse_typed_struct_syntax_bigquery_and_generic() {
         expr_from_projection(&select.projection[3])
     );
 
-    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1-2 3 4:5:6.789999'), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
+    let sql = r#"SELECT STRUCT<INTERVAL>(INTERVAL '1' MONTH), STRUCT<JSON>(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#;
     let select = bigquery_and_generic().verified_only_select(sql);
     assert_eq!(2, select.projection.len());
     assert_eq!(
         &Expr::Struct {
-            values: vec![Expr::Interval(ast::Interval {
-                value: Box::new(Expr::Value(Value::SingleQuotedString(
-                    "1-2 3 4:5:6.789999".to_string()
-                ))),
-                leading_field: None,
+            values: vec![Expr::Interval(Interval {
+                value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))),
+                leading_field: Some(DateTimeField::Month),
                 leading_precision: None,
                 last_field: None,
                 fractional_seconds_precision: None
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 505e994352..844fb2a94e 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4472,7 +4472,8 @@ fn parse_window_functions() {
                sum(qux) OVER (ORDER BY a \
                GROUPS BETWEEN 1 PRECEDING AND 1 FOLLOWING) \
                FROM foo";
-    let select = verified_only_select(sql);
+    let dialects = all_dialects_except(|d| d.require_interval_units());
+    let select = dialects.verified_only_select(sql);
 
     const EXPECTED_PROJ_QTY: usize = 7;
     assert_eq!(EXPECTED_PROJ_QTY, select.projection.len());
@@ -4969,21 +4970,6 @@ fn parse_interval_all() {
         expr_from_projection(only(&select.projection)),
     );
 
-    let sql = "SELECT INTERVAL '1 DAY'";
-    let select = verified_only_select(sql);
-    assert_eq!(
-        &Expr::Interval(Interval {
-            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
-                "1 DAY"
-            )))),
-            leading_field: None,
-            leading_precision: None,
-            last_field: None,
-            fractional_seconds_precision: None,
-        }),
-        expr_from_projection(only(&select.projection)),
-    );
-
     let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND");
     assert_eq!(
         ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()),
@@ -5015,13 +5001,32 @@ fn parse_interval_all() {
     verified_only_select("SELECT INTERVAL 1 HOUR");
     verified_only_select("SELECT INTERVAL 1 MINUTE");
     verified_only_select("SELECT INTERVAL 1 SECOND");
-    // these are allowed with both variants for backwards compatibility
-    verified_only_select("SELECT INTERVAL '1 YEAR'");
-    verified_only_select("SELECT INTERVAL '1 MONTH'");
-    verified_only_select("SELECT INTERVAL '1 DAY'");
-    verified_only_select("SELECT INTERVAL '1 HOUR'");
-    verified_only_select("SELECT INTERVAL '1 MINUTE'");
-    verified_only_select("SELECT INTERVAL '1 SECOND'");
+}
+
+#[test]
+fn parse_interval_dont_require_unit() {
+    let dialects = all_dialects_except(|d| d.require_interval_units());
+
+    let sql = "SELECT INTERVAL '1 DAY'";
+    let select = dialects.verified_only_select(sql);
+    assert_eq!(
+        &Expr::Interval(Interval {
+            value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
+                "1 DAY"
+            )))),
+            leading_field: None,
+            leading_precision: None,
+            last_field: None,
+            fractional_seconds_precision: None,
+        }),
+        expr_from_projection(only(&select.projection)),
+    );
+    dialects.verified_only_select("SELECT INTERVAL '1 YEAR'");
+    dialects.verified_only_select("SELECT INTERVAL '1 MONTH'");
+    dialects.verified_only_select("SELECT INTERVAL '1 DAY'");
+    dialects.verified_only_select("SELECT INTERVAL '1 HOUR'");
+    dialects.verified_only_select("SELECT INTERVAL '1 MINUTE'");
+    dialects.verified_only_select("SELECT INTERVAL '1 SECOND'");
 }
 
 #[test]
@@ -5082,10 +5087,18 @@ fn parse_interval_allow_interval_expr() {
             fractional_seconds_precision: None,
         }),
     );
+}
 
+#[test]
+fn parse_interval_generic() {
     // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false
     let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
-    let select = dialects.verified_only_select(sql);
+
+    let generic = TestedDialects {
+        dialects: vec![Box::new(GenericDialect {}) as Box<dyn Dialect>],
+        options: None,
+    };
+    let select = generic.verified_only_select(sql);
     assert_eq!(
         expr_from_projection(only(&select.projection)),
         &Expr::Interval(Interval {
@@ -5114,7 +5127,7 @@ fn parse_interval_allow_interval_expr() {
 
 #[test]
 fn parse_interval_disallow_interval_expr() {
-    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let select = dialects.verified_only_select(sql);
@@ -5168,7 +5181,7 @@ fn parse_interval_disallow_interval_expr() {
 
 #[test]
 fn interval_disallow_interval_expr_gt() {
-    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let expr = dialects.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
         expr,
@@ -5193,7 +5206,7 @@ fn interval_disallow_interval_expr_gt() {
 
 #[test]
 fn interval_disallow_interval_expr_double_colon() {
-    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT");
     assert_eq!(
         expr,
@@ -5220,7 +5233,7 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let dialects = all_dialects_except(|d| d.allow_interval_expressions());
+    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let actual_ast = dialects.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
@@ -5323,20 +5336,19 @@ fn parse_interval_and_or_xor() {
 
     assert_eq!(actual_ast, expected_ast);
 
-    let dialect_no_unit = all_dialects_except(|d| d.allow_interval_expressions());
-    dialect_no_unit.verified_stmt(
+    dialects.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'",
     );
 
-    dialect_no_unit.verified_stmt(
+    dialects.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         OR d2_date > d1_date + INTERVAL '3 days'",
     );
 
-    dialect_no_unit.verified_stmt(
+    dialects.verified_stmt(
         "SELECT col FROM test \
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         XOR d2_date > d1_date + INTERVAL '3 days'",

From 4e13da3205a2b878fc1437d8571733b8b0ce2eaf Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Thu, 29 Aug 2024 09:47:15 +0100
Subject: [PATCH 11/14] git commit cleanup

---
 src/parser/mod.rs         |  7 ++++---
 tests/sqlparser_common.rs | 24 ++++++++++++++++++++----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 2117fc8be9..5616d01b75 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2061,9 +2061,10 @@ impl<'a> Parser<'a> {
         let leading_field = if self.next_token_is_unit() {
             Some(self.parse_date_time_field()?)
         } else if self.dialect.require_interval_units() {
-            return Err(ParserError::ParserError(
-                "INTERVAL requires a unit like DAY after the literal value".to_string(),
-            ));
+            return parser_err!(
+                "INTERVAL requires a unit after the literal value",
+                self.peek_token().location
+            );
         } else {
             None
         };
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 844fb2a94e..76f2acb468 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -5029,6 +5029,18 @@ fn parse_interval_dont_require_unit() {
     dialects.verified_only_select("SELECT INTERVAL '1 SECOND'");
 }
 
+#[test]
+fn parse_interval_require_unit() {
+    let dialects = all_dialects_where(|d| d.require_interval_units());
+
+    let sql = "SELECT INTERVAL '1 DAY'";
+    let err = dialects.parse_sql_statements(sql).unwrap_err();
+    assert_eq!(
+        err.to_string(),
+        "sql parser error: INTERVAL requires a unit after the literal value"
+    )
+}
+
 #[test]
 fn parse_interval_allow_interval_expr() {
     let dialects = all_dialects_where(|d| d.allow_interval_expressions());
@@ -5127,7 +5139,8 @@ fn parse_interval_generic() {
 
 #[test]
 fn parse_interval_disallow_interval_expr() {
-    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects =
+        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let select = dialects.verified_only_select(sql);
@@ -5181,7 +5194,8 @@ fn parse_interval_disallow_interval_expr() {
 
 #[test]
 fn interval_disallow_interval_expr_gt() {
-    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects =
+        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let expr = dialects.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
         expr,
@@ -5206,7 +5220,8 @@ fn interval_disallow_interval_expr_gt() {
 
 #[test]
 fn interval_disallow_interval_expr_double_colon() {
-    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects =
+        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT");
     assert_eq!(
         expr,
@@ -5233,7 +5248,8 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects =
+        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
     let actual_ast = dialects.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {

From 31a19f77a502d10fda487cffc0c65bfedcf4ffc9 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Thu, 29 Aug 2024 09:55:21 +0100
Subject: [PATCH 12/14] fix duckdb and linting

---
 src/dialect/duckdb.rs       | 4 ++++
 tests/sqlparser_bigquery.rs | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
index 3457dafe45..d8411972fe 100644
--- a/src/dialect/duckdb.rs
+++ b/src/dialect/duckdb.rs
@@ -59,4 +59,8 @@ impl Dialect for DuckDbDialect {
     fn allow_interval_expressions(&self) -> bool {
         false
     }
+
+    fn require_interval_units(&self) -> bool {
+        false
+    }
 }
diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs
index b17f1ae78a..4f84b376d0 100644
--- a/tests/sqlparser_bigquery.rs
+++ b/tests/sqlparser_bigquery.rs
@@ -13,7 +13,6 @@
 #[macro_use]
 mod test_utils;
 
-use sqlparser::ast;
 use std::ops::Deref;
 
 use sqlparser::ast::*;

From 4cc404a3d471fad34e5110dcdc6e70f7f21429d9 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 4 Sep 2024 12:48:49 +0100
Subject: [PATCH 13/14] rename and document next_token_is_temporal_unit

---
 src/parser/mod.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 6281f65c60..3ff6bcc021 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2058,7 +2058,7 @@ impl<'a> Parser<'a> {
         //
         // Note that PostgreSQL allows omitting the qualifier, so we provide
         // this more general implementation.
-        let leading_field = if self.next_token_is_unit() {
+        let leading_field = if self.next_token_is_temporal_unit() {
             Some(self.parse_date_time_field()?)
         } else if self.dialect.require_interval_units() {
             return parser_err!(
@@ -2103,7 +2103,9 @@ impl<'a> Parser<'a> {
         }))
     }
 
-    pub fn next_token_is_unit(&mut self) -> bool {
+    /// Peek at the next token and determine if it is a temporal unit
+    /// like `second`.
+    pub fn next_token_is_temporal_unit(&mut self) -> bool {
         if let Token::Word(word) = self.peek_token().token {
             matches!(
                 word.keyword,

From 297df8eda08e93a9fcfce25071eda9916ef2e697 Mon Sep 17 00:00:00 2001
From: Samuel Colvin <s@muelcolvin.com>
Date: Wed, 4 Sep 2024 15:11:52 +0100
Subject: [PATCH 14/14] simplify syntax choices

---
 src/dialect/ansi.rs       |  4 +++
 src/dialect/bigquery.rs   |  4 +++
 src/dialect/clickhouse.rs |  4 +++
 src/dialect/databricks.rs |  4 +++
 src/dialect/duckdb.rs     |  8 ------
 src/dialect/generic.rs    |  4 ---
 src/dialect/hive.rs       |  4 +++
 src/dialect/mod.rs        | 31 ++++++---------------
 src/dialect/mysql.rs      |  4 +++
 src/dialect/postgresql.rs |  8 ------
 src/dialect/redshift.rs   |  8 ------
 src/dialect/snowflake.rs  |  8 ------
 src/parser/mod.rs         |  7 +++--
 tests/sqlparser_common.rs | 58 ++++++---------------------------------
 14 files changed, 46 insertions(+), 110 deletions(-)

diff --git a/src/dialect/ansi.rs b/src/dialect/ansi.rs
index d07bc07eb2..61ae5829ef 100644
--- a/src/dialect/ansi.rs
+++ b/src/dialect/ansi.rs
@@ -24,4 +24,8 @@ impl Dialect for AnsiDialect {
     fn is_identifier_part(&self, ch: char) -> bool {
         ch.is_ascii_lowercase() || ch.is_ascii_uppercase() || ch.is_ascii_digit() || ch == '_'
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/bigquery.rs b/src/dialect/bigquery.rs
index d3673337f7..3bce6702b0 100644
--- a/src/dialect/bigquery.rs
+++ b/src/dialect/bigquery.rs
@@ -63,4 +63,8 @@ impl Dialect for BigQueryDialect {
     fn supports_select_wildcard_except(&self) -> bool {
         true
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/clickhouse.rs b/src/dialect/clickhouse.rs
index 34940467a5..09735cbeea 100644
--- a/src/dialect/clickhouse.rs
+++ b/src/dialect/clickhouse.rs
@@ -37,4 +37,8 @@ impl Dialect for ClickHouseDialect {
     fn describe_requires_table_keyword(&self) -> bool {
         true
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/databricks.rs b/src/dialect/databricks.rs
index 42d432d3d6..d3661444b7 100644
--- a/src/dialect/databricks.rs
+++ b/src/dialect/databricks.rs
@@ -38,4 +38,8 @@ impl Dialect for DatabricksDialect {
     fn supports_select_wildcard_except(&self) -> bool {
         true
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
index d8411972fe..1fc211685a 100644
--- a/src/dialect/duckdb.rs
+++ b/src/dialect/duckdb.rs
@@ -55,12 +55,4 @@ impl Dialect for DuckDbDialect {
     fn support_map_literal_syntax(&self) -> bool {
         true
     }
-
-    fn allow_interval_expressions(&self) -> bool {
-        false
-    }
-
-    fn require_interval_units(&self) -> bool {
-        false
-    }
 }
diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs
index a120323f17..c8f1c00d9a 100644
--- a/src/dialect/generic.rs
+++ b/src/dialect/generic.rs
@@ -90,8 +90,4 @@ impl Dialect for GenericDialect {
     fn supports_create_index_with_clause(&self) -> bool {
         true
     }
-
-    fn require_interval_units(&self) -> bool {
-        false
-    }
 }
diff --git a/src/dialect/hive.rs b/src/dialect/hive.rs
index 2340df611b..b32d44cb9a 100644
--- a/src/dialect/hive.rs
+++ b/src/dialect/hive.rs
@@ -42,4 +42,8 @@ impl Dialect for HiveDialect {
     fn supports_numeric_prefix(&self) -> bool {
         true
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index 3206a0db88..0be8c17c76 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -516,7 +516,10 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Whether expressions (like `1 + 1`) are allowed in `INTERVAL` expressions.
+    /// Whether `INTERVAL` expressions require units (called "qualifiers" in the ANSI SQL spec) to be specified,
+    /// e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`.
+    ///
+    /// Expressions within intervals (e.g. `INTERVAL '1' + '1' DAY`) are only allowed when units are required.
     ///
     /// See <https://github.com/sqlparser-rs/sqlparser-rs/pull/1398> for more information.
     ///
@@ -524,32 +527,14 @@ pub trait Dialect: Debug + Any {
     /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL 1 + 1 DAY` is VALID
     /// * `INTERVAL '1' + '1' DAY` is VALID
-    /// * `INTERVAL '1'` is VALID
-    /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but
-    /// * `INTERVAL '2' > INTERVAL '1'` is not!
-    ///
-    /// When `false`:
-    /// * `INTERVAL '1' DAY` is VALID
-    /// * `INTERVAL '1'` is VALID
-    /// * `INTERVAL 1 + 1 DAY` is INVALID
-    /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly
-    fn allow_interval_expressions(&self) -> bool {
-        true
-    }
-
-    /// Whether the `INTERVAL` keyword requires units to be specified, e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`.
-    ///
-    /// See <https://github.com/sqlparser-rs/sqlparser-rs/pull/1398> for more information.
-    ///
-    /// When `true`:
-    /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL '1'` is INVALID
     ///
     /// When `false`:
-    /// * `INTERVAL '1' DAY` is VALID
     /// * `INTERVAL '1'` is VALID
-    fn require_interval_units(&self) -> bool {
-        true
+    /// * `INTERVAL '1' DAY` is VALID — unit is not required, but still allowed
+    /// * `INTERVAL 1 + 1 DAY` is INVALID
+    fn require_interval_qualifier(&self) -> bool {
+        false
     }
 }
 
diff --git a/src/dialect/mysql.rs b/src/dialect/mysql.rs
index 32525658b9..b8c4631fd0 100644
--- a/src/dialect/mysql.rs
+++ b/src/dialect/mysql.rs
@@ -84,6 +84,10 @@ impl Dialect for MySqlDialect {
             None
         }
     }
+
+    fn require_interval_qualifier(&self) -> bool {
+        true
+    }
 }
 
 /// `LOCK TABLES`
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index 759c52b289..eba3a69896 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -166,14 +166,6 @@ impl Dialect for PostgreSqlDialect {
     fn supports_create_index_with_clause(&self) -> bool {
         true
     }
-
-    fn allow_interval_expressions(&self) -> bool {
-        false
-    }
-
-    fn require_interval_units(&self) -> bool {
-        false
-    }
 }
 
 pub fn parse_comment(parser: &mut Parser) -> Result<Statement, ParserError> {
diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs
index 1ae407f3f6..bd4dc817ce 100644
--- a/src/dialect/redshift.rs
+++ b/src/dialect/redshift.rs
@@ -63,12 +63,4 @@ impl Dialect for RedshiftSqlDialect {
     fn supports_connect_by(&self) -> bool {
         true
     }
-
-    fn allow_interval_expressions(&self) -> bool {
-        false
-    }
-
-    fn require_interval_units(&self) -> bool {
-        false
-    }
 }
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index 07a5092da8..4f37004b1c 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -166,14 +166,6 @@ impl Dialect for SnowflakeDialect {
     fn allow_extract_single_quotes(&self) -> bool {
         true
     }
-
-    fn allow_interval_expressions(&self) -> bool {
-        false
-    }
-
-    fn require_interval_units(&self) -> bool {
-        false
-    }
 }
 
 /// Parse snowflake create table statement.
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 3ff6bcc021..b7d44d2f20 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -2044,7 +2044,10 @@ impl<'a> Parser<'a> {
         // don't currently try to parse it. (The sign can instead be included
         // inside the value string.)
 
-        let value = if self.dialect.allow_interval_expressions() {
+        // to match the different flavours of INTERVAL syntax, we only allow expressions
+        // if the dialect requires an interval qualifier,
+        // see https://github.com/sqlparser-rs/sqlparser-rs/pull/1398 for more details
+        let value = if self.dialect.require_interval_qualifier() {
             // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid
             self.parse_expr()?
         } else {
@@ -2060,7 +2063,7 @@ impl<'a> Parser<'a> {
         // this more general implementation.
         let leading_field = if self.next_token_is_temporal_unit() {
             Some(self.parse_date_time_field()?)
-        } else if self.dialect.require_interval_units() {
+        } else if self.dialect.require_interval_qualifier() {
             return parser_err!(
                 "INTERVAL requires a unit after the literal value",
                 self.peek_token().location
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 5a5131824a..0ed677db95 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -4472,7 +4472,7 @@ fn parse_window_functions() {
                sum(qux) OVER (ORDER BY a \
                GROUPS BETWEEN 1 PRECEDING AND 1 FOLLOWING) \
                FROM foo";
-    let dialects = all_dialects_except(|d| d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
     let select = dialects.verified_only_select(sql);
 
     const EXPECTED_PROJ_QTY: usize = 7;
@@ -5005,7 +5005,7 @@ fn parse_interval_all() {
 
 #[test]
 fn parse_interval_dont_require_unit() {
-    let dialects = all_dialects_except(|d| d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let select = dialects.verified_only_select(sql);
@@ -5031,7 +5031,7 @@ fn parse_interval_dont_require_unit() {
 
 #[test]
 fn parse_interval_require_unit() {
-    let dialects = all_dialects_where(|d| d.require_interval_units());
+    let dialects = all_dialects_where(|d| d.require_interval_qualifier());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let err = dialects.parse_sql_statements(sql).unwrap_err();
@@ -5042,8 +5042,8 @@ fn parse_interval_require_unit() {
 }
 
 #[test]
-fn parse_interval_allow_interval_expr() {
-    let dialects = all_dialects_where(|d| d.allow_interval_expressions());
+fn parse_interval_require_qualifier() {
+    let dialects = all_dialects_where(|d| d.require_interval_qualifier());
 
     let sql = "SELECT INTERVAL 1 + 1 DAY";
     let select = dialects.verified_only_select(sql);
@@ -5101,46 +5101,9 @@ fn parse_interval_allow_interval_expr() {
     );
 }
 
-#[test]
-fn parse_interval_generic() {
-    // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false
-    let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'";
-
-    let generic = TestedDialects {
-        dialects: vec![Box::new(GenericDialect {}) as Box<dyn Dialect>],
-        options: None,
-    };
-    let select = generic.verified_only_select(sql);
-    assert_eq!(
-        expr_from_projection(only(&select.projection)),
-        &Expr::Interval(Interval {
-            value: Box::new(Expr::BinaryOp {
-                left: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
-                    "1 DAY"
-                )))),
-                op: BinaryOperator::Gt,
-                right: Box::new(Expr::Interval(Interval {
-                    value: Box::new(Expr::Value(Value::SingleQuotedString(String::from(
-                        "1 SECOND"
-                    )))),
-                    leading_field: None,
-                    leading_precision: None,
-                    last_field: None,
-                    fractional_seconds_precision: None,
-                }))
-            }),
-            leading_field: None,
-            leading_precision: None,
-            last_field: None,
-            fractional_seconds_precision: None,
-        }),
-    );
-}
-
 #[test]
 fn parse_interval_disallow_interval_expr() {
-    let dialects =
-        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
 
     let sql = "SELECT INTERVAL '1 DAY'";
     let select = dialects.verified_only_select(sql);
@@ -5194,8 +5157,7 @@ fn parse_interval_disallow_interval_expr() {
 
 #[test]
 fn interval_disallow_interval_expr_gt() {
-    let dialects =
-        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
     let expr = dialects.verified_expr("INTERVAL '1 second' > x");
     assert_eq!(
         expr,
@@ -5220,8 +5182,7 @@ fn interval_disallow_interval_expr_gt() {
 
 #[test]
 fn interval_disallow_interval_expr_double_colon() {
-    let dialects =
-        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
     let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT");
     assert_eq!(
         expr,
@@ -5248,8 +5209,7 @@ fn parse_interval_and_or_xor() {
         WHERE d3_date > d1_date + INTERVAL '5 days' \
         AND d2_date > d1_date + INTERVAL '3 days'";
 
-    let dialects =
-        all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units());
+    let dialects = all_dialects_except(|d| d.require_interval_qualifier());
     let actual_ast = dialects.parse_sql_statements(sql).unwrap();
 
     let expected_ast = vec![Statement::Query(Box::new(Query {
