GH-812: Fix appending to an empty UnionVector - #1292
hutiefang76 wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
|
The PR-format job failed because the required category label is missing. This is a bug fix for #812 with no breaking API change. Could a maintainer add |
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues remain.
Pull request overview
Fixes appending to an empty UnionVector by sizing from type-buffer capacity before child vectors are created.
Changes:
- Updates union capacity expansion logic.
- Adds bounded-allocator regression coverage.
File summaries
| File | Description |
|---|---|
vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java |
Adds regression coverage for appending floats to an empty union. |
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java |
Corrects union type-buffer capacity handling. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jbonofre
left a comment
jbonofre
left a comment
There was a problem hiding this comment.
LGTM, thanks @hutiefang76! This fixes the reproducer from #812.
One non-blocking nit: values.allocateNew() in the new test is redundant, since getFloat4Vector() and ValueVectorDataPopulator.setVector() already allocate.
What's Changed
Appending to a newly created
UnionVectorrepeatedly reallocates its type buffer until allocation fails: its value capacity remains zero until it has child vectors. Check the type buffer capacity at this stage, since the existing append loop creates and expands the child vectors afterward.The regression appends two floats to an empty union with a bounded allocator. It fails before the change with an allocation error and passes afterward. All 32
TestVectorAppendertests pass with both the Netty and Unsafe allocators on JDK 21. Checkstyle and Spotless also pass.Closes #812.
Validation (JDK 21):
mvn -B -pl vector -am test -Dtest=TestVectorAppender -Dsurefire.failIfNoSpecifiedTests=false mvn -B -pl vector spotless:checkDeveloped with AI assistance; the regression was verified to fail before the fix and pass afterward.