fix: read ARRAY<DOUBLE> through Arrow's Float8Vector - #69
Open
LuciferYang wants to merge 2 commits into
Open
Conversation
readArrayData cast the child vector to the private inner Double8Vector wrapper instead of org.apache.arrow.vector.Float8Vector. The wrapper is never instantiated anywhere, so every read of an array-of-double column (List or FixedSizeList, both map to ARRAY<DOUBLE>) threw ClassCastException. The write path and the Float read branch already used the Arrow vector directly; this was a leftover from an unfinished refactor. RowDataConverterTest covers both list representations for double and float reads, plus empty/null arrays and a write batch that forces the child vector to reallocate (the production sink path); each guard was verified to redden under a targeted regression mutation. The tests allocate off-heap memory, so the root pom also sets the --add-opens=java.base/java.nio argLine that JDK 17 CI legs require for Arrow's MemoryUtil to initialize.
Three things the guards did not actually establish. The realloc test's comment had every number wrong. setInitialCapacity(4) does not leave the child at 4: allocateNew rounds the 40-byte request to 64 and re-spreads it, landing on 7, and the child then grows 7 -> 15 -> 31 -> 63 -> 126 -> 252 -> 504 -> 1008, with the first realloc on row 2 rather than row 1. The comment now says that, and the test asserts the starting capacity is below 600 instead of taking it on faith. Without setInitialCapacity the default is 4032, which swallows all 600 with no realloc at all, so the test was one deleted line away from silently proving nothing; that line now reddens the assertion. setNull on a freshly allocated child is a no-op, since the validity bits start at zero. Both FixedSizeList tests wrote a value first, so the null slot sits over live data the way a real Arrow batch would, and row 2's own slots carry values so a drifted read fails on 4.5 instead of finding a conveniently null unwritten slot. The float test was missing two assertions the double test had: the list size, and a null parent row. It had no way to notice a broken parent-null short-circuit; removing that short-circuit now reddens it. Also asserts the middle element of each array in the realloc loop, which previously checked only the first and last.
LuciferYang
marked this pull request as ready for review
August 22, 2026 09:17
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
1 participant
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.
Fixes #68
Problem
The
DoubleTypebranch ofRowDataConverter.readArrayDatacast the child vector to the private inner classDouble8Vector, a wrapper that is never instantiated anywhere (its javadoc says "alias for Float8Vector"), a leftover from an unfinished refactor. Every read of anARRAY<DOUBLE>column, via either Arrow representation that maps to it (List<Float64>andFixedSizeList<Float64>, i.e. Lance f64 vector columns), threwClassCastException. The type layer accepts the schema (LanceTypeConverterdocumentsFixedSizeList<Float64> <-> ARRAY<DOUBLE>), so DDL and planning succeed and the failure only surfaces at scan time. All read entry points (LanceSource,LanceInputFormat,LanceAggregateSource,LanceVectorSearch) funnel into this one private method and were all affected.Fix
org.apache.arrow.vector.Float8Vectordirectly, matching the siblingFloatTypebranch and the write path.Double8Vectorwrapper.Tests (
RowDataConverterTest, 4 guards)testWriteThenReadArrayOfDoubleRoundTriptestReadFixedSizeListOfDoubletestWriteBeyondInitialListCapacitysetSafereallocation, the path the production sink hits on every 1024-row batch (defaultwrite.batch-size). The test asserts the small starting capacity so it cannot quietly become a no-optestReadFixedSizeListOfFloatFloatTypebranch (f32 vector columns, the primary embedding path): values, null element, null parent, list sizeVerification, on
lance-flink-1.18under JDK 17:RowDataConverterTest4/4 green, and the full module unit suite isTests run: 190, Failures: 0, Errors: 0, Skipped: 17. The 17 skips areLanceCatalogS3Test$MinioIntegrationTests, which need a MinIO endpoint and are unrelated to this change.Six targeted mutations each reddened the guard that watches it, while the others stayed green: reverting the cast fix, an FSL start-index off-by-one, dropping the element null guard, dropping the parent-null short-circuit,
setSafe->set, and a wrong cast in the Float branch. Reverting the production fix reddens three of the four tests, not all four:testReadFixedSizeListOfFloatguards the siblingFloatTypebranch and stays green by design.Build note
The new tests allocate off-heap Arrow memory, which requires
--add-opens=java.base/java.nio=ALL-UNNAMEDforMemoryUtilto initialize on JDK 17 (a CI matrix leg). The root pom now sets thatargLine; jacoco appends its agent to the same property, so the two coexist.