Skip to content

fix: read ARRAY<DOUBLE> through Arrow's Float8Vector - #2

Closed
LuciferYang wants to merge 1 commit into
ci/failsafe-integration-testsfrom
fix/converter-array-double-cast
Closed

fix: read ARRAY<DOUBLE> through Arrow's Float8Vector#2
LuciferYang wants to merge 1 commit into
ci/failsafe-integration-testsfrom
fix/converter-array-double-cast

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Fixes #1

Problem

The DoubleType branch of RowDataConverter.readArrayData cast the child vector to the private inner class Double8Vector — a wrapper that is never instantiated anywhere (its javadoc says "alias for Float8Vector"), a leftover from an unfinished refactor. Every read of an ARRAY<DOUBLE> column, via either Arrow representation that maps to it (List<Float64> and FixedSizeList<Float64>, i.e. Lance f64 vector columns), threw ClassCastException. The type layer accepts the schema (LanceTypeConverter documents FixedSizeList<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

  • Cast to org.apache.arrow.vector.Float8Vector directly, matching the sibling FloatType branch and the write path.
  • Delete the dead Double8Vector wrapper.

Tests (RowDataConverterTest, 4 guards)

Test Behavior it pins
testWriteThenReadArrayOfDoubleRoundTrip List-representation write/read round trip: dense values, null element, empty array, null parent array
testReadFixedSizeListOfDouble FixedSizeList (f64 vector column) read: values, null element, null parent, start-index arithmetic
testWriteBeyondInitialListCapacity Writing 600 elements against an initial child capacity of 4 forces repeated setSafe reallocation — the path the production sink hits on every 1024-row batch (default write.batch-size)
testReadFixedSizeListOfFloat The sibling FloatType branch (f32 vector columns, the primary embedding path): values and null element

Verification: 4/4 green; full module unit suite 234/0/0; six targeted mutations (reverting the cast fix, FSL start-index off-by-one, dropping the element null guard, dropping the parent-null short-circuit, setSafe -> set, wrong cast in the Float branch) each reddened exactly the corresponding test while the others stayed green; with the production fix reverted, the suite-level red set was exactly the tests above.

Build note

The new tests allocate off-heap Arrow memory, which requires --add-opens=java.base/java.nio=ALL-UNNAMED for MemoryUtil to initialize on JDK 17 (a CI matrix leg). The root pom now sets that argLine; jacoco appends its agent to the same property, so the two coexist.

@github-actions github-actions Bot added the bug Something isn't working label Aug 21, 2026
@LuciferYang
LuciferYang marked this pull request as draft August 21, 2026 16:45
@LuciferYang
LuciferYang force-pushed the fix/converter-array-double-cast branch from 3b7adfe to 9ee3bfd Compare August 21, 2026 16:51
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.
@LuciferYang
LuciferYang force-pushed the fix/converter-array-double-cast branch from 9ee3bfd to c7e7596 Compare August 21, 2026 16:53
@LuciferYang

Copy link
Copy Markdown
Owner Author

Superseded by upstream PR lance-format#69 (issue tracked at lance-format#68). Closing this fork-local PR.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant