fix(wren-core-wasm): drive query() through a tokio runtime (UNION ALL trap)#2291
Conversation
`@wrenai/wren-core-wasm` 0.3.0 and 0.4.0 trap with `RuntimeError: unreachable` on any SQL whose physical plan contains `CoalescePartitionsExec` over more than one partition — `UNION ALL`, `UNION`, `INTERSECT`, `EXCEPT`, and some parallel hash joins. The trivial `SELECT a FROM (SELECT 1 AS a UNION ALL SELECT 2) t` reproduces without any MDL or registered tables. Root cause: `CoalescePartitionsExec::execute` (DataFusion 53) spawns one task per input partition via `JoinSet::spawn` → `tokio::task::spawn`. That spawn panics with `there is no reactor running, must be called from the context of a Tokio 1.x runtime` because `wasm-bindgen-futures` drives Rust futures straight off JS microtasks without ever entering a tokio runtime. Single-partition queries hit `CoalescePartitionsExec`'s short-circuit branch and never spawn, which is why every non-set-op dashboard query worked and the bug stayed hidden until now. Fix: own a `current_thread` `tokio::runtime::Runtime` on `WrenEngine` and run the body of `query()` (and therefore `cube_query()`, which delegates) inside `runtime.block_on(...)`. block_on drives the inner future to completion synchronously, so DataFusion's spawned tasks now have a live scheduler to make progress on. Also adopt `console_error_panic_hook::set_once()` in `WrenEngine::new` so any future panic surfaces in the JS console with a Rust message instead of the opaque `unreachable` trap that hid this bug for two releases. Coverage: - 6 new Node SDK tests under "set operators": UNION ALL (subquery + top-level), UNION, INTERSECT, EXCEPT, and UNION ALL across two registered tables. - 1 new Rust `wasm_bindgen_test` (`test_union_all_does_not_trap`). - 1 native wren-core test (`test_union_all_local_runtime`) documents that the wren-core LocalRuntime path itself is fine — the bug is wasm32-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThis PR fixes WASM runtime traps when executing set-operator queries by introducing an owned tokio current-thread runtime to ChangesSet Operator Runtime Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
@wrenai/wren-core-wasm@0.4.0(and 0.3.0 before it) traps withRuntimeError: unreachableon any SQL whose physical plan containsCoalescePartitionsExecover more than one partition —UNION ALL,UNION,INTERSECT,EXCEPT, and some parallel hash joins. Thetrivial form (no MDL, no parquet, no tables) reproduces:
Reported in
BUG_REPORT_wren_core_wasm_union_all.md.Root cause
CoalescePartitionsExec::execute(DataFusion 53) spawns one task per inputpartition via
JoinSet::spawn→tokio::task::spawn. That spawn panicswith
because `wasm-bindgen-futures` drives Rust futures directly off JS
microtasks and never enters a tokio runtime. Single-partition queries hit
the `1 =>` short-circuit in `CoalescePartitionsExec` and never spawn,
which is why every `registerJson` + raw `query` example, every cube
query, and every dashboard query worked — and the bug stayed hidden until
the first `UNION ALL` in the wild.
The bug is wasm32-only: the same SQL + same DataFusion 53.0.0 + same
crate feature set succeeds when compiled natively (verified). It's not a
0.4.0 regression — 0.3.0 traps identically.
Fix
The public JS surface is unchanged — both methods are still `async` and
still return a Promise. The block_on drives the inner future synchronously
on the JS thread (which is what wasm-single-thread already does anyway).
Coverage
sdk/tests/index.test.mjs— 34 total tests now pass).Verification
Release
This should ship as a `@wrenai/wren-core-wasm` patch (`0.4.1`). The fix
is purely additive on the Rust side (new field, new dep `console_error_panic_hook`) — the JS API is unchanged.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests