perf: Transfer ResultWrapper rawData as Buffer(JSON), ~10x faster

[ovr]

Serialize JS result rows once on the JS side and pass them across the Neon bridge as a Buffer, so Rust can consume them via serde_json::from_slice instead of walking a JsArray through JsValueDeserializer.

[claude]

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

---

Code Review: PR #10728

• Gather context and read changed files
• Review TypeScript changes (ResultWrapper.ts)
• Review Rust changes (orchestrator.rs)
• Check for edge cases and potential issues
• Post review feedback

---

Summary

Clean, well-scoped performance optimization. The approach — serialize to JSON on the TS side and pass a Buffer across the Neon bridge so Rust can use serde_json::from_slice instead of walking a JsArray through JsValueDeserializer — is sound and well-motivated. The Rust-side changes correctly handle the new JsBuffer type while preserving full backward compatibility with the existing JsBox and JsArray paths.

Findings

1. Minor: Stale error message in extract_query_result

The fallback error message at orchestrator.rs:252-254 still reads:

"Second argument must be an Array of JsBox<Arc<QueryResult>> or JsArray"

Now that JsBuffer is also accepted, this should mention it too, e.g. "... JsBox<Arc<QueryResult>>, JsBuffer, or JsArray". A minor nit but helps with debugging. Fix this →

2. Observation: Double serialization cost is minimal

The TS side does JSON.stringify and wraps it in a Buffer, and the Rust side then does serde_json::from_slice. For the non-native (JS result) path this is strictly better than the JsValueDeserializer walk since V8's JSON.stringify is highly optimized C++ code and serde_json::from_slice is similarly well-optimized. The approach avoids N per-element Neon bridge crossings, which is where the real overhead was. Good tradeoff.

3. Observation: Backward compatibility is well handled

The downcast chain (JsBox → JsBuffer → JsArray) in both from_js_result_wrapper and extract_query_result means the old JsArray path still works if any caller hasn't been updated. This is good defensive design — no flag-day migration required.

4. Note: No test coverage for ResultWrapper

There are no existing unit tests for ResultWrapper or the getRawData path in packages/cubejs-backend-native/test/. While the change is low-risk (it just changes the serialization format, and the Rust side deserializes to the same JsRawData type), a regression test that round-trips some sample rows through Buffer.from(JSON.stringify(...)) → serde_json::from_slice would add confidence. Not a blocker for this PR, but worth tracking.

Verdict

Looks good. The optimization is well-targeted, the implementation is clean, and backward compatibility is preserved. One minor error message nit (finding #1) — otherwise ready to merge.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.81%. Comparing base (15626b1) to head (bf7b125).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #10728       +/-   ##
===========================================
+ Coverage   58.06%   78.81%   +20.74%     
===========================================
  Files         215      466      +251     
  Lines       16697    91994    +75297     
  Branches     3362     3362               
===========================================
+ Hits         9695    72502    +62807     
- Misses       6512    19002    +12490     
  Partials      490      490               

Flag	Coverage Δ
cube-backend	58.06% <ø> (ø)
cubesql	83.41% <ø> (?)

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.