Skip to content

fix(cubesql): Support type coercion in multi-branch UNION ALL#11284

Open
MazterQyou wants to merge 1 commit into
masterfrom
cubesql/union-resolve-qualifiers
Open

fix(cubesql): Support type coercion in multi-branch UNION ALL#11284
MazterQyou wants to merge 1 commit into
masterfrom
cubesql/union-resolve-qualifiers

Conversation

@MazterQyou

Copy link
Copy Markdown
Member

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made

This PR bumps cube-js/arrow-datafusion@1e33a69, fixing No field named '<cte>.<col>' planning errors in UNION ALL queries with three or more branches where a later branch introduces a column type mismatch (e.g. an Int64 count measure unioned with a Float64 sum measure). Related test is included.

Signed-off-by: Alex Qyoun-ae <4062971+MazterQyou@users.noreply.github.com>
@MazterQyou
MazterQyou requested review from a team as code owners July 17, 2026 15:32
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jul 17, 2026
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MazterQyou's task in 30s —— View job


Code Review

Summary: The PR is a small, focused change that bumps the vendored arrow-datafusion fork to pick up a fix for UNION ALL planning in the presence of type coercion across 3+ branches, plus a regression test with an insta snapshot. Real fix lives upstream in the fork.

Findings:

  • Correctness / scopeCargo.toml rev bump is the only functional change on the Rust side; matched Cargo.lock updates in both rust/cubesql/Cargo.lock and packages/cubejs-backend-native/Cargo.lock are consistent (same set of datafusion* / cube-ext crates advanced in lockstep). Good.
  • Test coverageunion_all_ctes_with_type_coercion reproduces exactly the reported failure mode: three CTEs, first two bigint, third float8, unioned by UNION ALL. Snapshot shows the promoted result column (3.5 alongside 1, 2) so the coercion actually happened rather than silently truncating. Good targeted test.
  • Nit — test only asserts success path. Consider one additional case where the type-promoted column is later consumed (e.g. wrapped in SUM(t) or t + 0), to make sure the coerced schema flows through downstream operators, not just the terminal projection. Not blocking.
  • Nit — snapshot ordering. UNION ALL doesn't guarantee row order across branches in general; DataFusion happens to preserve it here so the snapshot is stable, but if this test ever flakes in CI a follow-up would be to add ORDER BY l. Not blocking today.
  • Docs / changelog — no user-visible surface change beyond the bugfix; no docs update needed.
  • Security / perf — n/a for a dep bump + test.

Verdict: LGTM. The fix is a one-line dep bump backed by a targeted regression test; snapshot content confirms the coercion path works end-to-end. Since the actual code change is in the cube-js/arrow-datafusion fork at 1e33a69, reviewing that commit separately is worthwhile — this PR itself is minimal and safe to merge once CI is green.
Branch: cubesql/union-resolve-qualifiers

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.47%. Comparing base (fc42b0c) to head (902eb63).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11284   +/-   ##
=======================================
  Coverage   79.46%   79.47%           
=======================================
  Files         473      473           
  Lines       97669    97692   +23     
  Branches     3539     3539           
=======================================
+ Hits        77612    77640   +28     
+ Misses      19551    19546    -5     
  Partials      506      506           
Flag Coverage Δ
cube-backend 59.43% <ø> (ø)
cubesql 83.83% <100.00%> (+0.01%) ⬆️

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants