GH-1190: Reserve view slots for empty view vector values

[goutamadwant]

What's Changed

Fixes BaseVariableWidthViewVector.handleSafe so setSafe reserves a full 16-byte view slot for the target index even when the value length is zero.

Adds coverage for empty values written through the first view-buffer boundary for both ViewVarCharVector and ViewVarBinaryVector.

Tests

• mvn -pl vector -am -Dtest=TestVariableWidthViewVector#testSetSafeEmptyValueAtViewBufferBoundary -Dsurefire.failIfNoSpecifiedTests=false test
• mvn -pl vector -am -Dtest=TestVariableWidthViewVector -Dsurefire.failIfNoSpecifiedTests=false test

Closes#1190.

[mbutrovich]

Thanks @goutamadwant, this looks like what I was going to do! One coverage gap worth closing: handleSafe(index, 0) is also hit by setNull (1195) and fillEmpties via setValueCount (1022), so the bug was reachable without ever writing an empty string. The fix covers those too, but the test only exercises setSafe(i, new byte[0]). Suggest adding the two tests below. Non-blocking.

[mbutrovich]

Correct. Reserves the full slot regardless of payload; ((long) index + 1) avoids overflow. roundUpToMultipleOf16 is a no-op here but fine to keep.

[goutamadwant]

Thanks for reviewing @mbutrovich . I kept this code path unchanged since it reserves the full view slot regardless of payload size and the cast keeps the sizing expression safe before the multiplication. Let me know if there any other thoughts.

[mbutrovich]

Only covers the empty-value path. setNull and fillEmpties/setValueCount reach the same handleSafe(index, 0). Suggest adding tests for those.

setValueCount trailing gap (silent trigger, no empty string written)

@ParameterizedTest@MethodSource({"vectorCreatorProvider"})
publicvoidtestSetValueCountFillsEmptiesAtViewBufferBoundary(
Function<BufferAllocator, BaseVariableWidthViewVector> vectorCreator) {
try (finalBaseVariableWidthViewVectorvector = vectorCreator.apply(allocator)) {
vector.allocateNew();
finalintvalueCapacity = vector.getValueCapacity();
vector.setSafe(valueCapacity - 1, "x".getBytes(StandardCharsets.UTF_8));
// Leaves index valueCapacity unset: fillEmpties -> handleSafe(_, 0) on a full buffer.vector.setValueCount(valueCapacity + 1);
assertTrue(vector.getValueCapacity() > valueCapacity);
assertTrue(vector.isNull(valueCapacity));
}
}

setNull at the boundary

@ParameterizedTest@MethodSource({"vectorCreatorProvider"})
publicvoidtestSetNullAtViewBufferBoundary(
Function<BufferAllocator, BaseVariableWidthViewVector> vectorCreator) {
try (finalBaseVariableWidthViewVectorvector = vectorCreator.apply(allocator)) {
vector.allocateNew();
finalintvalueCapacity = vector.getValueCapacity();
for (inti = 0; i <= valueCapacity; i++) {
vector.setNull(i);
}
vector.setValueCount(valueCapacity + 1);
assertTrue(vector.getValueCapacity() > valueCapacity);
assertTrue(vector.isNull(valueCapacity));
}
}

[goutamadwant]

@mbutrovich added coverage for both additional paths: setNull and setValueCount/fillEmpties at the view-buffer boundary. The production fix is unchanged and his just closes the missing regression coverage. Let me know. thanks!

[goutamadwant]

@mbutrovich addressed your comments. let me know if you have more suggestions. thanks!

[mbutrovich]

This LGTM, thanks for the PR and revision @goutamadwant!

[parthchandra]

Thanks @goutamadwant. The original implementation is logically wrong and this is the correct fix.
Also, thanks for adding the tests !

[parthchandra]

@goutamadwant could you rebase on main and also run spotless?

[parthchandra]

@lidavidm@wgtmac@laurentgo@jbonofre Can you please take a look?

[lidavidm]

Seems reasonable, but CI is not passing

[parthchandra]

but CI is not passing

Looks like a rebase and running spotless will fix the CI issues.

[goutamadwant]

@parthchandra Rebased this PR on latest main and pushed the update... also ran Spotless; the only formatting change was wrapping the long comment in BaseVariableWidthViewVector.java.

checked locally with below runs :

• mvn --batch-mode -pl vector spotless:check
• mvn --batch-mode -pl vector -am -Dtest=TestVariableWidthViewVector -Dsurefire.failIfNoSpecifiedTests=false test

The focused vector test passed under both netty and unsafe runs. Let me know. thanks!