Correct FLOAT precision and column-less batch routing - #29
Open
Flanderzz wants to merge 2 commits into
Open
Conversation
An exchange splits a batch into per-channel sub-batches by taking row subsets. A projection can legitimately leave no columns behind — a key-only shuffle carries routing information and nothing else — and Arrow cannot infer how many rows such a batch holds without being told. Building the sub-batch without a row count therefore aborted the task, and only for the shapes where the columnar exchange is doing the least work. State the row count explicitly, matching how the aggregate and calc paths already construct batches, and cover both the destination and key-group splits.
Constants in a FLOAT expression were encoded as double literals, so the engine evaluated the whole expression in double precision. That contradicted the plan twice over: the result arrived as a DOUBLE column where the declared output type was FLOAT, and the arithmetic itself was carried out at a width the host never uses. The type mismatch was the louder half. The columnar boundary picks a Flink vector from the Arrow type it is handed rather than the declared one, so a widened result reached the host as a Double column and failed the moment a FLOAT field was read — an ordinary FLOAT projection was enough to abort the job. Encode single-precision constants as such, mirroring the existing treatment of narrow integer literals, which carry their declared width for the same reason. Float coverage was missing from the calc parity tests entirely, which is why this survived; add it.
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
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.
Summary:
Found two native-execution defects found while validating Flink 2.1.3 support (future PR).
Both reproduce on
mainagainst Flink 2.2.1.A projected FLOAT expression containing a FLOAT constant aborts the job. Such constants were encoded as double literals, so DataFusion promoted the whole expression to double precision and returned a DOUBLE column where the plan declared FLOAT. The columnar boundary selects a Flink column vector from the Arrow type it is handed rather than the type the plan declared, so the widened result arrived as a Double vector and threw
ClassCastExceptionthe moment a FLOAT field wasread.
The scope is narrower than it first appears, which is why it went unnoticed:
f4 * CAST(2 AS FLOAT)fails, butf4 * 2does not (narrow integer literals already carry their declared width), nor doesf4 + f4(no literal to widen), nor a FLOAT literal confined to a filter predicate (the widening lands on the boolean mask, not a projected output column).The width was wrong as well as the type. Flink evaluates FLOAT arithmetic in single precision, so computing in double and narrowing afterwards can differ in the last bits even where the cast succeeds.
The second is a task abort in the columnar exchange. Splitting a batch into per-channel sub-batches takes row subsets and rebuilds a batch from the resulting columns. A projection can legitimately leave no columns behind (a key-only shuffle carries routing information and nothing else), and Arrow cannot infer how many rows such a batch holds, so the split panicked.
Closes #28
Changes:
Testing
Validated locally with Java 17 (as always pls validate my parking here):
mvn -pl :streamfusion-runtime test -Dtest=FlinkCalcSqlHarnessTest68 tests passing, including three new FLOAT cases (literal arithmetic, columnarithmetic, and a cast projection).
cd native && cargo test --lib exchange::two new cases covering zero-column destination and key-group splits.main(c9f7905c) and confirmed fixed: a datagen table with a FLOAT column fails withArrowDoubleColumnVector cannot be cast to FloatColumnVectorbefore the change and returns correct values after. Verified across seven FLOAT query shapes; the three using a FLOAT literal in a projection fail before and pass after, the other four are unaffected either way.CastFunctionITCase: