perf(cubeorchestrator): Drop DBResponseValue wrapper (-15% transform)

[ovr]

DBResponseValue had three variants but only Primitive was ever constructed in production. DateTime and Object existed only in tests (the latter even carried // TODO: Is this variant still used?). The wrapper added per-cell discriminant + alignment overhead and a dead match arm in the transform_value hot loop.

TransformedData::transform columnar path:

bench	before	after	Δ
columnar/c16_r10000	2.62 ms	2.26 ms	−13.7%
columnar/c16_r50000	17.13 ms	13.89 ms	−18.9%
columnar/c16_r100000	46.05 ms	38.45 ms	−16.5%
columnar/c32_r10000	5.89 ms	4.81 ms	−18.3%
columnar/c32_r50000	30.24 ms	26.84 ms	−11.2%
columnar/c64_r100000	121.24 ms	108.39 ms	−10.6%

Real-shape scenarios (c16×100k):

scenario	before	after	Δ
columnar / no_time_dim	42.13 ms	32.12 ms	−23.8%
columnar / one_time_dim_day	87.41 ms	74.33 ms	−15.0%
columnar / one_time_dim_custom_granularity	93.79 ms	79.62 ms	−15.1%
columnar / two_time_dims	132.90 ms	115.98 ms	−12.7%
vanilla / one_time_dim_day	161.03 ms	146.46 ms	−9.1%
vanilla / two_time_dims	222.80 ms	206.43 ms	−7.3%

[claude]

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

---

Code Review: PR #10844

• Gather context and read changed files
• Review query_result_transform.rs (main changes)
• Review query_message_parser.rs (parsing changes)
• Review orchestrator.rs (native bridge changes)
• Review responses.ts (TypeScript type changes)
• Check for remaining references to removed types
• Post final review

---

Verdict: This is a clean, well-motivated removal of dead code. Looks good to merge. ✅

Detailed review

Correctness

The change is safe. DBResponseValue had three variants — Primitive, DateTime, and Object — but only Primitive was ever constructed in production code:

• The flatbuffers parser (query_message_parser.rs) only ever created DBResponseValue::Primitive(...).
• The columnar path (from_columnar) only wrapped DBResponsePrimitive values into DBResponseValue::Primitive(value).
• DateTime and Object { value } were only exercised in unit tests. The Object variant even had a // TODO: Is this variant still used? comment.

After this PR, all sites now work directly with DBResponsePrimitive, eliminating the wrapper. I verified:

1. Zero remaining references to DBResponseValue anywhere in the Rust or TypeScript codebase — clean removal.
2. transform_value behavior is preserved. The only production-reachable path was DBResponseValue::Primitive(DBResponsePrimitive::String(s)) with type_ == "time", which is now DBResponsePrimitive::String(ref s) with type_ == "time" — identical logic. The catch-all previously mapped Primitive(p) → p; now it's other → other, same result.
3. orchestrator.rs (get_cubestore_result) is behaviorally identical. Previously matched DBResponseValue::Primitive(DBResponsePrimitive::Null) → null, else other.to_string(). The old Display for DBResponseValue::Primitive(p) delegated to p.to_string(), so the new direct DBResponsePrimitive.to_string() produces the same output.
4. Serde deserialization — DBResponsePrimitive already has #[derive(Deserialize)] with #[serde(untagged)], so any JSON that previously deserialized into DBResponseValue::Primitive(x) will now deserialize directly into x.

Performance

The PR description's benchmark numbers are credible. Removing the enum wrapper eliminates:

• Per-cell discriminant + alignment overhead (the DBResponseValue enum was larger than DBResponsePrimitive due to the DateTime<Utc> variant, which is 12 bytes).
• A dead match arm in the hot transform_value loop.
• One layer of indirection in clone/match operations across all row/column processing paths.

Testing

The removed tests (test_transform_value_datetime_to_time, test_transform_value_datetime_empty_type, test_transform_value_object, test_transform_value_fallback_to_null) all tested the now-removed variants. The remaining tests cover the actual production path (String → time formatting with various input formats, primitive pass-through). Test coverage is adequate.

TypeScript side

The DBResponseValue type in responses.ts was removed. This type was not re-exported from the package's public API entry point, so there's no breaking change for downstream consumers. TransformDataResponse and DBResponsePrimitive (the types that are actually used) remain untouched.

Nit (non-blocking)

The chrono::TimeZone import in query_result_transform.rs (line 9) — is TimeZone still needed after removing the DateTime variant? It's used by Utc.from_utc_datetime(...) inside transform_value, so yes, it's still needed. Just double-checked.

No issues found. Nice cleanup. 👍

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.47%. Comparing base (8d8d2a9) to head (f9e5e5b).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10844      +/-   ##
==========================================
+ Coverage   78.84%   83.47%   +4.63%     
==========================================
  Files         470      254     -216     
  Lines       92274    75350   -16924     
  Branches     3433        0    -3433     
==========================================
- Hits        72751    62901    -9850     
+ Misses      19020    12449    -6571     
+ Partials      503        0     -503     

Flag	Coverage Δ
cube-backend	?
cubesql	83.47% <ø> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.