fix(tesseract): honour custom granularity origin for single-unit intervals

[igorlukanin]

Problem

A custom granularity that combines a single-unit interval with an origin was ignoring the origin entirely:

granularities:
  - name: fiscal_year_starting_on_april_01
    interval: 1 year
    origin: "2024-04-01"

This rendered as DATE_TRUNC('year', …) — calendar years, not fiscal ones. It is the example from the granularities reference docs, which state that "when origin is provided, time intervals will be shifted in a way that one of them will match the provided origin".

The date-range filter is skewed by the same cause, so a "this year" query returns calendar-year bounds rather than fiscal-year ones. It also surfaces with a time_shift measure, where the shifted and unshifted legs end up joined on incompatible buckets.

Cause

Granularity::try_new_custom decided natural alignment from the interval string alone, on the line right after resolving the origin:

let is_natural_aligned = granularity_interval.is_trivial();

is_trivial() is true for any 1 <unit> interval, so alignment was true regardless of origin. apply_to_input_sql then takes the aligned branch, which only knows how to handle granularity_offset — and since origin and offset are mutually exclusive, the offset is empty and it falls through to DATE_TRUNC. The origin-aware DATE_BIN branch was unreachable for every single-unit interval.

Every existing test using origin happened to use a non-trivial interval (5 minutes, 6 months), which routes to DATE_BIN and passes; the fiscal_year fixture uses offset, exercising the working sub-branch. Nothing combined a trivial interval with origin.

Fix

Natural alignment is a property of the interval and its origin, not the interval alone. is_natural_aligned now also requires that the origin sit on the natural boundary of the interval's unit, via a new QueryDateTime::is_start_of.

Everything currently correct stays on DATE_TRUNC — predefined granularities, 1 <unit> with a default origin, and 1 <unit> + offset. Only the genuinely misaligned origin case moves to DATE_BIN.

Two related corrections came out of reviewing that change:

• is_start_of judges the local wall clock, so a midnight that a DST gap pushes forward to 01:00 still counts as the start of its day. Otherwise a plainly calendar-aligned grain would silently drop off the DATE_TRUNC path for particular timezone/year combinations (e.g. America/Asuncion, 2023-10-01).
• origin now discards offset when both are set, matching the JS planner. Previously the offset was kept and shifted the rendered buckets away from the origin that align_date_to_origin and the materialized time series both bin on.

Backward compatibility

Generated SQL changes for models using a single-unit interval together with origin. Those queries currently return calendar-aligned buckets; they will now return the origin-aligned buckets the model asks for. This is a wrong-results fix rather than a behaviour change — the previous output contradicts the documented meaning of origin.

The legacy JS planner has the same defect and is deliberately left alone here, since Tesseract is the default. Note it remains reachable for pre-aggregation-related queries, which route through newQueryWithoutNative().

Tests

• Unit tests over the alignment decision: off-boundary vs on-boundary origins, judged against the interval's own unit; time components; ISO-Monday weeks; the DST-gap case; default/offset origins unchanged; non-trivial intervals.
• Integration tests asserting the rendered SQL — a fiscal-year grain must not emit DATE_TRUNC('year', an on-boundary origin still must, and the time_shift combination from the report.

Each new bug test was confirmed to fail without the fix.

[claude]

Claude finished @igorlukanin's task in 6m 12s —— View job

---

Review complete — diagnosis and fix look right; 1 high, 2 medium, 2 low on the widened DATE_BIN blast radius

Full review

• Gather PR diff and context
• Review date_time.rs (is_start_of)
• Review granularity.rs alignment logic
• Review tests & fixtures
• Cross-check the JS planner and the dateBin dialect implementations
• Post review

Verdict

The root-cause analysis is correct and the fix is minimal and in the right place: is_trivial() alone can't decide natural alignment, and is_natural_aligned = is_trivial() && (offset.is_some() || origin.is_start_of(unit)) narrows the change to exactly the misaligned-origin case. I confirmed the two side corrections independently:

• origin discarding offset matches the JS planner — Granularity.ts:52-61 only assigns granularityOffset in the else if (customGranularity.offset) branch, so JS already drops it. It also makes the Rust side self-consistent: physical_plan/time_series.rs:69 passes granularity_offset() into generated_time_series_select, which knows nothing about origin, so keeping the offset really would have shifted the DB-side series away from the buckets align_date_to_origin produces.
• Predefined / default-origin / offset paths are untouched — try_new_predefined hardcodes is_natural_aligned: true, default_origin() is a year start (on-boundary for every unit), and the offset case is short-circuited before is_start_of runs. The unit tests pin all three.

The concerns below are all about the destination of the newly rerouted cases rather than the routing decision itself: dateBin is considerably less uniform across dialects than the DATE_TRUNC path it replaces.

Findings

#	Severity	Issue
1	High	1 quarter + off-boundary origin emits INTERVAL '1 quarter', invalid on Postgres — wrong results become a query failure
2	Medium	The try_execute_pg snapshots for the three new tests aren't committed, and Postgres dateBin is epoch-approximated for year/month intervals — the headline fiscal-year example is unverified end to end and looks off-by-a-day
3	Medium	Dialects with no dateBin now hard-fail (below)
4	Low	Unreachable error arm in is_start_of
5	Low	is_start_of duplicates QueryDateTime::granularity(), which disagrees with it on DST-gap origins

#3, on backward compatibility. The PR body frames the change as wrong-results → right-results, but for some data sources it is wrong-results → error. BaseQuery.dateBin() throws "Date bin function, required for custom time dimension granularities, is not implemented for this data source", and only Postgres, Redshift, BigQuery, Snowflake, MySQL, MSSQL, ClickHouse, Oracle, Presto (→ Trino, Athena), CubeStore, Databricks and DuckDB override it. VerticaQuery, HiveQuery, SqliteQuery and MongoBiQuery extend BaseQuery without an override, so a model with interval: 1 year + origin that compiles today will start throwing there. Worth naming explicitly in the backward-compatibility section / release notes, since the failure is at query time rather than at model validation.

Where dateBin is implemented, correctness for year/month intervals splits by strategy: Snowflake, ClickHouse, BigQuery and MySQL divide in calendar units (DATEDIFF/DATETIME_DIFF/TIMESTAMPDIFF), so they're exact; Redshift explicitly normalizes year/quarter/month to total months; Postgres (and CrateQuery, which extends it) divides epoch seconds, which is the drift in #2. That Redshift override is essentially a note-to-self that the base form doesn't handle YM intervals — this PR makes that gap the default path for the documented origin example.

Smaller notes

• (dt.month() - 1).is_multiple_of(3) needs Rust ≥ 1.87; rust/cube/rust-toolchain.toml pins 1.90, so fine. No underflow risk since month() ≥ 1.
• is_start_of's dt.date() == start_of_day.date() guard degrades safely for zones that skip a whole calendar day (Pacific/Apia, 2011-12-30): start_of_day lands on the next date, the check returns false, and the grain falls to DATE_BIN rather than silently mistruncating.
• is_start_of covers every value SqlInterval::min_granularity() can return (second…year), so the other => arm is genuinely defensive rather than reachable.
• The comments explaining why each branch is what it is — particularly the "offset origin is exempt" rationale — are unusually good, and the test names read as specifications. explicit_origin_discards_the_offset asserting both the offset and the resulting alignment is exactly the right shape.
• Suggested extra coverage, matching the gaps above: 1 month and 1 quarter with off-boundary origins (each hits a different dialect limitation), and a case where origin is on-boundary while offset is set, to pin that origin wins.

What I couldn't verify

cargo test and git fetch weren't permitted in this environment, so I reviewed HEAD directly rather than a fetched origin/master diff, and I did not execute the new unit tests — the EXTRACT(EPOCH …) arithmetic in #2 is derived from the emitted SQL, not from a live Postgres run. Adding Bash(cargo test:*) to --allowedTools would let me confirm it next time.

• branch `igor/cub-3367-custom-granularity-ignored-with-time-shift-measure`

[claude]

The try_execute_pg half of these tests was never exercised — and it's the half that would catch the remaining problem.

No .snap files are committed for test_fiscal_year_by_origin_does_not_truncate_to_calendar_year, test_calendar_year_by_origin_still_truncates, or test_fiscal_year_by_origin_with_time_shift_measure, while the other tests in this file have them (e.g. …custom_granularities__fiscal_year_with_offset.snap). Under --features integration-postgres these three will fail on missing snapshots; without the feature try_execute_pg returns None and the assertion is skipped entirely. So the only thing verified is "the string date_trunc('year' is absent".

That matters because the DATE_BIN branch this now routes to is epoch-approximated on Postgres:

('2024-04-01'::timestamp + INTERVAL '1 year' *
  FLOOR(EXTRACT(EPOCH FROM (src - '2024-04-01'::timestamp)) / EXTRACT(EPOCH FROM INTERVAL '1 year')))

EXTRACT(EPOCH FROM INTERVAL '1 year') is 31 557 600 (365.25 d). A row on 2025-04-01 is 365 days past the origin → FLOOR(31536000 / 31557600) = 0 → it lands in the 2024-04-01 bucket, i.e. the fiscal-year boundary is off by a day, and it drifts differently each year within the leap cycle. 1 month is worse: epoch of INTERVAL '1 month' is 30 days, so origin 2025-01-15 puts 2025-03-15 (59 days) into the February bucket.

Meanwhile the planner's own bucketing — align_date_to_origin → QueryDateTime::add_interval — is calendar-accurate, so the SQL buckets and the materialized/generated time series can disagree at exactly these boundaries.

Running these tests against Postgres and committing the snapshots would either confirm the fiscal-year buckets are right or surface the drift; right now the PR's headline example isn't verified end to end.

[igorlukanin]

You're right, and it's worse than the snapshots being absent. Working it through against the CI run that did execute these (unit-core on b921f35, which failed only on the missing .snap files):

The fixture rows bucket correctly — 2024-04-01 | 8, 2025-04-01 | 4 — because none of them lands on a fiscal-year boundary. Add a row dated exactly 2025-04-01 and it files into the 2024-04-01 bucket: EXTRACT(EPOCH FROM INTERVAL '1 year') is 31 557 600 (365.25 d), the span from 2024-04-01 to 2025-04-01 is 365 d = 31 536 000 s, so FLOOR gives 0. For origin 2024-04-01 that's wrong on the +1, +2, +3, +5, +6 year boundaries and right only on +4.

1 year is the only unit that misbehaves here, which is why this wasn't already visible: INTERVAL '1 month' (30 d) and '6 months' (180 d) both under-count real calendar time, so FLOOR never overshoots and the existing 6 months + origin grains are stable. '1 year' is the one that over-counts — and it's exactly the unit this PR newly routes onto date_bin.

Meanwhile align_date_to_origin bins with calendar-accurate add_interval, so the SQL buckets and the generated time series disagree at precisely those boundaries. Snowflake is fine (DATEDIFF(year, …) is calendar-exact) and Redshift already overrides dateBin for this reason; it's Postgres and Crate that are exposed.

Fixing it properly means giving PostgresQuery.dateBin the month-arithmetic treatment RedshiftQuery already has, which is a change to shared JS SQL generation for every Postgres-family dialect — outside this PR's Tesseract-only scope and with its own backward-compat surface for the existing 6 months grains. Escalating rather than folding it in; not committing the snapshots in the meantime, since they'd bless output that's wrong one row over.

[claude]

NaiveDate::and_hms_opt(0, 0, 0) never returns None — midnight is representable for every NaiveDate — so this error arm is unreachable. dt.date().and_time(NaiveTime::MIN) gives the same value without the Result plumbing, which also lets the fallible part of the function be just from_local_date_time.

[igorlukanin]

Agreed, and_hms_opt(0, 0, 0) can't fail. Leaving it for the same push that deals with the date_bin boundary problem above rather than force-pushing a cosmetic change while that's still open.

[claude]

This match is a near-duplicate of QueryDateTime::granularity() (lines 185-210) — same "is this instant on a boundary" question, but that one has no DST tolerance and no quarter arm, and it cascades instead of answering per unit.

The two now co-decide the same grain: for the Asuncion-style origin from your test, is_start_of("month") is true (so apply_to_input_sql emits DATE_TRUNC('month', …)), while Granularity::min_granularity() → origin.granularity() returns "hour" because local time is 01:00 — which feeds pre-aggregation matching. That inconsistency predates this PR, but the new method makes it easy to remove: expressing granularity() as a descending scan over is_start_of would keep the two from drifting and pick up the DST handling for free.

[igorlukanin]

Fair, and the inconsistency you name is real: is_start_of("month") accepts the Asuncion DST-gap origin while granularity() reports hour for the same instant, and that one feeds pre-aggregation matching. Worth collapsing granularity() into a descending scan over is_start_of, but it changes pre-agg matching behaviour, so not as a rider on this fix.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.94%. Comparing base (d8d009b) to head (b921f35).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11432      +/-   ##
==========================================
- Coverage   83.95%   83.94%   -0.01%     
==========================================
  Files         257      257              
  Lines       80887    80887              
==========================================
- Hits        67908    67903       -5     
- Misses      12979    12984       +5     

Flag	Coverage Δ
cubesql	83.94% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.