perf(cubeorchestrator): Migrate QueryResult to columnar storage

[ovr]

QueryResult now stores data column-major (Vec<Vec>, one buffer per column + row_count) instead of row-major (Vec<Vec>, one Vec per row). The Compact, Columnar, and Vanilla transform branches and the JS getCubestoreResult bridge walk the new layout directly. Wire format (TransformedData JSON) and the JS API shape are unchanged.

Why: the Columnar branch had to re-pivot the row-major matrix on every call; with column-major storage that pivot disappears. Per-row Vec headers are also gone (24 B × row_count saved before any data).

Adds benches for QueryResult::from_cubestore_fb and QueryResult::from_js_raw_data alongside the existing transform benches.

Median time delta vs master, lower = faster:

TransformedData::transform/columnar

| cols / rows | 1 000 | 10 000 | 50 000 | 100 000 | | --- | --- | --- | --- | --- |
| 8 | -18.0% | -23.0% | -47.9% | -66.3% |
| 16 | -17.9% | -20.6% | -72.0% | -69.6% |
| 32 | -15.7% | -42.0% | -68.7% | -65.4% |
| 64 | -14.4% | -59.6% | -74.6% | -61.7% |

QueryResult::from_cubestore_fb

| cols / rows | 1 000 | 10 000 | 50 000 | 100 000 | | --- | --- | --- | --- | --- |
| 8 | -9.3% | -8.4% | -11.0% | -9.2% |
| 16 | -7.9% | -12.3% | -11.7% | -8.7% |
| 32 | -9.4% | -4.2% | -3.2% | +9.4% |
| 64 | -2.0% | -3.1% | +3.0% | +8.0% |

QueryResult::from_js_raw_data

| cols / rows | 1 000 | 10 000 | 50 000 | 100 000 | | --- | --- | --- | --- | --- |
| 8 | -10.1% | -9.0% | -7.3% | -11.0% |
| 16 | -2.2% | +3.8% | -2.3% | -0.8% |
| 32 | -1.8% | -1.3% | +4.7% | +13.6% |
| 64 | -1.1% | -4.1% | -0.8% | +17.3% |
-->

[claude]

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

---

Code Review: PR #10829 — Migrate QueryResult to columnar storage

• Read and understand all changed files
• Review query_message_parser.rs — core data structure changes
• Review query_result_transform.rs — transform logic changes
• Review orchestrator.rs — JS bridge changes
• Review transform.rs benchmarks
• Post comprehensive review

---

Overall Assessment

Clean, well-motivated performance refactor. The column-major layout eliminates the per-call re-pivot in the Columnar transform path, and the benchmark numbers speak for themselves (up to -74.6% on columnar transforms). The code is mechanically correct, the wire format is unchanged, and the new benchmarks add good coverage for the ingestion paths.

A few observations below, none blocking.

---

1. Unchecked indexing in the JS bridge

File: packages/cubejs-backend-native/src/orchestrator.rs, line 362

let value = &result.data[col_idx][row_idx];

This is the only call-site that uses direct indexing into the columnar data. All transform code consistently uses the safe .get(*key).and_then(|c| c.get(row_idx)) pattern. Since QueryResult fields are all pub, a mismatch between row_count and the actual column vector lengths would panic here rather than returning a graceful error.

In practice this is safe because the two constructors (from_cubestore_fb, from_js_raw_data) maintain the invariant. But for defensive consistency with the rest of the codebase, consider using .get() or the new cell() accessor.

---

2. row_count field can drift from actual data

File: rust/cube/cubeorchestrator/src/query_message_parser.rs, lines 39–40

row_count is stored as a separate pub field but is semantically derived from the column vector lengths. Since all struct fields are public, external code could construct a QueryResult where row_count != data[i].len(), causing panics in cell(), column(), and the JS bridge.

Two options to harden this:

1. Make fields private and enforce the invariant through constructors only
2. Add a debug_assert!(self.data.iter().all(|c| c.len() == self.row_count)) in debug builds

Not urgent since only two internal constructors exist today, but worth considering as the API surface grows.

---

3. Redundant accessor methods alongside pub fields

File: rust/cube/cubeorchestrator/src/query_message_parser.rs, lines 55–67

row_count(), column(), and cell() are new accessor methods, but the underlying row_count, data fields are also pub. The JS bridge already accesses result.row_count directly. Having both public fields and accessor methods can be confusing about which to use.

Suggestion: either make the fields private (and the accessors become the sole API), or drop the accessors until the fields are made private. This would also naturally address point #2.

---

4. Performance trade-off at high column counts is expected

The PR description transparently shows regressions for from_cubestore_fb (+9.4%) and from_js_raw_data (+17.3%) at 64 cols × 100k rows. This is the expected cost of the columnar pivot during ingestion — writing to 64 separate column vectors has worse cache locality than filling one row vector at a time. The dramatic transform wins (up to -74.6%) more than offset this for the overall pipeline. Good trade-off.

---

5. Benchmarks are well-structured

• Correct use of iter_batched with LargeInput for from_js_raw_data (which consumes its input)
• Proper black_box usage to prevent dead-code elimination
• Throughput tracking in cells/sec makes cross-configuration comparisons meaningful
• The FlatBuffer builder in build_cubestore_fb_message realistically mirrors CubeStore's wire format

---

6. Short-row padding is good defensive code

File: rust/cube/cubeorchestrator/src/query_message_parser.rs, lines 133–137

// Pad short rows with Null to keep all columns aligned.
for col_idx in values.len()..n_cols {
    result.data[col_idx]
        .push(DBResponseValue::Primitive(DBResponsePrimitive::Null));
}

This handles malformed FlatBuffer rows gracefully — a good addition that the old row-major code didn't need to worry about explicitly.

---

7. Consistent use of QueryResult::empty() in tests

The replacement of inline QueryResult { columns: vec![], rows: vec![], columns_pos: IndexMap::new() } with QueryResult::empty() across all test call-sites is a nice cleanup that makes tests more resilient to future field changes.

---

Summary

Solid performance optimization with clear benchmarks. The main suggestion is to consider tightening the QueryResult API (private fields + public accessors) to enforce the row_count invariant structurally rather than by convention. No correctness issues found.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.84%. Comparing base (df1479d) to head (cefbaf9).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10829      +/-   ##
==========================================
- Coverage   78.84%   78.84%   -0.01%     
==========================================
  Files         470      470              
  Lines       92274    92274              
  Branches     3433     3433              
==========================================
- Hits        72755    72750       -5     
- Misses      19016    19021       +5     
  Partials      503      503              

Flag	Coverage Δ
cube-backend	58.22% <ø> (ø)
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.