test: name the whole dispatched subtree in the decimal promotion assertion - #5849
Merged
andygrove merged 1 commit intoSep 10, 2026
Merged
Conversation
…rtion apache#5610 started recording every expression in a dispatched subtree rather than only its root, on the grounds that the whole subtree is bound into the one kernel and so all of it ran in the JVM. It regenerated the q78 plan-stability goldens for that change but left this assertion expecting `hypot` alone, so the `[expressions]` job has failed on all five Spark profiles since. Expect the promoted subtree's names: `hypot`, its `cast`, the `checkoverflow` wrapper promotion adds around the decimal `add`, and the `add` itself. Closes apache#5848 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
github-actions
Bot
added
enhancement
sunchao
approved these changes
Sep 10, 2026
This was referenced Sep 10, 2026
This was referenced Sep 11, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #5848.
Rationale for this change
CometCodegenSuite."codegen dispatch coverage survives the decimal promotion rewrite"fails on main and takes the[expressions]job down on all five Spark profiles, so every PR opened against main is red for a reason unrelated to its own changes:The behaviour is correct and the assertion is stale. #5610 deliberately started naming every expression in a dispatched subtree rather than only its root: the whole subtree is bound and closure-serialized into one kernel, so all of it ran in the JVM, and naming only the root would let a test assert that a nested
abswas native while anabswas in fact running inside the kernel. That PR regenerated the q78 plan-stability goldens for the wider dispatcher list but did not update this assertion, which predates it.What changes are included in this PR?
Expect the promoted subtree's full set in that assertion:
hypot, itscast, thecheckoverflowwrapper decimal promotion adds around theadd, and theadd. The surrounding comment now says why the set has more than the root in it, and whycheckoverflowappearing is the thing that makes the assertion evidence that the promoted copy, not the original tree, was recorded.No production code changes.
How are these changes tested?
CometCodegenSuitepasses in full (96 tests) on Spark 4.1. The failing assertion was confirmed red on a clean checkout of main first, and the actual set is identical on all five profiles in CI (3.4, 3.5, 4.0, 4.1, 4.2), so no version-specific expectation is needed.🤖 Generated with Claude Code
https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B