feat(cubeorchestrator): Use Arrow format for CubeStore response format

[ovr]

Add QueryResult::from_arrow to decode Arrow IPC stream payloads into the columnar QueryResult, and route the HttpQueryResult (Arrow) command in from_cubestore_fb to it. Arrow temporal columns map to a new dedicated DBResponsePrimitive::Timestamp variant that serializes to the existing ISO-8601 millisecond format.

BREAKING CHANGE: all numeric values in query results are now serialized as JSON strings regardless of the data source driver. For a long time, Cube returned inconsistent JSON types for numeric values depending on the driver — sometimes strings, sometimes numbers. The wire type of a value should not depend on the data source, so we standardize on strings. Clients that relied on receiving JSON numbers must parse the string values instead (front-end SDKs already provide the castNumerics option for this).

Fix #1879

[claude]

Claude finished @ovr's task in 4m 41s —— View job

---

Code Review: Arrow Format for CubeStore Response

• Gather context and understand changes
• Review Rust core changes (query_message_parser.rs, query_result_transform.rs)
• Review Rust benchmarks and Cargo config
• Review TypeScript codegen changes
• Review CubeStore driver changes
• Review backend-native changes
• Post comprehensive review

---

Summary

Well-structured PR that adds Arrow IPC stream decoding into the existing columnar QueryResult, with a new DBResponsePrimitive::Timestamp variant for zero-copy temporal data. The implementation is clean and the test coverage is solid. I've posted inline comments on the specific issues — here's the high-level breakdown:

Issues Found

1 High-severity bug, 2 medium items, rest are low/informational.

Severity	File	Issue
High	CubeStoreDriver.ts:91-98	this.config.responseFormat has no default value — toResponseFormat(undefined) will throw for every driver instance that doesn't explicitly set it. ConnectionConfig type also missing the field.
Medium	query_message_parser.rs:296-306	All macros (push_numeric!, push_str!, etc.) use .unwrap() on downcast_ref — will panic instead of returning an error on unexpected array types.
Medium	CubeStoreDriver.ts:118-125	WebSocketConnection.query() signature changed from positional args to options object — verify no external callers use the old signature.
Low	query_message_parser.rs:373-378	Int64/UInt64 → f64 cast loses precision for values > 2^53. Consistent with existing behavior but worth noting for Arrow path where integers arrive as native types, not strings.
Low	orchestrator.rs:140	Timestamp → FieldValue goes through Display instead of directly using TIMESTAMP_FORMAT — fragile coupling if Display impl ever diverges.

What's done well

• DBResponsePrimitive::Timestamp fits within the existing 32-byte enum size (verified by the new test_db_response_primitive_size guard test) — no memory regression
• Hand-written Serialize correctly replaces #[serde(untagged)] derive and handles the Timestamp → ISO-8601 string rendering with zero overhead for common variants
• Arrow type coverage is comprehensive (all numeric types, strings, booleans, timestamps, decimals, dates) with a clear error for unsupported types
• Backward-compatible responseFormat default in WebSocketConnection — old CubeStore ignores the field
• Benchmarks properly mirror the existing from_js_raw_data combos for apples-to-apples comparison
• Test coverage includes basic types, unsupported types, and full round-trip through from_cubestore_fb with Arrow payload

Testing gaps

• No tests for Timestamp pass-through in transform_value (Arrow timestamps bypass the string-parsing path for type_ == "time")
• No test for Decimal128/Decimal256 Arrow columns (the push_decimal! macro includes a parse::<f64> fallback to string — would be good to verify both paths)
• No integration test exercising the full TypeScript → Rust → transform pipeline with Arrow format

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.30%. Comparing base (af5478a) to head (e63d5e7).

Additional details and impacted files

@@                 Coverage Diff                  @@
##           feat/release-v1.7.0   #10949   +/-   ##
====================================================
  Coverage                79.30%   79.30%           
====================================================
  Files                      472      472           
  Lines                    96880    96880           
  Branches                  3539     3539           
====================================================
  Hits                     76826    76826           
  Misses                   19548    19548           
  Partials                   506      506           

Flag	Coverage Δ
cube-backend	59.21% <ø> (ø)
cubesql	83.70% <ø> (ø)

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 @ovr's task in 4m 13s —— View job

