[paveltiunov]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Issue Reference this PR resolves

Adds a failing test for #11474. It does not fix the bug.

Description of Changes Made

RedshiftQuery.subtractInterval builds DATEADD(${datePart}, -${intervalValue}, ...). When the value is already negative, the output is DATEADD(day, --1, ...). Redshift reads -- as the start of a line comment, so any custom granularity with a negative offset (for example offset: -1 day, as in the sunday_week recipe) produces invalid SQL.

Reproduced end to end on cubejs/cube:v1.8.18 with CUBEJS_DB_TYPE=redshift, on both Tesseract and the legacy planner: /v1/sql contains DATEADD(day, --1, ...) and /v1/load fails with syntax error at or near "FROM".

New test packages/cubejs-schema-compiler/test/unit/redshift-query.test.ts:

• calls subtractInterval('t.ts', '-1 day') directly and expects DATEADD(day, 1, t.ts);
• builds a full query with a sunday_week granularity (offset: -1 day) and expects no -- in the SQL.

Both cases currently fail with CUBEJS_TESSERACT_SQL_PLANNER=true and false.

---

Generated by Claude Code

[claude]

Claude finished @paveltiunov's task in 1m 3s —— View job

---

Changes requested: 1 high, 1 low. The tests are committed while they fail on purpose, so merging them turns master's unit job red.

Review details

• Read diff and RedshiftQuery.ts
• Checked which planners are affected
• Ran the new test: skipped, node_modules isn't installed in this checkout
• Review threads
• Posted review

Findings

• High redshift-query.test.ts:6: both tests are plain it and fail by design. Merging makes yarn unit red on master. Use it.failing (Jest 29) or ship the one-line fix in src/adapter/RedshiftQuery.ts:26.
• Low redshift-query.test.ts:12 and :57: the exact-string DATEADD(day, 1, ...) checks lock the test to one fix. not.toContain('--') is the real invariant.

Notes

• The diagnosis is correct and applies to both planners. Tesseract calls the JS subtractInterval through driver tools (rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs:243), so one fix covers both.
• A unit test rather than an integration test is reasonable here. This is interval arithmetic, not a sqlTemplates() render, and Redshift has no Docker runner.
• No docs needed: only a test is added.
• No security or performance concerns.

Review threads

The listing returned 0 of my threads (no prior threads), so none were resolved or re-affirmed. Both findings were posted as new threads.

[claude]

High: if this PR is merged as is, yarn unit on master goes red. Both tests are plain it and fail by design, since RedshiftQuery.subtractInterval (src/adapter/RedshiftQuery.ts:26) still emits -${intervalValue}. Every later PR touching schema-compiler then inherits a red unit job.

Jest 29 has it.failing. It records the bug as a passing test and turns red once the fix lands, which tells the fixer to flip it back to it:

Suggested change

	it('subtractInterval with a negative interval does not emit a SQL line comment', () => {
	it.failing('subtractInterval with a negative interval does not emit a SQL line comment', () => {

Use the same at line 42. The other option is to ship the fix in this PR. It's a one-line change: negate the parsed value (-1 * Number(intervalValue)) or wrap it as -(${intervalValue}). Do the same in any other dialect that builds -${...} from a parsed interval.

Both planners are affected. Tesseract calls back into the JS subtractInterval through driver tools (rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs:243).

[claude]

Low: toEqual('DATEADD(day, 1, t.ts)') here and toContain('DATEADD(day, 1, ') at line 57 lock the test to one fix. A fix that emits DATEADD(day, -(-1), t.ts) is equally valid on Redshift and still fails this test. The invariant that matters is not.toContain('--'). To also pin the direction of the shift, check the evaluated result (offset -1 day should move the bucket start one day earlier) rather than the exact SQL text.