fix(tesseract): bound a rolling window's base scan by literals

[waralexrom]

Summary

A rolling window's base scan read both ends of the span it restricts the fact table to back off the time_series CTE with a scalar sub-select. No engine can eliminate partitions by a sub-select, so every base scan read the whole table — and on a table declared with a mandatory partition filter the query is rejected outright rather than merely running slowly. The legacy planner emitted literals here, so this is a Tesseract regression.

This derives the span while planning and renders it as literal parameters, for regular (trailing/leading) and to_date windows alike, and fixes three defects found alongside it.

Part of #11770. Replaces #11824, which can be closed with its branch: the other half it carried — sharing one base scan between rolling measures — is now #11852, done as a pass over the logical plan after pre-aggregations are matched.

Changes

Literal base-scan bounds. The series follows from the time dimension's granularity and date range, so its span is known at plan time. QueryTimeSeries gained covering_bounds_predefined, which derives the span per bucket rather than by walking the series, and both rolling filters render the bounds as parameters. The span runs one interval past the range end on purpose: a series materialized while planning snaps its points to bucket boundaries, while one generated in SQL steps from the range start, and the span has to cover both. It is exact wherever the range's ends are already bucket boundaries, and a wider span changes no result — the rolling join applies the exact frame on top.

A custom granularity's bounds align to its origin the way the series' own range does, rather than through the step-capped helper the series walks with, which a fine interval and a far-off default origin trip on a range that converges perfectly well.

The sub-select remains where the span is not derivable: a date range that is itself a query, a granularity whose periods come off a calendar cube, and the placeholder ranges of pre-aggregation SQL.

FILTER_PARAMS under a rolling window. A callback column was handed the filter's raw values — the reported period — rather than the band the stage reads, so a cube narrowing its own scan that way was cut to the reporting period while the window summed a wider one. A month-to-date measure over a range opening mid-month answered the day of the range instead of the day of the month. The callback now receives the band itself; where a bound is not derivable it states nothing and renders as always-true, leaving the scan wider than needed rather than narrower than correct.

Timezone of the literal bounds. They were allocated through the path a plain date-range filter uses, which carries a date into the database's timezone — right for a filter comparing an unconverted column, an offset away from a rolling filter, which converts the member into the query's timezone and whose series places its points there. Under America/Los_Angeles a range of 2024-01-10..2024-01-12 gave the scan 2024-01-10T08:00:00 against a series opening at 2024-01-10T00:00:00, so the first eight hours of every window went unread. Legacy does the same split — allocateParam(timeSeries[0][0]) raw against convertedToTz().

A shift binding on a rolling base scan. A FILTER_PARAMS binding addressing a time shift is checked against the operator the query filters the member with, and the check named the plain date range only. A shifted stage can equally be a rolling window's base scan, whose filter carries the same two bounds, so such a model was refused outright — addressing the shift is exactly what the binding is told to do.

Testing

• cargo test -p cubesqlplanner --features integration-postgres — 1395 pass against a real Postgres.
• Each fix was confirmed to fail with itself reverted; the failing output is the test that pins it.
• New unit tests pin the covering bounds against a walked series for every predefined granularity and against an unaligned SQL-generated series.
• Snapshots record values the pushdown must not change: they are what the same queries answer with the binding removed, i.e. while the cube scans unrestricted.
• Two tests cover ground the suite could not see before: every existing test runs at UTC, and none mixed a window bounded by interval math with one counting off a calendar.

Risks

• The base scan reads up to one granularity interval more than the series strictly needs. Negligible at day granularity; at month granularity over a 12-month range about 8% more rows, traded for partition elimination that was entirely absent.
• A to_date window counting its period off a calendar cube still gets no literal bound — when a fiscal month begins is a row of the calendar. Such a query keeps the plan it has today. A FILTER_PARAMS binding passing the fact's own column restates the stage's predicate against it, which is exact and needs no promise from the model, but its bounds stay sub-selects and so do not prune. Whether BigQuery's require_partition_filter accepts that shape is untested here — the suite runs on Postgres, which has no such requirement.
• The remaining half of Rolling-window measures generate an unrunnable plan: time-series join has no equality predicate #11770 is untouched: the rolling CTE still joins time_series to its base CTE on a date range only, with no equality on the group-by dimension. rolling_window::fanout_repro::test_rolling_join_restricts_by_dimension stays ignored to mark it.

🤖 Generated with Claude Code

[claude]

Claude finished @waralexrom's task in 8m 5s —— View job

---

Review complete — no blocking issues; 1 medium, 6 low (details)

Full review — 7 inline comments posted, verdict and reasoning

• Gather context (PR diff, linked issue Rolling-window measures generate an unrunnable plan: time-series join has no equality predicate #11770)
• Review time-series covering-bounds derivation
• Review filter rendering (physical/planner typed_filter, rolling operators)
• Review multi-stage planner changes
• Review tests & snapshots
• Check prior review threads (none existed), post findings

Verdict

The core change is sound and well targeted. I traced the covering-bounds derivation against both series shapes and could not break it:

• Upper bound. range_end + interval − 1s covers the materialized series (last bucket_start ≤ range_end, so bucket_end ≤ range_end + interval − 1s) and the SQL-stepped one (last point < range_end, same ceiling). Exact when the range end is already a bucket end, wider otherwise — and the rolling join applies the real frame on top, so wider changes no result.
• Lower bound. predefined_bucket(range_start).start_str is the series' own first edge for the snapped shape, and ≤ the SQL shape's first point. to_date then floors that to the period start, which is ≤ the period start of any actual anchor.
• Precision. format_to_date special-cases a .999 tail into .999999 at precision 6 (date_time_helper.rs:133), so the upper bound doesn't silently lose 999 µs on a microsecond dialect. Good that this already existed.
• Timezone. date_range_literals deliberately skips apply_db_time_zone while RegularRollingWindowOp/ToDateRollingWindowOp compare convert_tz(member) — consistent. The FILTER_PARAMS band goes the other way (format_and_allocate_*_no_cast, which does apply db tz) and is spliced against an unconverted cube column — also consistent. The split is correct in both directions.
• Shared series. add_time_series reuses one time_series CTE keyed on alias only, and all rolling measures in a query derive their time dimension from the same query_properties.time_dimensions(), so the derived bounds cannot drift from the series they describe. Registering a calendar period column on a shared series doesn't reshape its rows — and a_window_sharing_a_calendar_series_keeps_its_rows pins exactly that, which was the one risk I went looking for.
• Bail-outs. Query-shaped date range → date_range_vec() is None; calendar granularity → calendar_sql().is_some(); pre-aggregation placeholders → parse_native_date_time fails. All three return None and keep today's sub-select. Positions in the FilterValue list line up ([from, to, trailing, leading, series_from, series_to] vs get(4)/get(5); [from, to, granularity, window_from, window_to] vs get(3)/get(4)), with FilterValue::from(None) holding the slot for an absent interval.

Testing is genuinely strong. The flipped snapshot in a_callback_binding_leaves_the_window_whole (1..7 → 30 for every day) is the bug in numbers, the covering-bounds unit tests assert against a walked series rather than against themselves, and a_shifted_window_accepts_a_binding_addressing_its_shift pins the refusal that the shift fix removes.

Findings

#	Sev	Where	What
1	Medium	physical_plan/.../typed_filter.rs:283	shift_bound uses UTC arithmetic for time-only intervals while the stage's SQL does naive wall-clock arithmetic; across a DST boundary a sub-day leading puts the FILTER_PARAMS band inside the band the stage reads
2	Low	filter_sql_context.rs:214	format_and_allocate_in_query_tz duplicates format_and_allocate_date for one boolean; both its guards are unreachable from the only caller
3	Low	multi_stage_query_planner.rs:1314	Rebuilds the Granularity that calendar_to_date_granularity already builds; holds compiler().borrow_mut() across the rest of the body while change_date_range_filter_impl takes the same borrow right after
4	Low	multi_stage_query_planner.rs:1285	Hardcoded .000/.999 where default_format() exists; let precision = 3 triplicated; new call sites use the uncapped QueryDateTime::align_to_origin rather than the converge_guard-capped twin (pre-existing hazard, but now two more call sites)
5	Low	planner/.../to_date_rolling_window.rs:14	Manual Debug omits the new window_range, so it's invisible in the FILTER_PARAMS error path
6	Low	fanout_repro.rs:88	Asserting the whole unique param list equals two values over-pins; also #[tokio::test] with nothing awaited
7	Low	time_series.rs:64 + 3 other sites	The same nine-line rationale restated in four places; the load-bearing sentence is "the span has to cover both series shapes"

Nothing here blocks. #1 is the only one with a wrong-answer path, and it's narrow (sub-day rolling window + DST transition + callback/compiled binding) — but it is the same class of bug the rest of the PR fixes, so it's worth either matching the SQL's arithmetic or pinning it with a test.

Two notes on the stated risks, both of which I agree are correctly scoped rather than papered over: the "up to one interval more" read is real and the month-granularity 8% figure checks out against the derivation; and a trailing: unbounded window now yields None from rolling_window_band, so a FILTER_PARAMS binding on a cumulative window renders 1 = 1. That's a correctness win over today's silently-narrow restatement, but it does mean the most common cumulative shape gets no pushdown at all — worth saying out loud in the PR body alongside the other risks.

No prior review threads existed on this PR, so nothing to resolve or dedupe against.
· branch tesseract-rolling-window-literal-bounds

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.37%. Comparing base (03377e2) to head (bd4dc3d).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #11870   +/-   ##
=======================================
  Coverage   80.37%   80.37%           
=======================================
  Files         500      500           
  Lines      106573   106573           
  Branches     3885     3885           
=======================================
  Hits        85658    85658           
  Misses      20369    20369           
  Partials      546      546           

Flag	Coverage Δ
cube-backend	60.41% <ø> (ø)
cubesql	84.76% <ø> (ø)

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.

[claude]

Claude finished @waralexrom's task in 3m 4s —— View job

---

Re-review of 4c2ea18 — all 7 prior findings addressed; 1 new medium, 2 new low (details)

Full re-review — prior threads resolved, 3 inline comments posted, verdict and reasoning

• Gather context (new commits since the last review)
• Check which prior findings the new commits address
• Review the new changes on their own terms
• Resolve stale threads, post new findings

Verdict on the fix-up commit

4c2ea18 answers every one of the seven findings from the previous round, and the medium one is answered properly rather than papered over.

The DST fix (prior #1) is correct. shift_wall_clock routes a time-only interval through add_duration, which does naive_local() += duration then re-resolves in the zone (date_time.rs:230-234) — wall-clock arithmetic, matching what subtract_interval(param, '18 hour') does to a naive local parameter in the stage's SQL. The 18 hour test over the 2024-03-10 spring-forward pins exactly the case that was wrong, and the 1 day test pins the other side of the branch (a calendar day stays on the same clock time through a 23-hour day). Rewriting subtract as interval.inverse() up front instead of branching on sub_interval is the right shape: one arithmetic path, one place to be wrong.

The other six:

Prior	Status
#2 dup format_and_allocate_in_query_tz	Folded into format_and_allocate_date_impl(…, in_db_time_zone); the two unreachable guards are gone with it
#3 Granularity built twice / borrow held	One to_date_period_granularity builder, borrow scoped to the build, both call sites converted
#4 hardcoded .000, triplicated precision	default_format() at both sites, SERIES_BOUND_PRECISION const (see new finding 1)
#5 Debug omits window_range	Added
#6 over-pinned param list, needless async	contains-per-edge, plain #[test]
#7 four copies of the covering-span rationale	Cut to the load-bearing sentence in all four

All seven threads resolved. Two things from #4 remain and are fine to leave: let precision = 3 at physical_plan/filter/typed_filter.rs:262 is in a different module from the new const, and the converge_guard gap on QueryDateTime::align_to_origin was flagged as pre-existing.

New findings

#	Sev	Where	What
1	Medium	physical_plan/.../typed_filter.rs:317	carries_date counts quarter, but add_interval ignores quarter in both its is_time_only guard and its date branch — a 1 quarter frame shifts the band by zero, so a quarter-length rolling window's FILTER_PARAMS band is the bare series bounds
2	Low	multi_stage_query_planner.rs:52	SERIES_BOUND_PRECISION landed between the MultiStageQueryPlanner doc comment and the struct — the struct is now undocumented and its doc reads as the const's
3	Low	multi_stage_query_planner.rs:1294	Extracting to_date_period_granularity left to_date_window_bounds's doc block on the helper; the helper is now documented as computing a span it doesn't compute, and to_date_window_bounds has no doc

Finding 1 is the only one with a wrong-answer path. It is pre-existing in add_interval — sub_interval had the same hole before this PR — but the band is new code that depends on it, and it is the same undercount the PR set out to fix. Findings 2 and 3 are both artifacts of where the extraction cut; neither changes behaviour.

Nothing here blocks. I could not run cargo test -p cubesqlplanner in this environment (the command needs approval), so the assertions above come from reading date_time.rs, sql_interval.rs, and the new test module rather than from a run.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 2m 1s —— View job

---

Re-review of c7a4208 — 1 of 3 prior findings addressed; 2 still open, nothing new

Full re-review — 1 stale thread resolved, 2 re-affirmed, no new inline comments

• Gather context (commits since the last review round)
• Check which round-2 findings the new commits address
• Review the new changes on their own terms
• Resolve stale threads, post findings

What this push changed

The branch was force-pushed: the fix-up commit previously reviewed as 4c2ea18 is now c7a4208, with one addition. Round-2 finding 2 (const placement) is fixed, and fixed better than suggested — SERIES_BOUND_PRECISION is gone from multi_stage_query_planner.rs entirely and lives as QueryTimeSeries::MILLISECOND_PRECISION (time_series.rs:26), inside the impl block so it steals no struct doc, and next to the two functions whose precision argument it feeds. Both call sites (multi_stage_query_planner.rs:1253, :1338) read it from there.

Everything else in c7a4208 matches what round 2 verified: the wall-clock shift_wall_clock split with the 18 hour spring-forward and 1 day tests, format_and_allocate_date_impl(…, in_db_time_zone) folding the duplicated formatting path, window_range in the Debug impl, to_date_period_granularity with a build-scoped compiler borrow, contains-per-edge in fanout_repro, and the trimmed comment blocks. No re-verification needed and nothing regressed.

Still open

Both re-checked against the current working tree, both confirmed still live. Existing unresolved threads cover them, so I posted no duplicate inline comments.

Prior	Sev	Where	State
r2 #1	Medium	physical_plan/filter/typed_filter.rs:324	Open. shift_wall_clock's carries_date still counts interval.quarter != 0, and QueryDateTime::add_interval still ignores quarters on both paths — is_time_only is year == 0 && month == 0 && week == 0 && day == 0 (date_time.rs:167, quarter absent) and the date branch never reads interval.quarter. SqlInterval::from_str parses quarter/quarters (sql_interval.rs:233) into its own field and never folds it into months. So trailing: 1 quarter still routes to add_interval, lands in the time-only branch, adds a zero duration, and the band equals the bare series bounds.
r2 #3	Low	multi_stage_query_planner.rs:1290-1303	Open. to_date_window_bounds's two doc paragraphs are still concatenated onto to_date_period_granularity, which computes neither the span nor the None cases they describe; to_date_window_bounds (l.1319) still has no doc.

One small thing the const move newly enables, not worth its own thread: physical_plan/filter/typed_filter.rs:262 still has a bare let precision = 3; in the RollingWindowOffset arm. Now that QueryTimeSeries::MILLISECOND_PRECISION is pub and in a shared module, that last copy can read it too.

Caveat

cargo test needs approval in this environment, so as in round 2 the assertions above come from reading date_time.rs, sql_interval.rs, time_series.rs and the new test module rather than from a run. The author's own cargo test -p cubesqlplanner --features integration-postgres result (1395 pass) stands unchallenged.

Nothing here blocks; finding r2 #1 is the only one with a wrong-answer path, and it is narrow (a quarter-length rolling frame with a FILTER_PARAMS binding).

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 1m 32s —— View job

---

Re-review of b7a3ada — both remaining findings fixed; no new issues, nothing open

Full re-review — 2 threads resolved, no new inline comments

• Gather context (commits since the last round)
• Check round-2 findings ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 (quarter) and ResultSet pivot methods documentation  #3 (doc block)
• Review the new changes on their own terms
• Resolve stale threads, post findings

What this push changed

One new commit, b7a3ada, on top of the previously reviewed c7a4208. It closes both of the threads still open.

r2 #1 — quarter dropped (Medium, the only wrong-answer path left). Fixed, and fixed at the root rather than at the call site. QueryDateTime::add_interval now counts quarter in both places it was missing:

• is_time_only gained && interval.quarter == 0 (date_time.rs:167), so a quarter-only interval no longer falls into the absolute-arithmetic branch and returns the anchor untouched.
• The date branch folds it in: month = date.month() + interval.quarter * 3 + interval.month (date_time.rs:188).

That matches what the series' own helper has always done — add_interval_to_dt computes year * 12 + quarter * 3 + month (time_series.rs:344) — so the two arithmetic paths now agree, which was the actual defect. No double-count risk: SqlInterval::from_str parses quarter into its own field and nothing else folds it into month before calling, and inverse() negates quarter alongside the rest (sql_interval.rs:140), so sub_interval follows.

Two things worth saying about the blast radius, since the fix touches a shared primitive:

• The other callers of QueryDateTime::add_interval — Granularity::is_aligned_with_date_range (granularity.rs:285), QueryDateTime::align_to_origin (date_time.rs:278/:284), the pre-aggregation optimizer (optimizer.rs:601) — were all silently wrong for a quarter-only interval before, never right. In the two loop-shaped ones the old behaviour was worse than wrong: offset never advanced, so a custom granularity of 1 quarter reaching either loop spun forever. This commit removes that hang as a side effect.
• granularity.rs:210-215 already documented the quarter grain as stepping through add_interval and clamping at month ends — the comment described behaviour the code didn't have. It does now.

The test pins it from both directions: equality against 3 month (the invariant), and a literal for 2 quarter (the magnitude), so a future edit can't satisfy it by making both sides equally wrong.

r2 #3 — doc block on the wrong function. Fixed. The two paragraphs about the span and the None cases moved back onto to_date_window_bounds (and got shorter in the move); to_date_period_granularity keeps only the sentence about the Granularity and the scoped compiler borrow, which is what it actually does.

Also in this commit: the last bare let precision = 3 in the RollingWindowOffset arm now reads QueryTimeSeries::MILLISECOND_PRECISION, closing the loose end noted at the end of round 3.

State of the review

All ten threads from rounds 1–3 are resolved. Nothing open, no new findings. The three fix-up commits addressed every item raised — including both mediums (DST wall-clock arithmetic, quarter) at the root rather than by special-casing the caller — and each came with a test that fails without it.

The stated risks in the PR body still stand as written and are correctly scoped: up to one granularity interval of extra rows read, no literal bound for a calendar-backed to_date period, and the FILTER_PARAMS band rendering 1 = 1 for a trailing: unbounded window (wider-but-correct, replacing a silently-narrow restatement). That last one is still worth a line in the Risks section.

Caveat

cargo test needs approval in this environment, so as in rounds 2 and 3 the verification above comes from reading date_time.rs, sql_interval.rs, time_series.rs, granularity.rs and the new test module rather than from a run. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 1m 25s —— View job

---

Re-review of 50b643e — test-only commit, correct and well-scoped; no findings, nothing open

Full re-review — no new inline comments, all 10 prior threads already resolved

• Gather context (commits since b7a3ada)
• Review the new test commit on its own terms
• Re-check prior threads for anything still live
• Post findings

What this push changed

One commit, 50b643e, on top of the previously reviewed b7a3ada. It adds no production code: an amount_cumulative measure (trailing: unbounded) to the filter_params_rolling_window fixture, a CUMULATIVE_QUERY, and two tests pinning the asymmetry between the two FILTER_PARAMS forms for a window with one derivable bound.

That asymmetry is the last item I'd flagged as worth stating out loud rather than leaving as behaviour a reader discovers. It's now pinned by tests instead, which is better.

Verification of the two claims

Both check out against the code as it stands:

a_column_binding_keeps_the_upper_bound_of_a_cumulative_window. The FilterParamsColumn::String path routes to dispatch_to_sql → RegularRollingWindowOp::to_sql (physical_plan/filter/operators/rolling_window.rs:7), which takes date_range_literals(&self.series_range), then extend_date_range_bound per side. trailing: unbounded drops the lower bound, leaving the (None, Some(to)) arm → lte alone. So 2024-03-07T23:59:59.999 as the sole literal, and no >=, is exactly right — and .999 is the covering upper bound this PR derives, not the raw range end, which is the thing that would regress silently.

The two assertions are tight together in a way neither is alone: !contains(">=") on its own would also pass for a predicate that stated nothing, and the predicate_values equality rules that out. Worth noting the equality here is scoped to placeholders the predicate actually names (predicate_values walks $n out of the predicate text), so it doesn't repeat the over-pinning of the whole param list that round 1 flagged in fanout_repro.

a_callback_binding_states_nothing_for_a_cumulative_window. rolling_window_band does from.zip(to); shift_bound returns None for "unbounded", so the zip is None, filter_param_values returns Ok(None), and the Callback arm returns always_true() → (1 = 1). Matches.

The timezone split stays consistent in both: the column path compares convert_tz(member) against a query-tz literal, the band path hands the callback dates through the _no_cast (db-tz) helpers against the cube's own unconverted column. Unchanged by this commit, re-checked because the new fixture exercises both.

Notes

• No snapshot churn, as expected — amount_cumulative is only added to the fixture schema and never appears in the other queries, so nothing else's SQL moves.
• Unlike the trailing-30d and month-to-date cases, the cumulative pair has no try_execute_pg counterpart proving the numbers. Fine as-is: the claim is about which bounds the scan carries, not about a count the scan could get wrong in a way SQL shape hides.
• The PR body's Risks section still doesn't mention that a trailing: unbounded window renders 1 = 1 for a callback binding. With this test the behaviour is pinned rather than merely true, so it matters less, but a line there would still save a reader the archaeology.

State of the review

All ten threads from rounds 1–4 are resolved; nothing open, nothing new. The three fix-up commits addressed every item raised, both mediums at the root, and this commit adds coverage for the one remaining behaviour that was documented only in prose.

Caveat

cargo test needs approval in this environment, so as in rounds 2–4 the verification above comes from reading rolling_window.rs, typed_filter.rs and the test module rather than from a run. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 3m 42s —— View job

---

Re-review of 5d8ad7c — fix is correct for regular windows; 1 medium, 2 low; all 10 prior threads already resolved

Full re-review — 3 inline comments posted, verdict and reasoning

• Gather context (commits since 50b643e)
• Review the new commit on its own terms
• Re-check prior threads for anything still live
• Post findings

What this push changed

One commit, 5d8ad7c, on top of the previously reviewed 50b643e. It addresses a defect the earlier rounds didn't reach: the base scan's bounds became literals, but the window's frame was still applied around them in SQL (subtract_interval(?, '30 day')), making the whole bound a constant expression. Pinot fails to plan TIMESTAMPADD(MONTH, -2, CAST(? AS TIMESTAMP)), so every rolling-window query over it broke.

The frame is now folded into the bound while planning — RegularRollingWindowOp::series_range becomes scan_range, already carrying the frame — and the predicate is a bare comparison against a date. shift_wall_clock moved off typed_filter.rs onto QueryDateTime::add_interval_wall_clock.

Verification

Traced the whole path; the regular-window half is right.

• The fold matches what the SQL did. fold_frame → add_interval_wall_clock, which for a time-only interval does naive_local() += duration then re-resolves, and otherwise routes to add_interval's local-date branch. Both are wall-clock, which is what subtract_interval against a naive local parameter was. The DST split from round 2 survives the move intact.
• Precision survives the fold. add_interval step 3 recombines with self.naive_local().time(), which carries nanoseconds, so a .999 upper bound folded by a leading interval stays .999 and format_to_date's .999 → .999999 special case still fires on a microsecond dialect.
• Unbounded handling is consistent across all three consumers. fold_frame keeps the unshifted bound; keep_bounded drops it at render time, so trailing: unbounded renders lte alone; rolling_window_band returns None when either side is unbounded, so a callback binding renders 1 = 1. That matches the two cumulative tests added in 50b643e, which still hold.
• The sub-select fallback still applies the frame in SQL — correct, since there the bound is a column and no constant folding happens. Reached for use_raw_values (pre-aggregation), a query-shaped date range, and a calendar granularity, all unchanged.
• Timezone. scan_range is derived in the query timezone and rendered by date_range_literals without apply_db_time_zone, against convert_tz(member). The FILTER_PARAMS band takes the same scan_range through the db-tz _no_cast helpers against the cube's own unconverted column. Both splits are as they were; the fold doesn't cross either.
• Test arithmetic checks out. fanout_repro: 2026-08-01 − 7d = 2026-07-25, − 30d = 2026-07-02 ✓. timezone.rs: 2024-01-10 − 7d = 2024-01-03 ✓. No SQL snapshots move (the four rolling .snap files record result values, not SQL).
• to_date is untouched and already emitted bare literals, so the Pinot shape never existed there.

Findings

#	Sev	Where	What
1	Medium	physical_plan/.../rolling_window.rs:64,73	RollingWindowOffsetOp::to_sql still wraps an allocated parameter in subtract_interval/add_interval — the exact TIMESTAMPADD(MONTH, -2, CAST(? AS TIMESTAMP)) shape this commit removes from the regular path. Rolling windows with offset: start/end stay broken on Pinot. The folded bounds already exist at plan time in rolling_window_band's offset arm
2	Low	tests/filter_params_rolling_window.rs:131	Nothing pins the fix. This test's predicate.contains("interval '30 day'") was the one assertion that observed SQL interval arithmetic, and it was replaced by a value assertion. Every surviving check — here, fanout_repro, timezone.rs — passes under a double application (folded literal and subtract_interval still around it), so reverting the to_sql half leaves the suite green
3	Low	multi_stage_query_planner.rs:1341	fold_frame duplicates shift_bound line for line except for what unbounded means; all four DST/quarter tests sit on shift_bound, so the copy that decides what the scan actually reads is untested. Also "unbounded" is now compared as a literal in four places, and keep_bounded takes an unused &self

Finding 1 is the only one that leaves a user-visible failure, and it's the same failure the commit set out to fix — just on the other rolling-window shape. Finding 2 is what would have caught it: a single !predicate.contains("interval") in the two tests that already extract the predicate.

Nothing here blocks the regular-window fix, which is correct and well targeted.

Prior rounds

All ten threads from rounds 1–4 are resolved; none re-opened by this commit. The shift_wall_clock → add_interval_wall_clock move preserves the round-2 DST fix and the round-3/4 quarter fix in add_interval verbatim.

Caveat

cargo test -p cubesqlplanner needs approval in this environment, so as in rounds 2–5 the verification above comes from reading date_time.rs, filter_sql_context.rs, rolling_window.rs, typed_filter.rs and the test modules rather than from a run. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged — though note finding 2 means a green suite is weaker evidence for this particular commit than for the earlier ones.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 2m 58s —— View job

---

Re-review of 786bce0 — all 3 prior findings fixed; 1 new low-medium, 1 leftover nit

Full re-review — 3 threads resolved, 1 inline comment posted, verdict and reasoning

• Gather context (commits since 50b643e)
• Check round-5 findings ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 (offset to_sql), MUTEX for requests #2 (no test pins the fold), ResultSet pivot methods documentation  #3 (fold_frame duplication)
• Review the new changes on their own terms
• Resolve stale threads, post findings

What this push changed

One commit, 786bce0, answering all three round-5 findings.

Prior	Status
r5 #1 — RollingWindowOffsetOp::to_sql still wrapped its parameter in subtract_interval/add_interval (Medium, the one user-visible failure left)	Fixed. The anchor is formatted but no longer allocated (ctx.format_from_date/format_to_date), the frame folds in via shift_bound_wall_clock, and the bound is allocated after. Both rolling operators now emit bare comparisons
r5 #2 — nothing pinned the fold; the suite stayed green under a double application	Fixed. !predicate.contains("interval") in filter_params_rolling_window, per-base-scan in fanout_repro, !sql.contains("interval") in basic_types, and the four offset/no-granularity cases now assert the shifted date instead of the interval text
r5 #3 — fold_frame duplicated shift_bound	Fixed. One shift_bound_wall_clock in date_time.rs serves all three callers; the planner uses it with .unwrap_or(bound), so the DST and quarter tests now sit on the function the base scan actually reads. UNBOUNDED_INTERVAL named alongside it

Verification

• Anchor still needs the pre-format. A bare 2024-01-20 has to become …T23:59:59.999 before the shift, or an offset: end window would shift midnight instead of end-of-day. Keeping format_from_date/format_to_date unallocated for that is the right split.
• Test arithmetic checks out. basic_types: 2024-01-20T23:59:59.999 − 3d = 2024-01-17, + 1d = 2024-01-21 ✓; trailing-only: − 7d = 2024-01-13, upper = the unshifted 2024-01-20 ✓. offset_no_granularity: 2024-01-10 − 7d = 2024-01-03 (start), 2024-01-16T23:59:59.999 − 7d = 2024-01-09 (end) ✓.
• Unbounded stays consistent. shift_bound_wall_clock returns None, the offset arm's if let Some drops the condition, the planner's unwrap_or keeps the unshifted bound for keep_bounded to drop at render, and rolling_window_band still yields None — the two cumulative tests from 50b643e are unaffected.
• Timezone. The offset operator compares an unconverted member, and the shift is wall-clock in the query tz followed by apply_db_time_zone at allocation. That now matches how rolling_window_band derives the FILTER_PARAMS band for the same operator (query-tz shift, then the db-tz _no_cast helpers) — the two derivations agree, which is the point.
• DST and quarter fixes survive the move verbatim: add_interval_wall_clock still branches on carries_date (quarter included) and add_interval still folds quarter * 3 into months.

New findings

#	Sev	Where	What
1	Low–Medium	physical_plan/.../rolling_window.rs:70	Each folded bound is re-normalised by the side of the comparison, not of the anchor. At timestampPrecision() == 6 (BigQuery) an offset: end lower bound round-trips .999999 → .999 (via default_format's %.3f) → .999000 (via format_from_date, which has no .999 case). This predicate is the window's frame — nothing re-applies it — so the window gains 999 µs of rows. Was exact before the commit
2	Low	filter_sql_context.rs:281, :293	keep_bounded and extend_date_range_bound still compare the "unbounded" string literal now that UNBOUNDED_INTERVAL exists two modules over; keep_bounded also still takes an unused &self. The leftover half of r5 #3 — not worth its own thread

Finding 1 is invisible in the suite because every offset test runs at precision 3, where steps 2–3 round-trip exactly. Narrow (microsecond dialect + offset: end + a trailing frame), and it makes the window slightly wider rather than narrower, so no rows are lost — but it is a result change on the dialect this PR was written for, and the fix is one closure.

Nothing here blocks. The offset-window fix is correct in shape and closes the last place a dialect had constant interval arithmetic to fold.

Prior rounds

All thirteen threads from rounds 1–5 are now resolved; the three that were open are answered by this commit and I resolved them rather than re-affirming.

Caveat

cargo test -p cubesqlplanner needs approval in this environment, so as in rounds 2–5 the verification above comes from reading date_time.rs, date_time_helper.rs, filter_sql_context.rs, rolling_window.rs and the test modules rather than from a run. BigqueryQuery.ts:162 was read directly to confirm precision 6 is reachable. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged — noting that it runs at precision 3, which is exactly where finding 1 hides.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 2m 12s —— View job

---

Re-review of 9f0182d — both prior findings fixed; 1 new low (narrower residual of the same defect)

Full re-review — 1 thread resolved, 1 inline comment posted, verdict and reasoning

• Gather context (commits since 786bce0)
• Check round-6 findings ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 (precision round-trip) and MUTEX for requests #2 (unbounded literal / unused &self)
• Review the new changes on their own terms
• Resolve stale threads, post findings

What this push changed

One commit, 9f0182d, answering both round-6 findings.

Prior	Status
r6 #1 — folded bounds re-normalised by comparison side, losing 999 µs off the offset: end lower bound at precision 6 (Low–Medium, the one result-changing path)	Fixed. An allocate closure keyed on from_start sends both bounds through the anchor's own helper (rolling_window.rs:68)
r6 #2 — "unbounded" compared as a literal in keep_bounded/extend_date_range_bound; keep_bounded took an unused &self	Fixed. Both read UNBOUNDED_INTERVAL (date_time.rs:23); keep_bounded is an associated fn and both call sites say FilterSqlContext::keep_bounded

Verification

• The fix is right, and right for both offsets. For offset: end the lower bound now goes format_to_date(to, 6) → .999999 → shift → .999 → format_to_date → .999999, exact; before it landed on .999000. For offset: start nothing moves — the anchor is .000000, and both helpers pad .000 → .000000 identically — so the change is confined to the arm that was wrong.
• The test pins it from both sides. Asserting the format_from_date result as well (.999000, "which is what the bug did") means a future edit can't satisfy the test by making both helpers equally lossy. Good shape.
• UNBOUNDED_INTERVAL now covers every place in the rolling path: shift_bound_wall_clock (date_time.rs:38), is_unbounded (typed_filter.rs:282), keep_bounded and extend_date_range_bound. The two literals left — query_properties.rs:999 and join.rs:45 — are outside it and pre-existing.
• No behavioural drift elsewhere. keep_bounded losing &self is mechanical; RegularRollingWindowOp is otherwise untouched, so the round-5 verification of the regular fold still stands.

New finding

#	Sev	Where	What
1	Low	physical_plan/.../rolling_window.rs:68	The round-trip is lossless only because format_to_date special-cases a .999 tail. shift_bound_wall_clock renders through default_format() (%.3f) regardless of dialect precision, and parse_native_date_time accepts %.f — so a dateRange end of 2024-01-16T12:00:00.123456 at precision 6 comes back .123000, 456 µs earlier on the same frame-defining > bound

Same defect as the one just fixed, one order narrower: it needs an explicitly microsecond-valued dateRange, not merely a microsecond dialect. Choosing the right helper can't close it — the loss happens between the two helpers, in the shift's own %.3f. Letting shift_bound_wall_clock render at the dialect's precision would, and would stop the .999 special case from being load-bearing here.

Two comment/structure nits, neither worth a thread: the new tests landed in a second #[cfg(test)] mod shift_bound_tests directly above the file's existing mod tests, which already holds the DST and quarter cases — one module would keep the shift tests together; and the new test's four-line preamble restates what the two assertion messages already say.

Prior rounds

Fourteen threads across rounds 1–6; the one that was open is answered by this commit and I resolved it. Nothing else re-opened.

Caveat

cargo test -p cubesqlplanner needs approval in this environment, so as in rounds 2–6 the verification above comes from reading date_time.rs, date_time_helper.rs, filter_sql_context.rs and rolling_window.rs rather than from a run. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged — noting again that it runs at precision 3, where the new test's own subject is invisible, which is why the test states the precision-6 round-trip explicitly rather than going through a query.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

This closes the .999 case, and the new test pins it. One narrower leak survives it, same shape: the round-trip is only lossless because format_to_date special-cases a .999 tail — every other sub-millisecond tail still gets truncated.

shift_bound_wall_clock renders through QueryDateTime::default_format() (%.3f, date_time.rs:194), and parse_native_date_time accepts %.f, so a dateRange end of 2024-01-16T12:00:00.123456 at precision 6 goes:

1. ctx.format_to_date(to) → .123456 (26 chars, matches DATE_TIME_LOCAL_U_RE, returned as-is);
2. shift → .123;
3. format_and_allocate_to_date → .123000, because the .999 case doesn't apply.

456 µs earlier against the > lower bound, which — as with the case this commit fixes — is the window's own frame with nothing to re-apply it. Requires an explicitly microsecond-valued dateRange on a microsecond dialect, so narrower still than the .999 case, but it's the same defect and it isn't fixed by choosing the right helper.

The root of it is that the shift renders at millisecond precision regardless of what the anchor carried. A default_format_with_precision(p) — or having shift_bound_wall_clock take the precision it should render at — would make the round-trip exact for any tail and let format_to_date's .999 special case stop being load-bearing here.

Fix this →

[claude]

Claude finished @waralexrom's task in 4m 0s —— View job

---

Re-review of 96b5785 — rebase onto master's base-scan sharing (#11852); no new findings, 1 prior low still open

Full re-review — no new inline comments, 1 prior thread re-affirmed, verdict and reasoning

• Gather context (commits since 9f0182d)
• Review the new changes on their own terms
• Check the interaction with the newly-merged base-scan sharing pass
• Re-check the round-7 finding
• Resolve stale threads, post findings

What this push changed

The branch was force-pushed. The 16 commits are the same subjects in the same order as round 7's 9f0182d, the diffstat is unchanged (+1654 / −126 over 25 files), and every file I re-read matches what round 7 verified line for line — the allocate closure keyed on from_start at physical_plan/filter/operators/rolling_window.rs:68, FilterSqlContext::keep_bounded as an associated fn reading UNBOUNDED_INTERVAL, QueryTimeSeries::MILLISECOND_PRECISION at its three call sites, shift_bound_wall_clock in date_time.rs:31.

What actually moved is underneath: the merge-base is now 03377e2, fix(tesseract): share a rolling window's base scan across measures (#11852) — the other half of #11770 the PR body points at. So this round's real subject is whether literal scan bounds and shared base scans agree. They do.

Interaction with #11852

• Sharing cannot merge two scans with different bounds. reads_same_rows compares same_filter → tree_ops::eq_with_member → BaseFilter::eq, which compares typed_filter.values() (base_filter.rs:26-32). replace_regular_date_range_filter pushes scan_from/scan_to into that same value list (query_properties.rs:1046-1049), so the derived span is part of the equality, not merely implied by it. Two frames that differ already differ at trailing/leading; the folded bounds now differ too. The pass stays conservative in the direction it was built to be.
• No under-sharing either. The span derives from the time dimension's granularity, date range and the measure's own frame — nothing measure-specific — so two windows with equal frames get byte-equal bounds and still merge. Master's own guard test (base_scan_merge: differing frame, differing fact, time shift, no granularity, per-measure rollup) is unaffected.
• fanout_repro still says what it means under merging. The fixture's five rolling measures collapse to two frames, so sql.split("FROM rw_daily_activity") now yields two base scans instead of five — the !predicate.contains("interval") loop and the three-edge bounds check (2026-07-25, 2026-07-02, 2026-09-02T23:59:59.999) are per-edge contains, so neither is coupled to the count. The split does match a real scan: the cube's SELECT * FROM rw_daily_activity is collapsed by the trivial_subquery pass, so the fact reference renders bare.
• Time shifts are untouched by the fold. A shift is applied to the member, not by rewriting the filter's values, so a literal bound compares against the shifted expression exactly as the sub-select did. same_evaluation_context keeps a shifted leaf out of a shared scan regardless.

Still open

One thread, re-checked against the current tree and confirmed live. It already has an unresolved thread, so I posted no duplicate.

Prior	Sev	Where	State
r7 #1	Low	planner/time_dimension/date_time.rs:47-51	Open. shift_bound_wall_clock still renders through QueryDateTime::default_format() (%.3f, date_time.rs:193), so any sub-millisecond tail other than .999 is truncated on the round-trip: at precision 6 a dateRange end of 2024-01-16T12:00:00.123456 comes back .123000, 456 µs early against the frame-defining > bound. Narrow — needs an explicitly microsecond-valued range on a microsecond dialect — and it makes the window wider, not narrower.

Every other thread from rounds 1–7 is resolved; nothing re-opened by the rebase.

Caveat

cargo check / cargo test -p cubesqlplanner both need approval in this environment, and the compare API call I'd have used to diff 9f0182d...96b5785 directly was likewise unavailable — so "content unchanged" above rests on reading the files against round 7's verified descriptions rather than on a mechanical diff, and nothing here re-verifies that the rebased tree compiles. The author's cargo test -p cubesqlplanner --features integration-postgres run stands unchallenged; a fresh run on the rebased branch is what would settle the merge with #11852.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 3m 47s —— View job

---

Re-review of 19d9d4a — narrowing is correct and well-plumbed; 2 low, 1 prior low still open

Full re-review — 2 inline comments posted, 1 prior thread re-affirmed, verdict and reasoning

• Gather context (commits since 96b5785)
• Review the new commits on their own terms
• Re-check prior threads
• Post findings

What this push changed

Two commits on top of the previously reviewed 96b5785. 99d8d4a adds two integration tests, one per series shape; 19d9d4a replaces the single (String, String) scan span with a SeriesSpan carrying two upper bounds — to_aligned for a series materialized as rows, to_stepped for one generated in SQL — plus the predefined_granularity flag, and lets the filter pick between them via supports_generated_time_series.

That's the right place to decide it. FilterSqlContext::series_span_end calls exactly the predicate the physical plan builder calls to choose the shape (multi_stage_time_series.rs:93, physical_plan/time_series.rs:87), with exactly the argument it passes, so the bound cannot describe a series the query doesn't render.

Verification

• to_aligned is exact for the walked shape. generate_predefined keeps buckets while bucket_start <= range_end, so its last bucket is predefined_bucket(range_end) and its end_str is last.end_str — the unit test now asserts equality rather than >=, which is the strongest form of this claim and it holds.
• Nothing under-reads. Walked ≤ to_aligned, generated ≤ to_stepped on both paths; for a custom granularity the generated series starts origin-aligned (get_range_for_time_series aligns only for non-predefined granularities, time_dimension_symbol.rs:378-383) so its points are origin-aligned and align(range_end) + interval − 1s covers them, and the walked custom series is a subset of that. The one-value collapse for custom is sound in the covering direction.
• Plumbing positions line up. series_span_values writes [from, to_aligned, to_stepped, Bool(predefined)]; TypedFilterBuilder::series_span reads at..at+3 with at = 4 (regular, after trailing/leading) and at = 3 (to_date, after granularity). None still writes nothing, so the sub-select fallbacks are untouched. The Bool never reaches a param — both rolling arms are matched ahead of the generic to_param_string() fan-out — and it lands in values(), which is what BaseFilter::eq compares, so the base-scan-sharing pass from fix(tesseract): share a rolling window's base scan across measures #11852 keeps two differently-shaped spans apart.
• to_date_window_bounds keeps both ends via ..series and replaces only from; regular_scan_span folds leading into both upper bounds and trailing into the lower one, through the same shift_bound_wall_clock the DST and quarter tests sit on.
• The two new tests pin opposite directions and would each fail on the other shape's bound — 2024-03-31T23:59:59.999 present and nothing in 2024-04 for the walked one, >= 2024-04-14T23:59:59.999 for the generated one.

Findings

#	Sev	Where	What
1	Low	time_series.rs:128	The over-read is gone for the walked shape only. to_stepped is range_end + interval − 1s, not the end of the last step (the new test's own numbers: last bucket ends Apr 14, bound says Apr 19), and from is the bucket-snapped start for both shapes though the generated series' first point is the raw range start. Same defect, same fix shape, at the end of the range that BigQuery actually renders
2	Low	multi_stage_query_planner.rs:1291	For a custom granularity the two ends are collapsed into one, but the walked series stops exclusively (aligned < range_end) where the generated one is inclusive. A range end exactly on a bucket boundary makes the walked scan read one extra bucket — the case this commit exists to remove

Neither is a wrong-answer path; both are reads wider than needed, in the same direction the commit narrows.

Still open

Prior	Sev	Where	State
r7 #1	Low	planner/time_dimension/date_time.rs	Open, untouched by these commits. shift_bound_wall_clock still renders through default_format() (%.3f), so a microsecond-valued dateRange bound round-trips lossy at precision 6. Existing thread covers it; no duplicate posted

Every other thread from rounds 1–7 is resolved.

Caveat

cargo check / cargo test -p cubesqlplanner need approval in this environment, so nothing above was run — the verification comes from reading time_series.rs, time_dimension_symbol.rs, multi_stage_time_series.rs, physical_plan/time_series.rs, plan.rs and query_properties.rs. In particular the new SeriesSpan threading is a type change across eight files and has not been compiled here; the author's cargo test -p cubesqlplanner --features integration-postgres run is the evidence for that.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

The upper bound is now exact for the walked shape, but the same over-read this commit removes survives on the generated shape, at both ends — and the generated shape is the one BigQuery renders, i.e. the partitioned table the PR is aimed at.

• to_stepped is range_end + interval − 1s, not the end of the last step. Stepping from the range start, the last point is range_start + k·interval ≤ range_end, so the bound is up to a full interval past it. The new test's own arithmetic shows it: stepping months from Jan 15 ends the last bucket on Apr 14, while to_stepped is Apr 19 — hence the >= in the assertion, where the walked test can say assert_eq!.
• from is predefined_bucket(range_start).start_str for both shapes, but the generated series' first point is the raw range_start. So the low end reads up to an interval of rows no frame can reach, symmetric to what to_aligned just fixed at the high end.

Both close the same way to_aligned did — a from_aligned/from_stepped pair, and a stepped upper bound walked the way the series walks it (the loop already exists in generate_predefined, capped by MAX_BUCKETS). If that is deliberately left for later, it's worth saying in the Risks section, which currently reads as if the extra-interval read is gone.

Fix this →

[claude]

Collapsing the two ends into one value for a custom granularity is right for the generated shape but half an interval too generous for the walked one, because the two series differ in more than alignment: generate_custom stops at while aligned < range_end (exclusive), while the generated SQL steps d <= end (inclusive) — the asymmetry generate_predefined's own comment calls out as intentional.

So for a range end that lands exactly on a bucket boundary — dateRange: ["2024-01-01", "2024-04-01"] against a 1 month custom granularity with a Jan 1 origin — the walked series' last bucket is Mar 1–Mar 31, while align_date_to_origin(Apr 1) + 1 month − 1s gives Apr 30. One whole extra bucket of the fact table, on the shape this commit exists to tighten.

to_aligned wants the last bucket the walked series actually emits: align, and step back one interval when the alignment is exact.

(Also: this adds a second Granularity::align_date_to_origin call site, which is the uncapped twin of the converge_guard-protected helper the series walks with — pre-existing, noted in an earlier round, just now one site wider.)

Fix this →

[claude]

Claude finished @waralexrom's task in 1m 53s —— View job

---

Re-review of 61f5006 — dependency-only commit, correct; no new findings, 3 prior lows still open

Full re-review — no new inline comments, 3 prior threads re-affirmed, verdict and reasoning

• Gather context (commits since 19d9d4a)
• Review the new commit on its own terms
• Re-check prior threads
• Post findings

What this push changed

One commit, 61f5006, on top of the previously reviewed 19d9d4a. It touches no Rust: packages/cubejs-oracle-driver/package.json moves oracledb from ^6.2.0 to ^6.3.0 in optionalDependencies, and yarn.lock follows, resolving 6.2.0 → 6.10.0. Everything the seven earlier rounds verified in the planner is byte-identical, so nothing there needed re-verification.

Verification of the bump

• The premise holds. ^6.2.0 already admitted every fixed release; only the lockfile pinned 6.2.0, so a fresh yarn install off this repo was the one place the broken thin-mode client could come back. Raising the floor alongside the lockfile is what makes that unrepeatable — a lockfile-only change would have left npm i @cubejs-backend/oracle-driver free to resolve 6.2.0 again.
• No other consumer to keep in step. oracledb appears in exactly one package.json in the monorepo (this one), and in no Dockerfile or CI compose file, so the single lockfile entry is the whole surface. @types/oracledb stays ^6.2.3, which resolves forward on its own and is a devDependency besides — the defect is in the wire protocol, not the typings.
• Nothing in the driver is version-sensitive across the jump. OracleDriver.js touches oracledb at four lines: outFormat = OBJECT, partRows, maxRows, prefetchRows (l.78–82), plus getConnection/ping/close. All are stable 6.x surface; there is no initOracleClient, no thick-mode branch, no dbType constant use beyond the metaData.dbTypeName string map, which is driven by Oracle's own type names rather than by the client.
• Node floor is fine. 6.10.0 needs Node 14.6+; the package declares engines.node >= 20.0.0.
• Thin mode means no native build, so the optional dependency stays installable in the same environments it was before — no new toolchain requirement sneaks in with the eight-minor jump.

The one thing worth saying out loud: the declared floor moves one minor (6.3.0) but the installed version moves eight (6.10.0), and the commit message only justifies 6.3.0. That's ordinary caret resolution and a lockfile that had been frozen for a long while rather than anything this commit does wrong — but 6.3→6.10 is the part no test here has exercised, and the driver has no integration suite in this repo to exercise it. If a smaller step is preferred, the lockfile is where to say so; the manifest is already right either way.

Scope

This is an unrelated fix riding a Tesseract PR — it will publish as an @cubejs-backend/oracle-driver patch whose changelog sits under a branch named for rolling-window bounds. Harmless and already reflected in the PR title/labels; noting it only so the coupling is deliberate rather than incidental.

Still open

No Rust changed, so all three prior findings stand exactly as last round. Each already has an unresolved thread; no duplicates posted, nothing to resolve.

Prior	Sev	Where	State
r7 #1	Low	planner/time_dimension/date_time.rs	Open. shift_bound_wall_clock renders through default_format() (%.3f), so a microsecond-valued dateRange bound round-trips lossy at precision 6
r8 #1	Low	planner/time_dimension/time_series.rs:128	Open. Re-read on the current tree: to_stepped is still range_end + interval − 1s rather than the end of the last step, and from is still first.start_str for both shapes though the generated series opens at the raw range start
r8 #2	Low	multi_stage_query_planner.rs:1291	Open. Custom granularity still collapses both ends into one value, while the walked series stops exclusively and the generated one inclusively

None is a wrong-answer path; all three are reads wider than needed, in the direction the PR narrows.

Caveat

cargo test / yarn install both need approval in this environment, so nothing was run — the verification above comes from reading OracleDriver.js, the two changed manifest files, and the planner files backing the three open threads. The author's cargo test -p cubesqlplanner --features integration-postgres result stands unchallenged, and the NJS-111 reproduction is theirs against a real Oracle server, which this environment has no way to check.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Claude finished @waralexrom's task in 2m 39s —— View job

---

Re-review of bd4dc3d — the Oracle fix was traded for a skip; 1 medium, 2 low, 3 prior lows still open

Full re-review — 1 inline comment posted, 3 prior threads re-affirmed, verdict and reasoning

• Gather context (commits since 61f5006)
• Review the revert and the Oracle skip on their own terms
• Re-check prior threads
• Post findings

What this push changed

Two commits on top of the previously reviewed 61f5006, neither touching Rust:

• 2b94dd0 reverts 61f5006 wholesale — optionalDependencies.oracledb back to ^6.2.0, yarn.lock back to 6.2.0 from 6.10.0.
• bd4dc3d adds SQL API: Rolling Window YTD (year + month + day + date_trunc equal) and … date_trunc IN to tesseractSkip in packages/cubejs-testing-drivers/fixtures/oracle.json, with a comment attributing the failure to an NJS-111 protocol bug in the pinned thin client.

So the same defect is answered the other way round from last round: instead of moving past the driver bug, the two tests that hit it are turned off. Every planner file is byte-identical to 61f5006, so the eight rounds of verification behind the Tesseract half all stand unchanged.

Verification

• The skip mechanics are right. execute/executePg treat tesseractSkip as the complete list under isTesseractEnv rather than as an addition to skip (src/tests/testQueries.ts:82, :94), so a name already in skip does need repeating there. Both strings match the executePg names exactly (testQueries.ts:2882, :2898) — a typo here would silently skip nothing.
• The pairing claim checks out in shape. Both queries are the same YTD rolling window over BigECommerce differing only in = CAST(…) vs IN (CAST(…)), which CubeSQL folds to the same predicate — so they do reach one plan, and skipping one alone would move the failure rather than remove it.
• These are not previously-failing tests. On master both names appear only in the legacy skip list (origin/master:oracle.json:210-211), never in tesseractSkip. Under Tesseract they run and pass there and fail here — the new literal-bound to_date plan is what reaches the driver bug.
• ^6.2.0 is not the problem for fresh installs, as noted last round: the caret already admits 6.10.0. The lockfile is the whole exposure — and packages/cubejs-docker/latest.Dockerfile:20 installs the published image from it.

Findings

#	Sev	Where	What
1	Medium	fixtures/oracle.json:256-262 + 2b94dd0	The revert puts yarn.lock back on oracledb@6.2.0, which the published Docker image installs, so an Oracle user running a YTD rolling window through that image hits NJS-111 at runtime. The skip hides it in CI without closing it, and names no follow-up — "waits on a driver upgrade" has no addressee
2	Low	fixtures/oracle.json:256	Five comment lines where the rest of this file runs one to three; the load-bearing sentence is the last one (both shapes reach one plan) plus the version to upgrade past
3	Low	fixtures/oracle.json:256	(6.2.0) restates what yarn.lock owns and goes stale silently when the lockfile moves; oracledb below 6.3.0 says the same and stays true

Finding 1 is a judgement call rather than a defect — if an eight-minor dependency jump was felt not to belong in a Tesseract PR, that is a reasonable reason to revert it. The ask is only that the skip point at whatever picks it up.

Still open

No Rust changed, so all three prior findings stand exactly as last round. Each has an unresolved thread; no duplicates posted, and nothing became resolvable.

Prior	Sev	Where	State
r7 #1	Low	planner/time_dimension/date_time.rs	Open. shift_bound_wall_clock renders through default_format() (%.3f), truncating a microsecond-valued dateRange bound at precision 6
r8 #1	Low	planner/time_dimension/time_series.rs:128	Open. to_stepped is still range_end + interval − 1s rather than the end of the last step, and from is the bucket-snapped start for both shapes
r8 #2	Low	multi_stage_query_planner.rs:1291	Open. Custom granularity collapses both ends into one value, while the walked series stops exclusively and the generated one inclusively

Caveat

Nothing was run: cargo test needs approval here, and the failure this push works around is reproducible only against a real Oracle server, which this environment has no access to. The diagnosis (NJS-111, fixed in 6.3.0, succeeds via sqlplus) is the author's and I have taken it as given; what I verified is the skip's mechanics, that the two tests passed on master under Tesseract, and that the published image installs from the reverted lockfile.

· branch `tesseract-rolling-window-literal-bounds`

[claude]

Taken together with the revert in 2b94dd0, this pair of commits removes the protection rather than the failure, and it's worth being explicit about who is left holding it.

Both names are on master in the skip list only (origin/master:oracle.json:210-211), so under Tesseract they run and pass today. Adding them to tesseractSkip is therefore not "still skipped, now also under Tesseract" — it's a shape that worked before this branch and doesn't after. execute/executePg treat tesseractSkip as the complete list under isTesseractEnv (src/tests/testQueries.ts:82,94), not as an addition to skip, so the two names are correctly placed and the mechanics are right.

The part that isn't only a test concern: packages/cubejs-docker/latest.Dockerfile:20 builds the published image with yarn install --prod against the root yarn.lock, and the revert put that lockfile back on oracledb@6.2.0. So the shipped image carries the client that raises NJS-111, and a YTD rolling window against Oracle breaks the connection at runtime for anyone on it — not just in CI. 61f5006 was what closed that, and the manifest half of it (^6.2.0 → ^6.3.0) was the part protecting a fresh npm i too.

If the bump was reverted because an eight-minor jump doesn't belong in a Tesseract PR, that's a fair call — but then the skip is standing in for a follow-up that nothing here names. A tracking issue in the comment (waits on cube-js/cube#NNNNN) would make these two unskippable by someone other than whoever wrote them; as it reads now, "waits on a driver upgrade" has no addressee.

Two smaller things in the block itself:

• Five comment lines where the rest of this file runs one to three. The load-bearing sentence is the last one — both shapes reach the same plan, so skipping one moves the failure onto the other — plus the version to upgrade past. The sqlplus detail is evidence for the diagnosis, not something a later edit could break.
• (6.2.0) restates what yarn.lock owns; when the lockfile moves, this line quietly becomes wrong while still reading as authoritative. oracledb below 6.3.0 says the same thing and stays true.

Fix this →