---

Code Review: Arrow Format for CubeStore Response (Re-review after 6 commits)

• Gather context and review updated diff
• Check previous review threads and resolve stale ones
• Review Rust core changes (query_message_parser.rs, query_result_transform.rs)
• Review TypeScript changes (CubeStoreDriver, WebSocketConnection, codegen)
• Review backend-native orchestrator changes
• Review benchmarks and Cargo config
• Post comprehensive review

---

Summary

Good iteration on the previous review. The follow-up commits addressed the key robustness concerns: downcast macros now use ok_or_else instead of .unwrap() (commit b14653e), DBResponsePrimitive numeric variants are properly split into Int64/UInt64/Float64 (commit 8826169), and timestamp formatting is optimized with pre-computed Item slices (commit 38ab4f2). I've resolved the 5 stale review threads that were addressed.

Two new issues found in this pass:

Issues Found

Severity	File	Issue
Medium	CubeStoreDriver.ts:121	|| operator silently overrides explicit QueryResultFormat.Legacy (value 0, falsy) to Arrow. Should use ??.
Medium	query_message_parser.rs:262	from_arrow is private but benches/parser.rs calls it from external benchmark code — won't compile.

What improved since last review

• Downcast safety: All macros now use ok_or_else → ParseError::ArrowError instead of .unwrap() panics
• Numeric fidelity: Int64/UInt64/Float64 split preserves integer precision through the pipeline until the JS boundary
• Timestamp formatting: TIMESTAMP_ITEMS const slice avoids re-parsing the format string on every cell — measurable win for large timestamp columns
• Display consistency: Both Display and Serialize impls for Timestamp now use the same TIMESTAMP_ITEMS, eliminating the fragile coupling concern

Testing observation

transform_value only reformats DBResponsePrimitive::String values when type_ == "time". Arrow timestamps arrive as Timestamp(NaiveDateTime) and pass through unchanged (via the other => other fallback). This is correct because the Timestamp variant already serializes to ISO-8601 — but there are no tests exercising this path. Consider adding a test that passes a Timestamp value through transform_value with type_ = "time" to document this intentional bypass.

---

[claude]

Claude finished @ovr's task in 2m 53s —— View job

---

Code Review: Arrow Format for CubeStore Response (Re-review after 8 commits)

• Gather context and review updated diff
• Check previous review threads and resolve stale ones
• Review Rust core changes (query_message_parser.rs, query_result_transform.rs)
• Review TypeScript changes (CubeStoreDriver, WebSocketConnection, codegen)
• Review backend-native orchestrator changes
• Review benchmarks and Cargo config
• Post comprehensive review

---

Summary

Good iteration. The latest commit (7eab593) fixes the || → ?? bug flagged in the previous review. I've resolved 6 stale threads (the || fix, plus positive/informational comments that don't need author action). One actionable issue remains from the prior pass.

Remaining Issue

Severity	File	Issue
Medium	query_message_parser.rs:262	from_arrow is still private but called from benches/parser.rs:187 — benchmark won't compile. Needs pub fn.

This was flagged in the previous review and is still present. I've posted an inline comment with a suggested fix.

What was resolved since last review

• || → ?? fix (commit 7eab593): responseFormat ?? QueryResultFormat.Arrow now correctly preserves an explicit QueryResultFormat.Legacy (value 0, falsy). Resolved the stale thread.
• Resolved 5 positive/informational comment threads that didn't require author action.

Overall Assessment

The PR is in good shape. The Arrow IPC decoding path is well-implemented with comprehensive type coverage, safe downcast error handling, and proper backward compatibility. The single remaining issue (pub visibility on from_arrow) is a straightforward one-line fix.

---

[claude]

Claude finished @ovr's task in 2m 31s —— View job

---

Code Review: Arrow Format for CubeStore Response (Re-review after sync)

• Gather context and review updated diff
• Check previous review threads and resolve stale ones
• Review Rust core changes (query_message_parser.rs, query_result_transform.rs)
• Review TypeScript changes (CubeStoreDriver, WebSocketConnection, codegen)
• Review backend-native orchestrator changes
• Review benchmarks and Cargo config
• Post comprehensive review

---

Summary

The branch was rebased (6 commits, down from 8 in the prior review). The good improvements from earlier rounds are preserved — safe downcast handling (ok_or_else), split numeric variants (Int64/UInt64/Float64), and optimized timestamp formatting (TIMESTAMP_ITEMS). However, the || → ?? fix from commit 7eab593 was lost in the force-push. One prior issue also remains unfixed.

Re-affirmed 2 prior unresolved threads that still apply (both on from_arrow visibility). All 11 previously resolved threads remain correctly resolved — changes still addressed.

Issues Found

Severity	File	Issue
Medium	CubeStoreDriver.ts:121	|| operator silently overrides explicit QueryResultFormat.Legacy (value 0, falsy) to Arrow. Should use ??. Fix was previously committed but lost in force-push.
Medium	query_message_parser.rs:262	from_arrow is still private (fn) but benches/parser.rs:187 calls it from external benchmark code. Benchmarks compile as separate crates — this will fail cargo bench.

Both have inline comments with suggested fixes.

What's in good shape

• Downcast safety: All macros use ok_or_else → ParseError::ArrowError instead of .unwrap() panics
• Numeric fidelity: Int64/UInt64/Float64 split preserves integer precision through the pipeline
• Timestamp formatting: TIMESTAMP_ITEMS const slice avoids re-parsing the format string on every cell
• Hand-written Serialize: Zero-overhead for common variants, correct ISO-8601 rendering for Timestamp
• Size guard test: test_db_response_primitive_size catches accidental enum layout growth
• Arrow type coverage: Comprehensive (all numeric, string, boolean, temporal, decimal types) with clear UnsupportedArrowType error for unsupported types
• Test coverage: Basic types, unsupported types, and full round-trip through from_cubestore_fb with Arrow payload
• Codegen: Auto-generated FlatBuffer bindings look correct (HttpQueryResult, HttpQueryResultArrow, QueryResultFormat)
• Backward compatibility: WebSocketConnection defaults to Arrow; old CubeStore ignores the field gracefully
• No breaking external callers: CubeStoreCacheDriver calls CubeStoreDriver.query() (unchanged signature), not WebSocketConnection.query() directly

Testing observation

transform_value only reformats DBResponsePrimitive::String values when type_ == "time". Arrow timestamps arrive as Timestamp(NaiveDateTime) and pass through via the other => other fallback. This is correct — the Timestamp variant already serializes to ISO-8601 — but there are no tests exercising this path. A test passing a Timestamp through transform_value with type_ = "time" would document this intentional bypass.

---

[claude]

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

---

No new issues found. PR remains in good shape.

Re-review after sync — 2 new commits since last review (21844e4, 0823a00)

• Fetched diff vs origin/feat/release-v1.7.0 and inspected new commits
• Verified all 19 prior review threads remain resolved
• Reviewed each new commit for correctness and consistency

New since last review

21844e4 — chore: tmp (query_message_parser.rs +36/−46)

Reverses the earlier Decimal32/64 support (7e1577f / 391e9a5). The two match arms are collapsed and now return ParseError::UnsupportedArrowType with an explanatory message.

The comment explains the root cause well: CubeStore's arrow-rs fork (cube-js/arrow-rs#48) advertises bitWidth: 32/64 in the IPC schema as a backward-compatibility signal for decimals with precision ≤ 27, but the buffers underneath still contain 16-byte Decimal128 values. Parsing them via Decimal32Array/Decimal64Array would use a 4- or 8-byte stride and silently interleave real values with zeros — a data-corruption footgun. CubeStore's normalize_decimals_for_ipc widens all decimals to precision 38 before writing the IPC stream, so this branch should be unreachable on healthy paths; failing loudly rather than parsing garbage is the correct choice.

The renamed test test_from_arrow_rejects_decimal32_and_decimal64 correctly asserts the failure path and pins the error variant.

Verified:

• The push_decimal! macro's stale comment ("handles Decimal32/64/128/256") was updated to just Decimal128/256 — consistent with the new match arms.
• No other call sites reference Decimal32Array / Decimal64Array outside the test.
• Error message contains "Decimal32" (the test's assertion substring), which works because format!("{:?}", DataType::Decimal32(9,2)) starts with Decimal32.
• Combined arm DataType::Decimal32(_, _) | DataType::Decimal64(_, _) uses array.data_type() for the actual discriminant in the error — precise reporting.

0823a00 — chore: bump minimal cube.js version (CubeStoreDriver.ts +1/−2)

Bumps arrowFormat capability floor from 1.6.55 → 1.6.66. Presumably matches the CubeStore release that ships normalize_decimals_for_ipc — clients talking to older CubeStores now stay on Legacy, avoiding the Decimal32/64 rejection path entirely.

Verified:

• Single-value change in the CubeStoreCapabilityMinVersion object; hasCapability('arrowFormat') gate at the constructor's ?? default remains intact.
• Stale two-line comment ("Arrow format + Completed response type") was dropped — fine, the version is self-documenting via the map key.
• No other references to 1.6.55 or arrowFormat need updating.

Everything else remains verified

All 19 prior review threads are still correctly resolved. Every fix from prior rounds is present in the current diff:

• Capability-gated Arrow default (??, no ||)
• Downcast safety via ok_or_else → ParseError::ArrowError
• Split Int64/UInt64/Float64 variants, rendered as JSON strings (collect_str)
• Shared TIMESTAMP_ITEMS between Display and Serialize
• 32-byte enum size guard
• Custom decimal_to_string handles i32/i64/i128/i256 uniformly and strips trailing fractional zeros
• Variable-length fraction parse (%.f) for chrono 0.4.42 compatibility
• HttpQueryResultCompleted → QueryResult::empty() for no-result-set statements
• Benchmarks route through public from_cubestore_fb (so from_arrow staying pub(crate) is correct)
• No breaking external callers of WebSocketConnection.query()

Testing observation (unchanged from prior rounds)

transform_value only reformats DBResponsePrimitive::String when type_ == "time"; Timestamp(NaiveDateTime) values pass through the other => other fallback and rely on the dedicated Serialize impl. Correct but not directly tested — a single assertion would document the intentional bypass. Non-blocking.

Release-notes reminder (from earlier rounds)

Wire format for numeric values (measures and dimensions) is now JSON strings rather than JSON numbers in v1.7.0. Callers doing typeof val === 'number' or arithmetic without coercion will break. Worth a prominent call-out in the changelog.

• [Branch](https://github.com/cube-js/cube/tree/perf/query-orchestrator-arrow-results)

[claude]

Claude finished @ovr's task in 33s —— View job

---

No new issues found. PR remains in good shape.

Re-review after sync — new commit 2a4fa33 (driver test snapshot updates)

• Fetched diff vs origin/feat/release-v1.7.0 and inspected the new commit
• Verified all prior review threads remain resolved
• Reviewed the single new commit since the last review pass

New since last review

2a4fa33 — chore(cubejs-testing-drivers): update snapshots for Arrow numeric string serialization (+4799/−4799, tests only)

Bulk snapshot refresh across 13 driver test snapshot files (Databricks JDBC ×7, MySQL, Oracle, Postgres ×2, Redshift ×2). All changes are numeric measure/dimension values being re-quoted as JSON strings — the fourth batch of downstream fallout from commit 3bef5c0 (Serialize numerics via collect_str), following the earlier 86baced/831ebcc/64d165b snapshot updates.

Verified:

• Only .snap files touched under packages/cubejs-testing-drivers/test/__snapshots__/; no production code changes.
• Symmetrical +/− counts confirm this is a re-serialization of the same test data with the new wire shape, not a semantic change.
• Consistent with the story from commits 3bef5c0 and 2e3ef7a: numeric primitives and decimals both go over the wire as JSON strings.

No new issues. Everything from prior reviews remains in good shape.

Release-notes reminder (unchanged from prior rounds): In v1.7.0 the CubeStore Arrow response path renders numeric measure/dimension values as JSON strings rather than JSON numbers. Downstream consumers using typeof val === 'number' or arithmetic without coercion will break — worth a prominent call-out in the changelog. Decimal columns additionally strip trailing fractional zeros (e.g. 1.5, not 1.500) per the dad214d/2e3ef7a formatter.

• Branch •

[claude]

Claude finished @ovr's task in 35s —— View job

---

No new issues found. PR remains in good shape.

Re-review after sync — new commit e901431 (driver-postgres snapshot update)

• Fetched diff vs origin/feat/release-v1.7.0 and inspected new commit
• Verified all prior review threads remain resolved
• Reviewed the single new commit since the last review pass

New since last review

e901431 — chore(cubejs-testing): update driver-postgres snapshots for Arrow numeric string serialization (packages/cubejs-testing/test/__snapshots__/driver-postgres.test.ts.snap +950/−950)

Test-only update. Fifth batch of downstream snapshot refreshes tracking the wire-format change from commit 33d840e (numeric variants Serialize via collect_str). Symmetrical +/− counts confirm this is a re-serialization of the same test data with the JSON-string wire shape — no semantic changes.

Verified:

• Only the driver-postgres snapshot file touched; no production code.
• Consistent with the earlier 86baced, 831ebcc, 64d165b, and c50c691 snapshot updates.
• The Arrow → JSON-string story now covers: cubejs-api-gateway tests, smoke-trino, birdbox-postgresql, smoke-rbac, 13 driver test snapshots, and now driver-postgres — the migration to the new wire shape is comprehensive across the test suite.

No new issues. Everything from prior reviews remains in good shape (capability-gated Arrow default ≥ 1.6.66, ?? fix, downcast safety via ok_or_else, TIMESTAMP_ITEMS shared between Display/Serialize, split numeric variants rendered as JSON strings, size guard at 32 bytes, Completed payload handling, variable-length fraction parsing, Decimal128/256 support with custom decimal_to_string formatter, Decimal32/64 correctly rejected due to CubeStore's bitWidth compatibility hack, benchmark refactor through from_cubestore_fb).

Release-notes reminder (unchanged from prior rounds): v1.7.0's CubeStore Arrow response path renders numeric measure/dimension values as JSON strings rather than JSON numbers. Callers doing typeof val === 'number' or arithmetic without coercion will break — worth a prominent call-out in the changelog. Decimal columns additionally strip trailing fractional zeros (e.g. 1.5, not 1.500).

• Branch •

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude finished @ovr's task in 56s —— View job

---

No new issues found. PR remains in good shape.

Re-review after squash (2 commits: 8dd2d21, dbf0029)

• Fetched diff vs origin/feat/release-v1.7.0 and inspected commits
• Verified all prior review fixes are present in the squashed history
• Re-checked Rust core, TypeScript, codegen, and backend-native

State of the branch

The branch was squashed/rebased down to just 2 commits atop the base — 8dd2d21 (Arrow format for CubeStore response) and dbf0029 (serialize numerics as JSON strings). All accumulated fixes from the previous 15+ commits of iteration are folded into these two commits.

Verified end-to-end

Area	Location	Verdict
?? default for responseFormat	CubeStoreDriver.ts:120	Nullish coalescing preserved; capability-gated (Arrow only for CubeStore ≥ 1.6.66)
Downcast safety	query_message_parser.rs:347	ok_or_else → ParseError::ArrowError (no .unwrap())
from_arrow visibility	query_message_parser.rs:279	pub(crate) — correct; benchmarks use public from_cubestore_fb
Numeric fidelity + wire shape	query_result_transform.rs:1196-1201	Int64/UInt64/Float64/Timestamp all render via collect_str — legacy string wire shape preserved
Timestamp formatting	query_result_transform.rs:1169,1201,1317	TIMESTAMP_ITEMS const shared by Display + Serialize
Size guard	query_result_transform.rs:1391	32-byte enum layout still enforced
Completed payload	query_message_parser.rs:218	HttpQueryResultArrow handled; HttpQueryResultCompleted → QueryResult::empty()
Decimal handling	query_message_parser.rs	Custom decimal_to_string for i32/i64/i128/i256; Decimal32/64 rejected due to CubeStore bitWidth compat hack
Test coverage	query_message_parser.rs:690-855	Basic types, Decimal256 beyond i128, leading-null decimal, unsupported types, Completed round-trip

Testing observation (unchanged)

transform_value reformats only DBResponsePrimitive::String when type_ == "time"; Timestamp(NaiveDateTime) passes through via other => other and relies on the dedicated Serialize impl. Correct but not directly tested — a single assertion would document the intentional bypass. Non-blocking.

Release-notes reminder

v1.7.0 renders numeric measure/dimension values as JSON strings rather than JSON numbers. Callers doing typeof val === 'number' or arithmetic without coercion will break. Decimal columns strip trailing fractional zeros (e.g. 1.5, not 1.500). Worth a prominent changelog call-out.

• Branch •

[claude]

Claude finished @ovr's task in 33s —— View job

---

No new issues found. PR remains in good shape.

Re-review after sync — 2 commits atop base (no substantive changes since the prior squash review)

• Fetched diff vs origin/feat/release-v1.7.0
• Verified branch state (8dd2d21, cc7d3d7)
• Re-checked Rust core, TypeScript, codegen, backend-native, tests

State of the branch

The branch is squashed to 2 commits atop the base:

• 8dd2d21 — feat(cubeorchestrator): Use Arrow format for CubeStore response format
• cc7d3d7 — fix(cubeorchestrator)!: serialize all numeric query results as JSON strings

All 15+ commits of iteration are folded into these two. No production-code deltas relative to the last review pass.

Verified end-to-end

Area	Location	Verdict
?? default + capability gate	CubeStoreDriver.ts:120	Arrow only for CubeStore ≥ 1.6.66; explicit Legacy (falsy 0) preserved
Downcast safety	query_message_parser.rs:347	ok_or_else → ParseError::ArrowError (no .unwrap())
from_arrow visibility	query_message_parser.rs:279	pub(crate) — benchmarks route through public from_cubestore_fb
Numeric wire shape	query_result_transform.rs:1196-1201	Int64/UInt64/Float64/Timestamp render via collect_str — legacy string wire shape preserved
Timestamp formatting	query_result_transform.rs:1169,1201,1317	TIMESTAMP_ITEMS const shared by Display + Serialize
Size guard	query_result_transform.rs:1391	32-byte enum layout enforced
Completed payload	query_message_parser.rs:218	HttpQueryResultCompleted → QueryResult::empty()
Decimal handling	query_message_parser.rs	Custom decimal_to_string for i32/i64/i128/i256 (trailing zeros stripped); Decimal32/64 rejected due to CubeStore bitWidth compat
Variable-length fraction parse	query_result_transform.rs	%.f on parse (chrono 0.4.42 compat), %.3f on output
Test coverage	query_message_parser.rs:690-855	Basic types, Decimal256 beyond i128, leading-null decimal, unsupported types, Completed round-trip
Snapshot updates	packages/cubejs-testing*/**	All downstream JSON-string wire-shape snapshots refreshed

Prior threads

All 19 prior review threads from earlier rounds remain correctly resolved against the current diff.

Testing observation (unchanged)

transform_value reformats only DBResponsePrimitive::String when type_ == "time"; Timestamp(NaiveDateTime) passes through via other => other and relies on the dedicated Serialize impl. Correct but not directly tested — a single assertion would document the intentional bypass. Non-blocking.

Release-notes reminder

v1.7.0 renders numeric measure/dimension values (both) as JSON strings rather than JSON numbers. Callers doing typeof val === 'number' or arithmetic without coercion will break. Decimal columns strip trailing fractional zeros (e.g. 1.5, not 1.500). Worth a prominent changelog call-out — this is a breaking wire-format change (which the ! in the commit message reflects).

• Branch •

[claude]

Claude finished @ovr's task in 39s —— View job

---

No new issues found. PR remains in good shape.

Re-review after sync — no substantive changes since last pass

• Fetched diff vs origin/feat/release-v1.7.0 and inspected commits
• Verified all prior review threads remain resolved
• Reviewed current commit set

State of the branch

2 commits atop the base, same content as the prior review:

• 8dd2d21 — feat(cubeorchestrator): Use Arrow format for CubeStore response format
• c84bcd0 — fix(cubeorchestrator)!: serialize all numeric query results as JSON strings (fix Cast all numerical values as strings #1879) — renamed from the prior cc7d3d7 with an expanded commit message referencing Cast all numerical values as strings #1879 and documenting the intentional wire-format break

No code deltas since the last review pass.

Verified end-to-end

Area	Location	Verdict
?? default + capability gate	CubeStoreDriver.ts:120	Arrow only for CubeStore ≥ 1.6.66; explicit Legacy (falsy 0) preserved
Downcast safety	query_message_parser.rs:347	ok_or_else → ParseError::ArrowError (no .unwrap())
from_arrow visibility	query_message_parser.rs:279	pub(crate) — benchmarks route through public from_cubestore_fb
Numeric wire shape	query_result_transform.rs:1196-1201	Int64/UInt64/Float64/Timestamp render via collect_str — legacy string wire shape preserved
Timestamp formatting	query_result_transform.rs:1169,1201,1317	TIMESTAMP_ITEMS const shared by Display + Serialize
Size guard	query_result_transform.rs:1391	32-byte enum layout enforced
Completed payload	query_message_parser.rs:218	HttpQueryResultCompleted → QueryResult::empty()
Decimal handling	query_message_parser.rs	Custom decimal_to_string for i32/i64/i128/i256; Decimal32/64 rejected due to CubeStore bitWidth compat
Variable-length fraction parse	query_result_transform.rs	%.f on parse (chrono 0.4.42 compat), %.3f on output
Snapshot updates	packages/cubejs-testing*/**	All downstream JSON-string wire-shape snapshots refreshed

Prior threads

All 19 prior review threads from earlier rounds remain correctly resolved.

Release-notes reminder

v1.7.0 renders numeric measure/dimension values as JSON strings rather than JSON numbers. Callers doing typeof val === 'number' or arithmetic without coercion will break. Decimal columns strip trailing fractional zeros (e.g. 1.5, not 1.500). The ! in the c84bcd0 commit message plus the new fix #1879 reference correctly flag this as a breaking wire-format change.

• Branch •

[claude]

Claude finished @ovr's task in 39s —— View job

---

No new issues found. PR remains in good shape.

Re-review after sync — no substantive changes since last pass

• Fetched diff vs origin/feat/release-v1.7.0 and inspected commits
• Verified all 19 prior review threads remain resolved
• Reviewed current commit set

State of the branch

2 commits atop the base:

• 8dd2d21 — feat(cubeorchestrator): Use Arrow format for CubeStore response format
• e63d5e7 — fix(cubeorchestrator)!: Serialize all numeric query results as JSON strings, fix Cast all numerical values as strings #1879

No production-code deltas since the last review pass — the tip SHA changed from c84bcd0 to e63d5e7 but the content is equivalent.

Verified end-to-end

Area	Location	Verdict
?? default + capability gate	CubeStoreDriver.ts:120	Arrow only for CubeStore ≥ 1.6.66; explicit Legacy (falsy 0) preserved
Downcast safety	query_message_parser.rs:347	ok_or_else → ParseError::ArrowError (no .unwrap())
from_arrow visibility	query_message_parser.rs:279	pub(crate) — benchmarks route through public from_cubestore_fb
Numeric wire shape	query_result_transform.rs:1196-1201	Int64/UInt64/Float64/Timestamp render via collect_str — legacy string wire shape preserved
Timestamp formatting	query_result_transform.rs:1169,1201,1317	TIMESTAMP_ITEMS const shared by Display + Serialize
Size guard	query_result_transform.rs:1391	32-byte enum layout enforced
Completed payload	query_message_parser.rs:218	HttpQueryResultCompleted → QueryResult::empty()
Decimal handling	query_message_parser.rs	Custom decimal_to_string for i32/i64/i128/i256; Decimal32/64 rejected due to CubeStore bitWidth compat
Variable-length fraction parse	query_result_transform.rs	%.f on parse (chrono 0.4.42 compat), %.3f on output
Snapshot updates	packages/cubejs-testing*/**	All downstream JSON-string wire-shape snapshots refreshed

Prior threads

All 19 prior review threads from earlier rounds remain correctly resolved.

Release-notes reminder

v1.7.0 renders numeric measure/dimension values as JSON strings rather than JSON numbers. Callers doing typeof val === 'number' or arithmetic without coercion will break. Decimal columns strip trailing fractional zeros (e.g. 1.5, not 1.500). The ! in the e63d5e7 commit message plus the fix #1879 reference correctly flag this as a breaking wire-format change.

• Branch •