Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arrow.enable_null_check_for_get causes change in the behavior of VarBinaryVector#getObject #469

Closed
aqni opened this issue Dec 27, 2024 · 4 comments · Fixed by #486
Closed
Labels
Type: bug Something isn't working Type: usage usage question
Milestone

Comments

@aqni
Copy link
Contributor

aqni commented Dec 27, 2024

Describe the bug, including details regarding any error messages, version, and platform.

  • version: 18.1.0
  • platform: windows, jdk17
  @Test
  void testVarbinary(){
    try(RootAllocator allocator = new RootAllocator(Long.MAX_VALUE);
        BigIntVector bigIntVector = new BigIntVector("test", allocator);
        VarBinaryVector varBinVector = new VarBinaryVector("test", allocator)){
      bigIntVector.setNull(0);
      bigIntVector.setValueCount(1);
      varBinVector.setNull(0);
      varBinVector.setValueCount(1);
      Assertions.assertNull(bigIntVector.getObject(0));
      Assertions.assertNull(varBinVector.getObject(0));
    }
  }

When I set arrow.enable_null_check_for_get to true, this test passes. However, when I set arrow.enable_null_check_for_get to false, it failed.

org.opentest4j.AssertionFailedError: 
Expected :null
Actual   :[B@73e9cf30
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertNull.failNotNull(AssertNull.java:50)
	at org.junit.jupiter.api.AssertNull.assertNull(AssertNull.java:35)
	at org.junit.jupiter.api.AssertNull.assertNull(AssertNull.java:30)
	at org.junit.jupiter.api.Assertions.assertNull(Assertions.java:279)
	at demo.MainTest.testVarbinary(MainTest.java:27)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Why does BigIntVector's getObject always return null, while VarBinary Vector's getObject has a different behavior?

@aqni aqni added the Type: bug Something isn't working label Dec 27, 2024
@lidavidm
Copy link
Member

lidavidm commented Jan 3, 2025

Because you've disabled null checking entirely, so VarBinaryVector takes it as an opportunity to skip the null check and return whatever string data happens to be in that slot (even though the slot is meant to be null, it can still have data allocated, though it is probably an empty string). BigIntVector doesn't have that optimization.

@lidavidm
Copy link
Member

lidavidm commented Jan 3, 2025

Arguably, in this case, BigIntVector should also be returning non-null.

@lidavidm lidavidm added the Type: usage usage question label Jan 3, 2025
@aqni
Copy link
Contributor Author

aqni commented Jan 6, 2025

Is there any plan in the future to unify the behavior of getObject when arrow.enable_null_check_for_get is set to false?

@lidavidm
Copy link
Member

lidavidm commented Jan 6, 2025

Contributions would be welcome. I don't think anyone is actively working on it.

lidavidm pushed a commit that referenced this issue Jan 14, 2025

Verified

This commit was signed with the committer’s verified signature.
…et` behavior about null value (#486)

Fixes #469.
@lidavidm lidavidm added this to the 18.2.0 milestone Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: bug Something isn't working Type: usage usage question
Projects
None yet
2 participants