fix(cubesql): Fail instead of returning NULL for out-of-range timestamps

[marciodfg]

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

#11407

Description of Changes Made

transform_response materialises a Cube response into Arrow. Time dimensions land in Timestamp(Nanosecond), whose i64 nanosecond count only spans 1677-09-21 to 2262-04-11. A source value outside that window — legal in every database that stores dates as date / datetime2 / timestamp — was logged server-side and appended as NULL:

if let Some(nanos) = timestamp.and_utc().timestamp_nanos_opt() {
    builder.append_value(nanos)?;
} else {
    log::error!("Unable to cast timestamp value to nanoseconds: {}", timestamp);
    builder.append_null()?;
}

The client sees no error. SELECT MIN(d) FROM t comes back NULL over a column that holds no NULLs, the row count and the schema are both what the caller expects, and nothing downstream can tell the answer apart from a real one. That is the half of #11407 worth fixing — the other symptom in that issue already fails loudly.

A value that fails to parse one line above already aborts the query (parse_date_str(...)?). This makes a value that parses but cannot be represented behave the same way, and quotes the representable range so the error says what to do about it. pg-srv already rejects out-of-range timestamps on the wire with Timestamp out of range: '{}', so the scan path is now consistent with it.

Compatibility

This turns a silent NULL into a hard error, and 9999-12-31 end-of-time sentinels in SCD-2 dimension tables are common enough that some users will notice. That is the intent — today they read NULL and have no way to tell — but if you would rather ship it behind a config flag, or hold it for the Timestamp(Microsecond) migration that the // TODO switch parsing to microseconds above this branch points at, say the word and I will rework it.

Worth flagging: test_df_cube_scan_execute asserted the old behaviour. Its fixture fed 9999-12-31 00:00:00.000 to the timestamp column and expected None back, while the Date32 column in the same fixture round-tripped the same date fine. That fixture value now uses an in-range timestamp near the upper bound, and the out-of-range cases (below and above the range) get a dedicated test.

Failure shape

The reasoning above covers the batch path. On the native streaming path the same error is raised per chunk and thrown at packages/cubejs-backend-native/src/stream.rs:275, so with CUBESQL_STREAM_MODE=true a client can receive several chunks and then the error — a partial result followed by a failure. Still better than silently wrong data, but a distinct shape that the tests do not cover.

Any query projecting an affected dimension now fails, including a BI tool's opening SELECT * … LIMIT 100, and it fails after the source database has already done the work. COUNT(*) over the same cube is unaffected. Users are not cornered, which is the main reason I think the hard error is shippable: the same value still succeeds as Date32 and as Timestamp(Millisecond) (scan.rs:1114), so SELECT d::date keeps working where SELECT d now errors, and clamping the sentinel in the data model is a second way out. Both are in the docs note.

Verification

cargo fmt --all -- --check
cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings
cargo test -p cubesql --lib     # 770 passed, 0 failed

[ovr]

/bot-deep-review

[claude]

Claude finished @ovr's task in 7m 48s —— View job

---

Deep review complete — no correctness bugs found; 1 medium, 2 low, plus 2 items needing a maintainer decision (compat flag, docs). 3 inline comments posted.

Full review — click to expand

• Read PR diff
• Read surrounding scan.rs context (macro expansion, error propagation)
• Verify the return Err actually propagates out of transform_response
• Check every other timestamp/date conversion path for consistency
• Trace how the error reaches the client (batch path, native path, streaming path)
• Verify the boundary constants and the changed fixture arithmetic
• Review test coverage
• Assess compatibility / behavior-change risk
• Check existing review threads for duplicates (none — no prior threads on this PR)
• Post review

Verdict

The diagnosis is right and the fix is in the right place. A silent append_null() on a value that exists is the worst possible outcome here: MIN(ts) returns NULL over a column with no NULLs, the row count and schema are exactly what the caller expects, and nothing downstream can distinguish it from a real answer. Failing loudly is correct, and it lines the scan path up with pg-srv, which already rejects out-of-range timestamps on the wire (pg-srv/src/values/timestamp.rs:187).

Mechanics check out:

• transform_response_body! expands directly into transform_response's body (scan.rs:1267), which returns Result<RecordBatch, CubeError> — the bare return Err(...) propagates to the right frame, not into a closure. transform_response and convert_transport_response are the only two consumers, so every response path (HTTP standalone transport/service.rs:344, native transport.rs:527/539, native streaming stream.rs:273) picks this up. No second silent-NULL site was left behind: a repo-wide sweep for timestamp_nanos_opt shows the remaining call sites are UDFs, filter rewrites (.ok()?, intentionally lossy for rewrite matching) and pg_prepared_statements, none of them on the response-materialisation path.
• Both bound strings are exactly i64::MIN/i64::MAX nanoseconds, inclusive, and the changed fixture value is right: 2262-04-11 00:00:00Z = 9,223,286,400 s = 9223286400000000000 ns, 23h47m16s under the ceiling. Retiring the old fixture — which asserted 9999-12-31 → None while the Date32 column in the same row round-tripped it fine — was the correct call; that assertion was encoding the bug.
• let-else needs 1.65, workspace is on 1.90. Fine.

