Skip to content

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

Draft
moshap-firebolt wants to merge 7 commits into
apache:mainfrom
firebolt-analytics:moshap/upstream-interval-multi-units
Draft

moshap-firebolt wants to merge 7 commits into
apache:mainfrom
firebolt-analytics:moshap/upstream-interval-multi-units

Conversation

@moshap-firebolt

@moshap-firebolt moshap-firebolt commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #2614, whose commits this branch includes; only the last two commits are new. It needs #2614 because the string form below has no unit, which Spark and Databricks reject until #2614 lands. I will rebase onto main once #2614 merges.

Spark's multi-units interval syntax lists several value / unit pairs, INTERVAL 10 YEAR 20 MONTH, documented as equivalent to INTERVAL '10 YEAR 20 MONTH'.

This adds Dialect::supports_interval_multi_units, enabled for SparkSqlDialect and DatabricksDialect. When set, parse_interval reads the extra pairs into that string form, so the AST is unchanged. Values may be numbers or single- or double-quoted strings, optionally signed. A single unit, or a unit followed by something else (INTERVAL 3 DAY + 1), parses as before. Tests in sqlparser_common.rs cover dialects with and without the hook.

@moshap-firebolt
moshap-firebolt marked this pull request as draft October 6, 2026 21:34
@moshap-firebolt
moshap-firebolt force-pushed the moshap/upstream-interval-multi-units branch from e3732a5 to f696fd3 Compare October 6, 2026 21:35
@codecov-commenter

codecov-commenter commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.98%. Comparing base (ce12f1f) to head (5855e37).

Files with missing lines Patch % Lines
src/parser/mod.rs 92.15% 1 Missing and 3 partials ⚠️
src/dialect/spark.rs 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2616      +/-   ##
==========================================
+ Coverage   82.96%   82.98%   +0.02%     
==========================================
  Files          42       42              
  Lines       36148    36221      +73     
  Branches    36148    36221      +73     
==========================================
+ Hits        29989    30059      +70     
  Misses       2657     2657              
- Partials     3502     3505       +3     

☔ 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.

@moshap-firebolt
moshap-firebolt force-pushed the moshap/upstream-interval-multi-units branch 2 times, most recently from 0e240d3 to 3a4d8c3 Compare October 14, 2026 01:42
@moshap-firebolt
moshap-firebolt force-pushed the moshap/upstream-interval-multi-units branch from c665a51 to f559398 Compare October 16, 2026 16:41
moshap-firebolt added a commit to firebolt-analytics/datafusion-sqlparser-rs that referenced this pull request Oct 22, 2026
…2616 (#19)

* Revert "Spark, Databricks: read multi-units interval literals (#17)"

This reverts commit bae1cb2.

* Revert "Spark, Databricks: accept an interval string without a unit (#16)"

This reverts commit ee51174.

* Spark, Databricks: allow an interval string without a unit

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.

* Spark, Databricks: keep the qualifier for numbers; a string may omit it

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.

* Spark, Databricks: double-quoted interval strings; a signed number after 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.

* Spark, Databricks: assert TO x, not TO; drop two duplicate tests

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.

* Spark, Databricks: parse multi-unit interval literals

Spark's multi-units interval syntax lists several value / unit pairs,
INTERVAL 10 YEAR 20 MONTH, documented as the same as the string form
INTERVAL '10 YEAR 20 MONTH'. A new supports_interval_multi_units hook,
enabled for SparkSqlDialect and DatabricksDialect, reads the extra pairs
into that string form, so the AST is unchanged.

* Spark, Databricks: double-quoted values in multi-unit intervals

A double-quoted string is a Spark string literal, so it may be any value
of a multi-units interval, e.g. INTERVAL "1" DAY "2" HOUR.

* Spark, Databricks: assert the error text in test_interval_rejects_value_arithmetic

clippy's assertions_on_result_states rejects assert!(... .is_err()).
Compare the message instead, as test_interval_numeric_requires_unit does.
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.
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.
…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.
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.
…ue_arithmetic

clippy's assertions_on_result_states rejects assert!(... .is_err()).
Compare the message instead, as test_interval_numeric_requires_unit does.
Spark's multi-units interval syntax lists several value / unit pairs,
INTERVAL 10 YEAR 20 MONTH, documented as the same as the string form
INTERVAL '10 YEAR 20 MONTH'. A new supports_interval_multi_units hook,
enabled for SparkSqlDialect and DatabricksDialect, reads the extra pairs
into that string form, so the AST is unchanged.
A double-quoted string is a Spark string literal, so it may be any value
of a multi-units interval, e.g. INTERVAL "1" DAY "2" HOUR.
@moshap-firebolt
moshap-firebolt force-pushed the moshap/upstream-interval-multi-units branch from f559398 to 5855e37 Compare November 2, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants