fix: INTERVAL n WEEK resolves to seven days instead of one hour

[barry3406]

Fixes #18665.

Description

INTERVAL '1' WEEK (and the unquoted INTERVAL 1 WEEK) was resolving to one hour instead of seven days. The repro from #18665 makes the symptom obvious:

SELECT
  MILLIS_TO_TIMESTAMP(0) + INTERVAL 1 WEEK,
  MILLIS_TO_TIMESTAMP(0) + INTERVAL 2 WEEK

was returning 1970-01-01T01:00:00, 1970-01-01T02:00:00 (one and two hours) instead of the expected 1970-01-08, 1970-01-15.

Per @gianm's review, INTERVAL n QUARTER is broken in Calcite 1.37.0 in the same way, so this PR also covers QUARTER.

Root cause

The bugs live in Calcite 1.37.0:

• SqlIntervalQualifier.evaluateIntervalLiteralAsWeek packages a WEEK literal into a year-month shaped int array ([sign, 0, week]), even though SqlIntervalQualifier.typeName() reports a WEEK qualifier as INTERVAL_DAY. The downstream SqlParserUtil.intervalToMillis loop walks ret[1..] as [day, hour, minute, second, ms], so the week count lands in the hour slot. INTERVAL '1' WEEK therefore becomes 1 * 3_600_000 ms = 1 hour.
• SqlIntervalQualifier.evaluateIntervalLiteralAsQuarter packages a QUARTER literal into the same year-month shaped array ([sign, 0, quarter]) without multiplying by three. SqlParserUtil.intervalToMonths reads 12 * 0 + 1 * quarter = quarter months instead of 3 * quarter months. INTERVAL '1' QUARTER therefore becomes one month.

Both bugs were fixed upstream as CALCITE-6581 in Calcite 1.38. Druid is still on Calcite 1.37, so we work around them inside the planner instead of upgrading Calcite in this PR.

Fixed the bug ...

Added SqlIntervalWeekRewriteShuttle, an SqlShuttle that walks the parsed SqlNode tree before validation and rewrites the affected parsed forms into equivalent intervals in a unit Calcite 1.37 handles correctly:

• INTERVAL '1' WEEK parses to a SqlIntervalLiteral with a WEEK qualifier. The shuttle replaces it with a SqlIntervalLiteral whose value has been multiplied by seven and whose qualifier is now DAY.
• INTERVAL 1 WEEK parses to SqlStdOperatorTable.INTERVAL.createCall(pos, n, qualifier). The shuttle rewrites the call so the numeric operand becomes MULTIPLY(n, 7) and the qualifier becomes DAY.
• INTERVAL '1' QUARTER and INTERVAL 1 QUARTER are rewritten the same way, using MONTH and a factor of three.

WEEK(SUNDAY)..WEEK(SATURDAY) and ISOWEEK qualifiers carry a non-null timeFrameName and are handled differently in Calcite, so the shuttle leaves them alone and only touches the bare WEEK / QUARTER forms that are broken in 1.37.

The shuttle is invoked in DruidPlanner.validate right before rewriteParameters, alongside the existing SqlParameterizerShuttle pass. It runs once per validate call and is a no-op for any tree that does not contain a WEEK or QUARTER interval.

This shuttle is a temporary workaround and should be removed when Druid upgrades to Calcite 1.38+.

Tests

• SqlIntervalWeekRewriteShuttleTest exercises the shuttle directly against constructed SqlIntervalLiteral and SqlCall nodes for WEEK, QUARTER, DAY and MONTH (positive, negative, multi-unit), and against parsed queries that use both quoted and unquoted week and quarter intervals.
• CalciteQueryTest#testIntervalWeekResolution is the end-to-end regression test from INTERVAL 1 WEEK in SQL is wrong interval #18665. Without the fix it produces an interval narrowing on 2000-01-01T01:00:00.001 / 2000-01-01T02:00:00.001 (the bug); with the fix it correctly narrows to 2000-01-08 and 2000-01-15.
• CalciteQueryTest#testIntervalQuarterResolution is the companion regression for QUARTER: INTERVAL '1' QUARTER narrows to 2000-04-01 (three months later) and INTERVAL 2 QUARTER to 2000-07-01.

I verified the bug is reachable by temporarily disabling the shuttle and watching the regression tests fail with exactly the actual-vs-expected from the issue. Re-enabling the shuttle makes them pass.

I also re-ran the surrounding test classes to make sure the rewrite did not regress any existing INTERVAL behavior:

• CalciteQueryTest (441 tests)
• CalciteSelectQueryTest (67 tests)
• CalciteJoinQueryTest (594 tests)
• CalciteSubqueryTest (54 tests)
• CalciteParameterQueryTest (20 tests)
• CalciteInsertDmlTest / CalciteReplaceDmlTest (107 tests)
• ExpressionsTest (54 tests)
• DruidSqlParserUtilsTest (57 tests)
• SqlQuidemTest (10 tests)

All green. mvn -pl sql -am verify -DskipTests=true (checkstyle, forbiddenapis, animal-sniffer) and mvn -pl sql -P rat verify -DskipTests=true (license headers) also pass.

Release note

Fixed an issue where INTERVAL n WEEK in SQL queries resolved to n hours instead of n * 7 days, and INTERVAL n QUARTER resolved to n months instead of n * 3 months. MILLIS_TO_TIMESTAMP(0) + INTERVAL '1' WEEK now correctly returns 1970-01-08, and INTERVAL '1' QUARTER adds three months, matching the SQL standard and other databases.

---

Key changed/added classes in this PR

• SqlIntervalWeekRewriteShuttle (new, covers WEEK and QUARTER)
• DruidPlanner (calls the shuttle in validate)
• SqlIntervalWeekRewriteShuttleTest (new)
• CalciteQueryTest (regression tests testIntervalWeekResolution and testIntervalQuarterResolution)

---

This PR has:

• been self-reviewed.
• added unit tests or modified existing tests to cover new code paths.
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• a release note entry in the PR description.

[Copilot]

Pull request overview

Fixes a Calcite 1.37.0 interval bug where INTERVAL n WEEK is interpreted as n hours by rewriting plain WEEK intervals to equivalent DAY intervals (multiplying the value by 7) before Calcite validation in Druid’s planner.

Changes:

• Add SqlIntervalWeekRewriteShuttle to rewrite bare WEEK interval literals/calls into DAY intervals.
• Invoke the shuttle during DruidPlanner.validate() prior to parameter rewriting/validation.
• Add unit tests for the shuttle and an end-to-end regression test for issue #18665.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlIntervalWeekRewriteShuttle.java	Introduces the AST rewrite workaround for Calcite’s broken plain WEEK interval handling.
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java	Runs the new shuttle during validation so Calcite never sees the buggy WEEK interval representation.
sql/src/test/java/org/apache/druid/sql/calcite/planner/SqlIntervalWeekRewriteShuttleTest.java	Adds direct unit coverage for quoted/unquoted week interval rewrites and parser-level checks.
sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java	Adds an end-to-end regression test demonstrating correct interval narrowing for WEEK.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SqlLiteral.toValue() for createExactNumeric returns a numeric type (typically BigDecimal), not a String. This assertion will fail at runtime; compare against BigDecimal.valueOf(7) (or use getValueAs(BigDecimal.class) / toValue().toString()).

[gianm]

Did you run into some problem while upgrading Calcite? It would be better to fix the bug that way, rather than through a workaround.

Also, the bug was fixed in https://issues.apache.org/jira/browse/CALCITE-6581. I know because I wrote the patch 🙂. At the time I figured we would get the fix the next time we upgraded Calcite.

[barry3406]

Good point — I didn't attempt the Calcite upgrade because it felt too big for a bug-fix PR (1.37 → 1.38 tends to pull in a lot of changes). This shuttle is removable once the upgrade lands.

Also thanks for the correction, the actual fix is CALCITE-6581 not CALCITE-6391 — I'll update the references. If you'd rather just bump Calcite instead I'm fine closing this.

[gianm]

Certainly I'd prefer upgrading Calcite if it's straightforward. That's why I was wondering if you tried and ran into any issues. It can a be a lot of work (see #14510, #16504) but I was hoping it wouldn't be too bad this time.

If upgrading Calcite isn't straightforward, we could merge your fix here temporarily, then remove it once we do upgrade Calcite. Btw, if we are going to do the temporary fix, please include QUARTER too since that is also broken in Calcite 1.37.0.

[barry3406]

Added QUARTER support in 09b6250 — mirrors the WEEK rewrite (n MONTH * 3) with the same test coverage (quoted/unquoted, positive/negative, multi-unit, parser end-to-end, plus a testIntervalQuarterResolution regression in CalciteQueryTest that proves INTERVAL 2 QUARTER narrows to 2000-07-01). Also corrected the CALCITE-6581 reference throughout the code and comments. Haven't attempted the 1.38 bump — happy to keep this as the temp workaround per your suggestion, and equally happy to drop it when someone gets to the upgrade.

[gianm]

I raised a PR for the Calcite update here: #19370. That should also fix this issue. The PR adds a test sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/interval_arithmetic.iq verifying it.

[FrankChen021]

The changes LGTM, no correctness issues found.