GH-343: Fix BaseVariableWidthVector and BaseLargeVariableWidthVector offset buffer serialization

[Yicong-Huang]

What's Changed

Fix BaseVariableWidthVector/BaseLargeVariableWidthVector IPC serialization when valueCount is 0.

Problem

When valueCount == 0, setReaderAndWriterIndex() was setting offsetBuffer.writerIndex(0), which means readableBytes() == 0. IPC serializer uses readableBytes() to determine buffer size, so 0 bytes were written to the IPC stream. This crashes IPC readers in other libraries because Arrow spec requires offset buffer to have at least one entry [0].

This is a follow-up to #967 which fixed the same issue in ListVector/LargeListVector.

Fix

Simplify setReaderAndWriterIndex() to always use (valueCount + 1) * OFFSET_WIDTH for offset buffer's writerIndex. When valueCount == 0, this correctly sets writerIndex to OFFSET_WIDTH, ensuring offset[0] is included in serialization.

Testing

Added tests for empty VarCharVector and LargeVarCharVector verifying offset buffer has correct readableBytes().

related to #343

[Yicong-Huang]

cc @viirya @jbonofre

[jbonofre]

It's the similar fix as for valueBuffer afair. It sounds good to me.

[jbonofre]

@Yicong-Huang sorry to be a pain, can you please rebase ? Thanks !

[Yicong-Huang]

@Yicong-Huang sorry to be a pain, can you please rebase ? Thanks !

Thanks @jbonofre . I did rebase but I think the root error in this change is BaseVariableWidthVector's offsetBuffer is allocated to be 0 capacity when it is empty. Writing the offset 0 to it would be invalid. It is also not easy to increase the allocation as all tests are written against that. Could you advise on this case?

[jbonofre]

@Yicong-Huang can you please rebase again ? Sorry about that. Else I can do the rebase for you.

[Yicong-Huang]

@Yicong-Huang can you please rebase again ? Sorry about that. Else I can do the rebase for you.

Thanks. Just merged the latest master back in.

[jbonofre]

@Yicong-Huang thanks a lot ! I would like to include this fix on Arrow Java 19.0.0 release 😄

[jbonofre]

Closes #343

[Yicong-Huang]

@Yicong-Huang thanks a lot ! I would like to include this fix on Arrow Java 19.0.0 release 😄

Thanks that'd be nice! I can work with you closely on fixing this. However I see it is still failing CI, I think the original issue is still persist

I did rebase but I think the root error in this change is BaseVariableWidthVector's offsetBuffer is allocated to be 0 capacity when it is empty. Writing the offset 0 to it would be invalid. It is also not easy to increase the allocation as all tests are written against that.

Do you have any suggestions?

[jbonofre]

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.

[jbonofre]

See my previous comment about the tests.