Skip to content

GH-88: Support importing C Data Interface arrays with a non-zero offset - #1419

Open
andygrove wants to merge 5 commits into
apache:mainfrom
andygrove:fix/c-data-import-offset
Open

andygrove wants to merge 5 commits into
apache:mainfrom
andygrove:fix/c-data-import-offset

Conversation

@andygrove

@andygrove andygrove commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

What's Changed

The C Data Interface importer ignored the offset of an ArrowArray struct, and since #1263 rejected it outright. Exporting a sliced RecordBatch from C++ puts the slice offset on every child array, so such batches could not be imported into Java.

ArrayImporter now supports a non-zero offset. Java vectors cannot represent an offset, so the importer loads the rows up to the end of the slice into a temporary vector and transfers the slice into the target with splitAndTransfer. Leading rows that fill whole bytes of a bitmap are skipped by advancing the buffer pointers, so the temporary vector, and any validity bitmap synthesized for it, starts fewer than 8 rows before the slice. Children indexed by the rows of their parent (struct, sparse union, fixed-size list) get the same skip and are trimmed to the slice. Run-end encoded arrays are sliced by the split alone. Fixed-width data buffers and byte-aligned validity bitmaps are shared with the C memory, while offset buffers, unaligned validity bitmaps and view data are copied.

The null count reported by a producer only covers the slice, so the temporary import passes an unknown null count (-1) when a validity bitmap is present and 0 when the producer omitted it.

The temporary vector is created from the imported field, so it has the extension types of vectors whose getField() reports only their storage type, and the top-level transfer happens between storage vectors. Importing into an existing vector whose nested extension vectors do not report their type fails with a clear error.

Two bugs in the transfers this relies on are fixed:

  • ExtensionTypeVector.makeTransferPair failed with a ClassCastException for an extension vector target, unless the subclass overrode it as UuidVector and JsonVector do and OpaqueVector does not.
  • The DenseUnionVector split indexed its per-child state by type id, which failed for type ids other than 0 to n - 1. It also returned wrong values when the offsets of a child had gaps.

Tests import 20-row vectors at every offset for int, bit, varchar, string view, list, large list, fixed-size list, map, struct, sparse and dense union, run-end encoded, extension types (UUID, opaque, and one that overrides neither getField() nor makeTransferPair(), at the top level and nested) and dictionary encoded arrays. They also cover the original report's scenario of a VectorSchemaRoot whose children carry the slice offset. Two tests check that importing 10 rows at an offset of 65,536 does not hold memory for the rows before the offset.

The documentation now describes offset support and its copying caveat instead of stating that offsets are unsupported.

Closes #88.

The C Data Interface importer ignored, and later rejected, the offset of
an ArrowArray struct, so a sliced record batch exported from C++ could
not be imported.

Java vectors cannot represent an offset, so the importer now loads the
leading offset + length values into a temporary vector and transfers the
requested window into the target with splitAndTransfer. Buffers that can
be sliced are shared; offset buffers and unaligned validity bitmaps are
copied.

Closes apache#88
@github-actions

This comment has been minimized.

@andygrove andygrove added the bug-fix PRs that fix a big. label Oct 31, 2026
@github-actions github-actions Bot added this to the 20.0.0 milestone Oct 31, 2026
// The producer's null count only covers the slice. Without a validity bitmap the null count
// must be 0 for the buffers to load, otherwise it is unknown.
boolean hasBitmap = snapshot.n_buffers > 0 && MemoryUtil.getLong(snapshot.buffers) != NULL;
snapshot.null_count = hasBitmap ? -1 : 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Producers commonly omit the validity bitmap for columns without nulls. With null_count = 0, BitVectorHelper.loadValidityBuffer then allocates and fills an all-ones bitmap for offset + length rows.
For byte-aligned offsets the target keeps a slice of that buffer, so the whole bitmap stays allocated as long as the target lives.

For example, importing 0 rows at offset 8,000,000 from a non-null int column allocates about 1 MB of bitmap and keeps it after the temporary vector is closed. That is the main use case of this PR, since a sliced RecordBatch puts the offset on every column.
When a large batch is streamed in slices, the cost of each import grows with its offset.

The synthesized bitmap should only cover the length rows that end up in the target.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e110114. The importer now advances the buffer pointers past the leading rows that fill whole bytes of a bitmap, so the temporary vector starts at most 7 rows before the slice. A bitmap synthesized for it covers at most length + 7 rows. The imported buffers shrink the same way, so the target no longer keeps a wrap of every row before the offset either.

Children whose rows line up with the parent's rows (struct, sparse union, fixed-size list) get the same skip. They are also trimmed to the slice, because a sliced struct exports them at full length. Run-end encoded arrays keep the split-only path, since their children are not indexed by row.

testImportSliceWithoutValidityBitmapAllocatesOnlyTheSlice and testImportStructSliceWithoutValidityBitmapsAllocatesOnlyTheSlice import 10 rows at offsets around 65,536 from arrays without bitmaps, and check that the importing allocator holds less than 1 KB. Before the change it held about 260 KB.

