Spark, Databricks: allow an interval string without a unit by moshap-firebolt · Pull Request #2614 · apache/datafusion-sqlparser-rs

moshap-firebolt · 2026-10-05T19:57:39Z

Spark's interval literal may be a string that carries its own units, INTERVAL '2 months', which SparkSqlDialect and DatabricksDialect reject with INTERVAL requires a unit after the literal value.

This adds Dialect::supports_interval_string_without_qualifier, enabled for both. The value must then be a literal: a number or a string, optionally signed. Only an unsigned string may omit the unit, so INTERVAL 1 and INTERVAL 1 + 1 DAY still fail, as in Spark. Double-quoted strings are string literals in Spark, so INTERVAL "2 months" and INTERVAL "1" DAY are intervals too.

INTERVAL is non-reserved in Spark, so both dialects now read it as a column name when no literal follows: SELECT interval FROM t, interval - 1, INTERVAL -'2 months'. A string window frame bound (RANGE BETWEEN '1 day' PRECEDING) stays a string literal, as in Spark.

Spark's multi-units interval syntax lets the string carry its own units, as in INTERVAL '1 YEAR 2 DAYS 3 HOURS', so no unit follows the literal. Databricks Runtime parses with the same grammar. A unit after a literal is still read; the expression form this gives up (INTERVAL 1 + 1 DAY) is not Spark syntax.

codecov-commenter · 2026-10-05T20:44:26Z

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.22%. Comparing base (14cbf75) to head (6693400).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dialect/spark.rs	83.33%	0 Missing and 1 partial ⚠️
src/parser/mod.rs	95.45%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2614      +/-   ##
==========================================
+ Coverage   81.14%   81.22%   +0.07%     
==========================================
  Files          42       42              
  Lines       33736    33936     +200     
  Branches    33736    33936     +200     
==========================================
+ Hits        27376    27564     +188     
+ Misses       2797     2795       -2     
- Partials     3563     3577      +14

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

LucaCappelletti94

You could gate unqualified intervals on string literals through a dialect capability, retaining qualifiers for numeric literals and excluding identifiers from literal values.

Setting require_interval_qualifier() to false makes both dialects accept SELECT INTERVAL 1, which Spark 4.0.1 rejects, and misparse SELECT interval FROM t into SELECT INTERVAL FROM AS t, losing the table reference.

I added some red tests to help out.

LucaCappelletti94 · 2026-10-06T10:19:56Z

+        Expr::Interval(i) => assert!(i.leading_field.is_none()),
+        other => panic!("Expected an interval, got {other:?}"),
+    }
+}


Suggested change

}

}

#[test]

fn test_interval_numeric_requires_unit() {

assert_eq!(

spark()

.parse_sql_statements("SELECT INTERVAL 1")

.unwrap_err()

.to_string(),

"sql parser error: INTERVAL requires a unit after the literal value"

);

}

#[test]

fn test_interval_rejects_value_arithmetic() {

assert!(spark()

.parse_sql_statements("SELECT INTERVAL 1 + 1 DAY")

.is_err());

}

#[test]

fn test_interval_preserves_column_query() {

let query = spark()

.run_parser_method("SELECT interval FROM t", |parser| parser.parse_query())

.unwrap();

let SetExpr::Select(select) = *query.body else {

panic!("Expected SELECT");

};

assert_eq!(

select.from,

vec![TableWithJoins {

relation: table("t"),

joins: vec![],

}]

);

assert_eq!(

select.projection,

vec![SelectItem::UnnamedExpr(Expr::Identifier(Ident::new(

"interval"

)))]

);

}

LucaCappelletti94 · 2026-10-06T10:21:32Z

+        Expr::Interval(i) => assert!(i.leading_field.is_none()),
+        other => panic!("Expected an interval, got {other:?}"),
+    }
+}


Suggested change

}

}

#[test]

fn test_interval_numeric_requires_unit() {

assert_eq!(

databricks()

.parse_sql_statements("SELECT INTERVAL 1")

.unwrap_err()

.to_string(),

"sql parser error: INTERVAL requires a unit after the literal value"

);

}

#[test]

fn test_interval_rejects_value_arithmetic() {

assert!(databricks()

.parse_sql_statements("SELECT INTERVAL 1 + 1 DAY")

.is_err());

}

#[test]

fn test_interval_preserves_column_query() {

let query = databricks()

.run_parser_method("SELECT interval FROM t", |parser| parser.parse_query())

.unwrap();

let SetExpr::Select(select) = *query.body else {

panic!("Expected SELECT");

};

assert_eq!(

select.from,

vec![TableWithJoins {

relation: table("t"),

joins: vec![],

}]

);

assert_eq!(

select.projection,

vec![SelectItem::UnnamedExpr(Expr::Identifier(Ident::new(

"interval"

)))]

);

}

Address review: turning require_interval_qualifier off accepted INTERVAL 1 and read SELECT interval FROM t as SELECT INTERVAL FROM AS t. Keep it on and add supports_interval_string_without_qualifier: the value must be a literal, and only an unsigned string may omit the qualifier. INTERVAL is not reserved for these dialects, so SELECT interval FROM t reads a column.

moshap-firebolt · 2026-10-08T00:30:35Z

You could gate unqualified intervals on string literals through a dialect capability, retaining qualifiers for numeric literals and excluding identifiers from literal values.

Setting require_interval_qualifier() to false makes both dialects accept SELECT INTERVAL 1, which Spark 4.0.1 rejects, and misparse SELECT interval FROM t into SELECT INTERVAL FROM AS t, losing the table reference.

Right, so kept require_interval_qualifier on true and added supports_interval_string_without_qualifier enabled for Spark/Databricks; also unreserved INTERVAL for both dialects similar to PG

I added some red tests to help out.

Thanks - that was super helpful 🙏

LucaCappelletti94 · 2026-10-12T18:13:54Z

+                        let interval_number = w.keyword == Keyword::INTERVAL
+                            && match &self.peek_token_ref().token {
+                                Token::Number(..) => true,
+                                Token::Plus | Token::Minus => {
+                                    matches!(self.peek_nth_token_ref(1).token, Token::Number(..))
+                                }
+                                _ => false,
+                            };


You should keep only the Token::Number arm. Spark 4.0.1 parses SELECT interval - 1 FROM t, SELECT interval + 1 FROM t and SELECT INTERVAL -1 as arithmetic on the column interval, but the sign arm rejects all three while interval * 2 parses. INTERVAL is non-reserved in Spark, so the grammar reads it as a column reference inside unary and binary arithmetic. Without the arm the identifier fallback parses them the way Spark does, and SELECT INTERVAL 1 keeps its error.

Suggested change

let interval_number = w.keyword == Keyword::INTERVAL

&& match &self.peek_token_ref().token {

Token::Number(..) => true,

Token::Plus | Token::Minus => {

matches!(self.peek_nth_token_ref(1).token, Token::Number(..))

}

_ => false,

};

let interval_number = w.keyword == Keyword::INTERVAL

&& matches!(self.peek_token_ref().token, Token::Number(..));

LucaCappelletti94 · 2026-10-12T18:14:53Z

+            !matches!(
+                &value,
+                Expr::Value(ValueWithSpan {
+                    value: Value::SingleQuotedString(_),


You should let a double-quoted string omit the qualifier too, since Spark's stringLit includes DOUBLEQUOTED_STRING and INTERVAL stringLit takes no unit. Both the Spark and Databricks docs allow "..." as a string literal, and Spark 4.0.1 parses SELECT INTERVAL "2 months" as INTERVAL '2' MONTH.

Suggested change

value: Value::SingleQuotedString(_),

value: Value::SingleQuotedString(_) | Value::DoubleQuotedString(_),

LucaCappelletti94 · 2026-10-12T18:15:26Z

+            Token::SingleQuotedString(s) => {
+                Expr::value(Value::SingleQuotedString(s).with_span(next.span))
+            }


You should accept Token::DoubleQuotedString here, since as aforementioned Spark's intervalValue takes any stringLit. SELECT INTERVAL "1" DAY parses on main and in Spark 4.0.1 but fails on this branch. SELECT INTERVAL "2 months" now parses as SELECT INTERVAL AS "2 months", a column INTERVAL aliased "2 months".

Suggested change

Token::SingleQuotedString(s) => {

Expr::value(Value::SingleQuotedString(s).with_span(next.span))

}

Token::SingleQuotedString(s) => {

Expr::value(Value::SingleQuotedString(s).with_span(next.span))

}

Token::DoubleQuotedString(s) => {

Expr::value(Value::DoubleQuotedString(s).with_span(next.span))

}

LucaCappelletti94 · 2026-10-12T18:17:49Z

+        "SELECT INTERVAL -'1' DAY",
+        "SELECT INTERVAL 3 DAY",
+        "SELECT INTERVAL -1 DAY, INTERVAL +2 HOURS",
+        "SELECT INTERVAL '2 seconds' * 2",


You could cover the double-quoted forms. Both lines fail on this branch and pass with the two changes above.

Suggested change

"SELECT INTERVAL '2 seconds' * 2",

"SELECT INTERVAL '2 seconds' * 2",

"SELECT INTERVAL \"2 months\"",

"SELECT INTERVAL \"1\" DAY",

LucaCappelletti94 · 2026-10-12T18:18:12Z

+        );
+    }
+    dialects.verified_stmt("SELECT interval, x FROM t");
+    dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");


You could cover signed arithmetic on the column. The new line fails on this branch and passes with the guard change above.

Suggested change

dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");

dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");

dialects.verified_stmt("SELECT interval - 1, interval + 1 FROM t");

Added. Also RANGE BETWEEN '1 day' PRECEDING now remains string literal (added test)

…ter interval is arithmetic Address review: a double-quoted string is a Spark string literal, so it may be an interval value and may omit the qualifier (INTERVAL "2 months", INTERVAL "1" DAY). Only a bare number or a string after INTERVAL keeps the interval error; a signed number falls back to the identifier, so interval - 1 and INTERVAL -1 are arithmetic on the column, as in Spark. A string window frame bound stays a string literal in these dialects (RANGE BETWEEN '1 day' PRECEDING), as in Spark, rather than being read as an interval without a qualifier.

LucaCappelletti94 · 2026-10-15T11:50:44Z

+            .parse_sql_statements("SELECT INTERVAL '1' DAY TO")
+            .unwrap_err()
+            .to_string(),
+        "sql parser error: Expected: date/time field, found: EOF"


You should assert the error on SELECT INTERVAL '1' DAY TO x instead. Spark 4.0.1 accepts SELECT INTERVAL '1' DAY TO as INTERVAL '1' DAY AS TO, so this assertion pins a rejection of valid Spark SQL. TO x is a syntax error in Spark 4.0.1 too and still fails if the string arms of the INTERVAL guard are removed.

Suggested change

.parse_sql_statements("SELECT INTERVAL '1' DAY TO")

.unwrap_err()

.to_string(),

"sql parser error: Expected: date/time field, found: EOF"

.parse_sql_statements("SELECT INTERVAL '1' DAY TO x")

.unwrap_err()

.to_string(),

"sql parser error: Expected: date/time field, found: x"

LucaCappelletti94 · 2026-10-15T11:51:16Z

+#[test]
+fn parse_interval_number_without_unit_error() {
+    for dialect in all_dialects().dialects {
+        if let Err(e) = Parser::parse_sql(&*dialect, "SELECT INTERVAL 1") {
+            assert_eq!(
+                e.to_string(),
+                "sql parser error: INTERVAL requires a unit after the literal value",
+                "{dialect:?}"
+            );
+        }
+    }
+}
+


You should drop this test. It skips every dialect that accepts SELECT INTERVAL 1, and for Spark and Databricks test_interval_numeric_requires_unit already asserts the same message.

Suggested change

#[test]

fn parse_interval_number_without_unit_error() {

for dialect in all_dialects().dialects {

if let Err(e) = Parser::parse_sql(&*dialect, "SELECT INTERVAL 1") {

assert_eq!(

e.to_string(),

"sql parser error: INTERVAL requires a unit after the literal value",

"{dialect:?}"

);

}

}

}

LucaCappelletti94 · 2026-10-15T11:51:36Z

+#[test]
+fn parse_interval_expression_value() {
+    let dialects = all_dialects_where(|d| {
+        d.require_interval_qualifier() && !d.supports_interval_string_without_qualifier()
+    });
+    dialects.verified_stmt("SELECT INTERVAL 1 + 1 DAY");
+}


You should drop this test, since parse_interval_require_qualifier right below already runs verified_only_select on SELECT INTERVAL 1 + 1 DAY over the same dialects.

Suggested change

#[test]

fn parse_interval_expression_value() {

let dialects = all_dialects_where(|d| {

d.require_interval_qualifier() && !d.supports_interval_string_without_qualifier()

});

dialects.verified_stmt("SELECT INTERVAL 1 + 1 DAY");

}

Address review: Spark accepts INTERVAL '1' DAY TO as an alias, so assert the error on TO x instead. parse_interval_number_without_unit_error and parse_interval_expression_value repeat test_interval_numeric_requires_unit and parse_interval_require_qualifier. The guard comment now covers strings as well as numbers.

github-actions Bot added Databricks Spark labels Oct 5, 2026

LucaCappelletti94 requested changes Oct 6, 2026

View reviewed changes

moshap-firebolt mentioned this pull request Oct 6, 2026

Spark, Databricks: parse multi-unit interval literals #2616

Draft

LucaCappelletti94 requested changes Oct 12, 2026

View reviewed changes

LucaCappelletti94 added waiting on contributor The review needs further refinements by its author enhancement New feature or request labels Oct 12, 2026

moshap-firebolt mentioned this pull request Oct 14, 2026

Spark, Databricks: port upstream interval PRs #2614 and #2616 firebolt-analytics/datafusion-sqlparser-rs#19

Open

LucaCappelletti94 requested changes Oct 15, 2026

View reviewed changes

-}
+}
+#[test]
+fn test_interval_numeric_requires_unit() {
+    assert_eq!(
+        spark()
+            .parse_sql_statements("SELECT INTERVAL 1")
+            .unwrap_err()
+            .to_string(),
+        "sql parser error: INTERVAL requires a unit after the literal value"
+    );
+}
+#[test]
+fn test_interval_rejects_value_arithmetic() {
+    assert!(spark()
+        .parse_sql_statements("SELECT INTERVAL 1 + 1 DAY")
+        .is_err());
+}
+#[test]
+fn test_interval_preserves_column_query() {
+    let query = spark()
+        .run_parser_method("SELECT interval FROM t", |parser| parser.parse_query())
+        .unwrap();
+    let SetExpr::Select(select) = *query.body else {
+        panic!("Expected SELECT");
+    };
+    assert_eq!(
+        select.from,
+        vec![TableWithJoins {
+            relation: table("t"),
+            joins: vec![],
+        }]
+    );
+    assert_eq!(
+        select.projection,
+        vec![SelectItem::UnnamedExpr(Expr::Identifier(Ident::new(
+            "interval"
+        )))]
+    );
+}

	value: Value::SingleQuotedString(_),
	value: Value::SingleQuotedString(_) \| Value::DoubleQuotedString(_),

	dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");
	dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");
	dialects.verified_stmt("SELECT interval - 1, interval + 1 FROM t");

Conversation

moshap-firebolt commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LucaCappelletti94 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moshap-firebolt commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

moshap-firebolt commented Oct 5, 2026 •

edited

Loading

codecov-commenter commented Oct 5, 2026 •

edited

Loading