Findings

#	Sev	Where	Issue
1	medium	scan.rs:1090-1095	Error names neither the column nor the raw input value — with several time dimensions in a scan the user can't tell which one to fix, and 9999-12-31 00:00:00.000 prints back as 9999-12-31 00:00:00 (re-Displayed NaiveDateTime, not s)
2	low	scan.rs:1090	CubeError::user → SQLSTATE 26000 invalid_sql_statement_name; post_processing → 22000 data_exception fits a bad data value better (Postgres uses 22008). Largely cosmetic — load_data overwrites the cause with DatabaseExecution at scan.rs:824
3	low	scan.rs:1518-1526	contains("Timestamp out of range") is loose; nothing pins the two boundary constants. Suggest asserting the offending value, plus inclusive-boundary success cases and ±1ns failures

Two things that need your call, @ovr — not the author's

1. The failure is all-or-nothing, and one path fails mid-stream.

The PR description reasons about the batch path only. On the native streaming path the same error is raised per chunk in js_stream_push_chunk and thrown as a JS error (packages/cubejs-backend-native/src/stream.rs:275), so with CUBESQL_STREAM_MODE=true a client can receive several chunks of rows and then an error — partial result plus failure, rather than a clean rejection. Still better than silently wrong data, but it's a distinct failure shape that neither the description nor the tests cover (the new test exercises convert_transport_response, i.e. the batch path).

Blast radius, concretely: any query projecting an affected dimension now fails, including a BI tool's opening SELECT * … LIMIT 100, and the failure lands after the source DB has already done the work. COUNT(*)-style queries that don't scan the column are unaffected.

One data point that argues for shipping it: users are not cornered. The same 9999-12-31 succeeds as Date32 (num_days_from_ce → 2_932_896, still in the fixture) and as Timestamp(Millisecond, None) (infallible timestamp_millis(), scan.rs:1114). So SELECT d::date keeps working where SELECT d now errors, and clamping the sentinel in the data model is a second workaround. That's a real remediation path, which is exactly what finding #1 asks the message to state.

My read: ship it as a hard error rather than behind a flag — a config flag that defaults to "silently return wrong answers" mostly preserves the bug for the people who most need to know about it, and the Timestamp(Microsecond) migration the // TODO points at would widen the window to ±294k years and retire this entirely, so a flag would be short-lived scaffolding. But the SCD-2 9999-12-31 pattern is common enough that this will produce support traffic, so if you'd rather not absorb that in a patch release, the author already offered to gate it.

2. Docs / changelog.

The docs checkbox is unchecked and this is user-visible: a query that returned rows yesterday errors today. Worth a short note in docs-mintlify (SQL API limitations / known issues) naming the representable range and the ::date workaround, so the error is searchable. That's a reasonable ask of the author before merge.

Non-blocking observation

The Timestamp(Nanosecond) arm is now the only one in transform_response_body! that hard-fails on a value it can't represent. Float32/Float64 still warn! + null on unparseable input (scan.rs:1012, 1037), Boolean log::error!s + nulls anything that isn't true/1/false/0 (scan.rs:1063), and Date32 nulls on parse failure (scan.rs:1138). The "the client cannot tell this apart from a real NULL" argument applies verbatim to all four. Not this PR's job — but the same issue exists three more times in the same macro and would make a clean follow-up.

Security / performance

Nothing to flag. No new allocation on the hot path (the format! only runs on the error branch), no unsafe, no input reaching a sink. The benches/transform_response.rs fixture generates 2024 dates only, so the benchmark is unaffected.

What I could and couldn't verify

Static review plus targeted reads; I did not compile or run the suite locally (the DataFusion fork build is long-running on this runner), so I'm taking the author's cargo test -p cubesql --lib / clippy / fmt results at face value. I did confirm no other lib-test fixture or insta snapshot in rust/cubesql feeds an out-of-range timestamp that this could newly break — the only other 9999- occurrence is date_parser.rs:101, which tests parsing only. CI at review time: Build, Build native and Drivers tests still in progress; the one red run is the review bot's own job, not the code.

· branch [`fix/cubesql-out-of-range-timestamp-silent-null`](https://github.com/cube-js/cube/tree/fix/cubesql-out-of-range-timestamp-silent-null)

[claude]

medium — the error doesn't say which column, and reformats the value the source sent

Now that this aborts the whole query, the message is the only diagnostic the user gets. Two things it drops:

1. No column name. A scan over a cube with several time dimensions produces Timestamp out of range: 9999-12-31 00:00:00 and nothing else — the user has to guess which dimension to fix. schema_field is in scope here: the Decimal arm below already reaches precision/scale bound by transform_response_body's match arm through the same two macro layers, so identifiers from that context resolve inside $builder_block.
2. timestamp is the re-Displayed NaiveDateTime, not the input. "9999-12-31 00:00:00.000" prints back as 9999-12-31 00:00:00, so the string in the error doesn't match what's in the source data / a grep of the response. s is still live (parse_date_str(s.as_ref()) only borrows), and printing it also matches pg-srv's wording, which quotes the raw text: Timestamp out of range: '{}' (pg-srv/src/values/timestamp.rs:187).

Suggested change

	return Err(CubeError::user(format!(
	"Timestamp out of range: {}. Timestamps are \
	represented with nanosecond precision and must be \
	between {} and {}",
	timestamp, MIN_NANOSECOND_TIMESTAMP, MAX_NANOSECOND_TIMESTAMP
	)));
	return Err(CubeError::user(format!(
	"Timestamp out of range in column {:?}: '{}'. \
	Timestamps are represented with nanosecond \
	precision and must be between {} and {}",
	schema_field.name(),
	s,
	MIN_NANOSECOND_TIMESTAMP,
	MAX_NANOSECOND_TIMESTAMP
	)));

Worth considering a closing hint too, since the error is now terminal — e.g. "cast the dimension to date, or clamp the sentinel in the data model" — so the message says what to do, not just what the range is. This matters most on the streaming path, where the failure lands mid-stream with no row/chunk context at all.

Fix this →

[claude]

low — CubeError::user maps to SQLSTATE 26000; post_processing (22000) fits a bad data value better

cube_to_error_response (sql/postgres/error.rs:69) maps CubeErrorCauseType::User → ErrorCode::InvalidSqlStatement → 26000 invalid_sql_statement_name (pg-srv/src/protocol.rs:1040). The SQL statement is fine here; a value in the source data can't be represented. Postgres itself raises 22008 datetime_field_overflow for this, and the closest cause available is CubeErrorCauseType::PostProcessing → DataException → 22000 — so CubeError::post_processing(...) is the more honest classification for a client that branches on SQLSTATE class.

Two caveats that make this cosmetic rather than urgent, both worth knowing before you decide:

• On the transport path the cause is overwritten anyway: load_data's map_err sets err.cause = CubeErrorCauseType::DatabaseExecution(...) (scan.rs:824), so the client actually sees 58000 system_error regardless of what's chosen here.
• On the native streaming path only the message survives — js_stream_push_chunk does cx.throw_error(e.message) (packages/cubejs-backend-native/src/stream.rs:275), dropping the cause entirely.

So either choice works today; post_processing just won't be wrong if the DatabaseExecution override is ever narrowed. Also note the neighbouring parse_date_str failure this is modelled on ends up as DataFusionError::Internal → an internal error, so "behaves the same way" holds for control flow but not for classification.

[claude]

low — assertion is loose, and nothing pins the two boundary constants

contains("Timestamp out of range") passes even if the message names the wrong value or the bounds go stale. Two cheap additions:

1. Assert the offending value (and, if you take the suggestion above, the column name) actually appears — that's the part a user needs and the part most likely to regress.
2. Add the inclusive boundaries as success cases, so MIN_NANOSECOND_TIMESTAMP / MAX_NANOSECOND_TIMESTAMP are covered by a test rather than by a comment:
   • 1677-09-21 00:12:43.145224192 → Some(i64::MIN)
   • 2262-04-11 23:47:16.854775807 → Some(i64::MAX)
   • one nanosecond outside either end → error

I checked both constants against timestamp_nanos_opt's arithmetic and they are exactly i64::MIN/i64::MAX nanoseconds (the negative end is representable because chrono rebalances timestamp + 1 / subsec - 1_000_000_000 before the multiply, so the bound is inclusive, unlike pandas which reserves i64::MIN for NaT). Note the sub-second precision only survives the %.f chrono fallback, not parse_fast — which needs the T separator — so these cases exercise a different parse path than the existing fixtures, which is a bonus.

The updated test_df_cube_scan_execute fixture (2262-04-11 00:00:00.000 → 9223286400000000000) is correct — 23h47m under the ceiling — and keeping 9999-12-31 in the Date32 column so both behaviours sit side by side in one fixture is a nice touch.

[ovr]

@marciodfg, thank you for submitting the PR, i've triggered our review bot. Could you review the comments from it? Thanks

[marciodfg]

Thanks for running the bot — all three comments are addressed in 0895a8f:

• Column name and value — the error now reads Timestamp out of range for column 'valid_to': '9999-12-31 00:00:00.000'. … and closes with the "read it as a date" hint.
• Classification — switched to post_processing. Cosmetic today since load_data overwrites the cause at scan.rs:824, but 22000 is the honest one for a value that can't be represented.
• Tests — the assertions now pin the column name and the offending value, and cover one nanosecond outside each end; a new test pins the inclusive bounds to i64::MIN / i64::MAX.

Also added the docs note (SQL API → Limitations → Timestamp range) and updated the description with the streaming-path failure shape.

On the red checks: I reproduced tests (postgres, false) locally against an image built from this branch — 119 passed, 0 failed, including every SQL API: case, which are the ones that exercise this code. Worth knowing that every green cloud driver cell was green because it was skipped — fork PRs get no secrets — so only 16 cells actually ran and 13 of those failed, each using all three retry attempts. Could you re-run?