unslicedImporter.recursionLevel = recursionLevel;
unslicedImporter.underlyingAllocation = underlyingAllocation;
unslicedImporter.importUnsliced(snapshot);
unslicedVector.makeTransferPair(vector).splitAndTransfer(offset, length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This relies on makeTransferPair(vector) + splitAndTransfer working for every vector type, and it doesn't for extension vectors. ExtensionTypeVector.makeTransferPair passes the extension target straight to the storage vector, which casts it to its own class:

java.lang.ClassCastException: class org.apache.arrow.vector.extension.OpaqueVector cannot be cast to class org.apache.arrow.vector.VarBinaryVector

UuidVector (the one in the tests) and JsonVector work only because they override makeTransferPair. OpaqueVector and any user-defined ExtensionTypeVector that doesn't override it fail, both at the top level and as a child of a sliced struct or list. A user-defined vector that keeps the default getField() fails as well: getField() then returns the storage field, so the temporary vector is created as the plain storage vector, and that vector can't transfer into the extension target either.

A sliced RecordBatch puts the offset on every column, so any batch with a custom extension column hits this.

Could we fix it in ExtensionTypeVector (unwrap an extension target to its underlying vector, and make sure the temporary vector has the extension type) and add tests with a custom extension type that overrides neither getField() nor makeTransferPair()?
The alternative is to keep rejecting offsets for these vectors with a clear error for now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aead27c and e110114:

  • ExtensionTypeVector.makeTransferPair now transfers into the underlying vector of an extension vector target, and getTo() still returns the extension vector. That covers OpaqueVector and any other vector that does not override it, at any depth.
  • The importer creates the temporary vector from the imported field when it has one, which importVector, importVectorSchemaRoot and importIntoVectorSchemaRoot do. That field has the extension type even when the vector's getField() reports only its storage type, so the temporary vector has the same classes as the target.
  • At the top level the importer transfers between the storage vectors, so importIntoVector into such a vector works as well.

One case is left: importIntoVector into a caller's vector that nests such an extension vector below the top level. No field reports that vector's type there, so the import now throws an UnsupportedOperationException that names the vector. Before, the struct's transfer pair silently replaced the child: the target's extension vector became a plain VarCharVector.

The tests use a PlainVector that overrides neither getField() nor makeTransferPair(). They cover it at the top level, in a struct, in a list, in a sliced record batch, and through importIntoVector. OpaqueVector is covered at the top level, in a struct and in a list. TestExtensionType tests the transfer pair itself with LocationVector.

unslicedImporter.recursionLevel = recursionLevel;
unslicedImporter.underlyingAllocation = underlyingAllocation;
unslicedImporter.importUnsliced(snapshot);
unslicedVector.makeTransferPair(vector).splitAndTransfer(offset, length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I believe dense unions have two problems in DenseUnionVector.TransferImpl.splitAndTransfer, which this PR now exposes to C Data imports:

  1. typeCounts / typeStarts are sized by the number of children (nextTypeId) but indexed by the raw type code read from the type buffer. A dense union with type codes [5, 10] and offset 1 fails with ArrayIndexOutOfBoundException: Index 10 out of bounds for length 2. The same array imports fine with offset 0.
  2. It assumes each child's offsets in the window are contiguous: it transfers count child values starting at the first offset it sees and renumbers them 0..count-1. The spec only requires the offsets to increase, so gaps are valid. With one child holding 100..107, row offsets [0, 2, 7] and a slice of 2 rows at offset 1, the result is 102, 103 instead of 102, 107, with no error.

The second one is wrong data with no error, where main rejects the same input today. The tests only use type codes 0 and 1 with contiguous offsets.

Could we either fix the split (map type code to child index, and rebuild the child offsets instead of assuming they are contiguous), or keep rejecting offsets for dense unions for now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed both in c8083d5, in DenseUnionVector.TransferImpl:

  1. The split maps each type id to the position of its child through typeMapFields. createTransferPairs() and transfer() had the same mix-up, which put the children of a new target under their positions rather than their type ids, so those are fixed as well.
  2. The split checks whether the values a window selects from each child are contiguous. Contiguous values are still split without copying. Values with gaps are copied with copyValueSafe and renumbered from 0.

Copying keeps the value count of each child equal to the number of rows that select it. DenseUnionVector.setValueCount() enforces that, and a parent struct or list calls it after its own split, so keeping the gaps would have been undone there.

With your example the slice now returns 102 and 107. TestDenseUnionVector covers type ids [5, 10], gaps at the top level and inside a struct, and a transfer pair that creates its target. RoundtripTest imports both cases at every offset.

andygrove and others added 4 commits October 31, 2026 12:29
ExtensionTypeVector.makeTransferPair passed an extension vector target to
the transfer pair of its underlying vector, which casts the target to its
own class. Transfers between extension vectors that do not override
makeTransferPair, such as OpaqueVector, failed with a ClassCastException.

Transfer into the underlying vector of an extension vector target instead.
DenseUnionVector's transfer pair indexed its per-child state by type id,
although it is sized and ordered by child. Splitting a dense union whose
type ids are not 0 to n - 1 failed with an ArrayIndexOutOfBoundsException,
and transfers placed the target's children under the wrong type ids.

The split also assumed that the values a window selects from a child are
contiguous. The offsets of a child only have to increase, so for offsets
with gaps the split returned the wrong values. Values with gaps are now
copied, while contiguous values are still split.
A sliced import loaded every row before the slice into a temporary
vector, so the validity bitmap synthesized for an array without one, and
the imported buffers, grew with the offset and stayed alive with the
target. Skip the leading rows that fill whole bytes of a bitmap by
advancing the buffer pointers, and import only the rows of the slice from
the children of structs, sparse unions and fixed-size lists.

Create the temporary vector from the imported field, which has the
extension types of vectors whose getField() reports their storage type,
and transfer between storage vectors at the top level. Importing into an
existing vector whose nested extension vectors do not report their type
fails with a clear error instead of replacing them.
@andygrove
andygrove requested a review from jbonofre November 1, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PRs that fix a big.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java][C] sliced RecordBatch offset info is lost when imported from c-data

2 participants