jbonofre commented on PR #989: URL: https://github.com/apache/arrow-java/pull/989#issuecomment-4037103928
I did a new pass on the PR and I don't think it's correct. 1. `TestValueVector` tests are failing. The PR changes offset buffer behavior for empty vectors, now always writes `OFFSET_WIDTH` bytes. In the case of empty vectors, I would expect 0 (as before). 2. There are some memory leaks (`TestDenseUnionVector.testGetFieldTypeInfo`, `TestStructVector.testAddChildVectorsWithDuplicatedFieldNamesForConflictPolicyReplace`, `TestSplitAndTransfer.testWithEmptyVector`, `TestUnionVector.testGetFieldTypeInfo`). This is a consequence of this change as the offset buffer now allocating extra bytes that aren't being properly freed in the test cleanup. I think the problem is that the PR changes `setReaderAndWriterIndex()` to always use `(valueCount + 1) + OFFSET_WIDTH` for the offset buffer. The text expectations need to be adjusted by +4 bytes (one 32-bit offset) for the affected cases. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
