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

[Java] AIOOBE trying to splitAndTransfer DUV within nullable struct #40999

Closed
jarohen opened this issue Apr 4, 2024 · 1 comment · Fixed by #41000
Closed

[Java] AIOOBE trying to splitAndTransfer DUV within nullable struct #40999

jarohen opened this issue Apr 4, 2024 · 1 comment · Fixed by #41000

Comments

@jarohen
Copy link
Contributor

jarohen commented Apr 4, 2024

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

If a DenseUnionVector is contained within a nullable StructVector, and that struct has nullable entries, trying to splitAndTransfer fails with an ArrayIndexOutOfBoundsException.

Within a struct vector, if an entry is null, then the entries in its children vectors are undefined, so DUV cannot always expect typeId >= 0, and should guard against it where necessary.

Test case:

  @Test
  public void testSplitAndTransferDuvInStruct() {
    try (StructVector struct = StructVector.empty("struct", allocator)) {
      DenseUnionVector duv = struct.addOrGet("duv",
          FieldType.notNullable(MinorType.DENSEUNION.getType()),
          DenseUnionVector.class);
      byte i32TypeId = duv.registerNewTypeId(Field.notNullable("i32", MinorType.INT.getType()));
      duv.addVector(i32TypeId, new IntVector("i32", allocator));

      struct.setIndexDefined(0);
      duv.setTypeId(0, i32TypeId);
      duv.setSafe(0, newIntHolder(42));

      struct.setNull(1);
      struct.setValueCount(2);

      try (StructVector dest = StructVector.empty("dest", allocator)) {
        TransferPair pair = struct.makeTransferPair(dest);
        pair.splitAndTransfer(0, 2);

        assertEquals(2, dest.getValueCount());
        assertFalse(dest.isNull(0));
        assertEquals(42, dest.getObject(0).get("duv"));
        assertTrue(dest.isNull(1));
      }
    }
  }

(PR incoming)

Component(s)

Java

@jarohen jarohen added this to 2.x Apr 4, 2024
@jarohen jarohen moved this to 👀 In review in 2.x Apr 4, 2024
lidavidm pushed a commit that referenced this issue Apr 7, 2024
…lable struct (#41000)

We add a `typeId >= 0` guard to `DUV.TransferImpl.splitAndTransfer` to fix #40999.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* GitHub Issue: #40999

Authored-by: James Henderson <[email protected]>
Signed-off-by: David Li <[email protected]>
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in 2.x Apr 7, 2024
@lidavidm lidavidm added this to the 16.0.0 milestone Apr 8, 2024
@lidavidm
Copy link
Member

lidavidm commented Apr 8, 2024

Issue resolved by pull request 41000
#41000

verma-kartik pushed a commit to verma-kartik/arrow that referenced this issue Apr 11, 2024
…in nullable struct (apache#41000)

We add a `typeId >= 0` guard to `DUV.TransferImpl.splitAndTransfer` to fix apache#40999.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* GitHub Issue: apache#40999

Authored-by: James Henderson <[email protected]>
Signed-off-by: David Li <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
…in nullable struct (apache#41000)

We add a `typeId >= 0` guard to `DUV.TransferImpl.splitAndTransfer` to fix apache#40999.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* GitHub Issue: apache#40999

Authored-by: James Henderson <[email protected]>
Signed-off-by: David Li <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…in nullable struct (apache#41000)

We add a `typeId >= 0` guard to `DUV.TransferImpl.splitAndTransfer` to fix apache#40999.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* GitHub Issue: apache#40999

Authored-by: James Henderson <[email protected]>
Signed-off-by: David Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants