[tonyalaribe]

The rollup matcher only recognised time_bucket('1 hour', timestamp). The time_bucket(INTERVAL '1 hour', timestamp) spelling declined as unsupported_shape and scanned raw rows. On prod, a cold 30d shipbubble hourly chart took 62s, against 2.8s on the repeat. Monoscope's own widgets use the string spelling; hand-written SQL hits this.

• Accept an IntervalMonthDayNano literal, using time_bucket's own interval_to_micros. Month widths are still refused.
• The string spelling's rewrite is unchanged.

Regression test an_interval_bucket_width_routes_like_its_string_spelling fails with the fix disabled (red arm checked).

Local signoff (make ci-signoff, rebased on 5877d77): fmt, clippy, test, pg-smoke and e2e all pass and are attested. Wall-clock timing tests (checkpoint_does_not_rescan…, a_deletion_vector_disables_the_ordinals) flaked under host load ~300 and passed on retry; they are unrelated to this change.

🤖 Generated with Claude Code

[monoscope-bot]

Monoscope production impact · Worth checking

Project 87576849-4941-49d3-a15d-680fef88a1a8 · reviewed 64ae14202745c1a6a2cb5e99d4bdfa3b0ce18c3e · advisory

Evidence window (UTC): 2026-10-19 08:39:54.190011094 UTC → 2026-10-26 08:39:54.190011094 UTC

src/rollup.rs:2366 (Head)

Production details are available in Monoscope.

telemetry evidence

Coverage: Open Monoscope for evidence and coverage details.

Review history and settings

[claude]

Review: Route time_bucket(INTERVAL ...) charts to rollups

Small, well-scoped change. It reuses interval_to_micros, keeps month widths refused, and leaves the string path untouched. The PR says the regression test was checked red with the fix disabled.

Minor points, none blocking:

1. Seconds truncation. format!("{} seconds", width / 1_000_000) truncates sub-second remainders. Today the width % grain check should reject those unless a grain is under 1s. Please confirm grains are always whole seconds. If so, a debug_assert!(width % 1_000_000 == 0) would document the invariant.
2. Rewritten spelling. An INTERVAL '1 hour' query is rewritten as time_bucket('3600 seconds', timestamp), not '1 hour'. Confirm that parse_bucket_micros and anything that compares the rewritten expression to the user's GROUP BY or projection accept this spelling. A cheap improvement is to assert in the test that the INTERVAL route's rewritten SQL or result equals the string route's.
3. Test cost. Per CLAUDE.md, tests should assert cost as well as correctness. is_some() on both arms covers routing. A check that both spellings pick the same spec or rollup would be stronger.
4. Style. The nested .ok().map(...) / and_then match is dense. Splitting it into a small helper such as bucket_width(expr) -> Option<(i64, String)> would read better. Optional.
5. Other literals. IntervalDayTime and IntervalYearMonth literals are not handled and will decline as unsupported_shape. That is probably fine, since the parser yields MonthDayNano for INTERVAL '...'.

LGTM otherwise.