Skip to content

ARROW-7021: [Java] UnionFixedSizeListWriter decimal type should check writer index #5802

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.

Sign up for GitHub

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

Closed
tianchen92 wants to merge 4 commits into from
Closed

ARROW-7021: [Java] UnionFixedSizeListWriter decimal type should check writer index #5802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f2df78
ARROW-XXXX: [Java] UnionFixedSizeListWriter decimal type should check…
tianchen92 Oct 16, 2019
5b66dcb
fix test
tianchen92 Nov 11, 2019
a17dded
resolve comments
tianchen92 Nov 12, 2019
1268aec
add write decimal tests
tianchen92 Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,25 @@ public void end() {

@Override
public void write(DecimalHolder holder) {
if (writer.idx() >= (idx() + 1) * listSize) {
throw new IllegalStateException(String.format("values at index %s is greater than listSize %s", idx(), listSize));
}
writer.write(holder);
writer.setPosition(writer.idx() + 1);
}

public void writeDecimal(int start, ArrowBuf buffer) {
if (writer.idx() >= (idx() + 1) * listSize) {
throw new IllegalStateException(String.format("values at index %s is greater than listSize %s", idx(), listSize));
}
writer.writeDecimal(start, buffer);
writer.setPosition(writer.idx() + 1);
}

public void writeDecimal(BigDecimal value) {
if (writer.idx() >= (idx() + 1) * listSize) {
throw new IllegalStateException(String.format("values at index %s is greater than listSize %s", idx(), listSize));
}
writer.writeDecimal(value);
writer.setPosition(writer.idx() + 1);
}
Expand Down
51 changes: 51 additions & 0 deletions java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.math.BigDecimal;
import java.util.Arrays;
import java.util.List;

import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.FixedSizeListVector;
Expand Down Expand Up @@ -265,6 +268,54 @@ public void testUnionFixedSizeListWriter() throws Exception {
}
}

@Test
Copy link
Contributor

@emkornfield emkornfield Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't look like there is a check to ensure writing succeeds? could you also add one here?

Copy link
Contributor Author

@tianchen92 tianchen92 Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, fixed.

public void testWriteDecimal() throws Exception {
try (final FixedSizeListVector vector = FixedSizeListVector.empty("vector", /*listSize=*/3, allocator)) {

UnionFixedSizeListWriter writer = vector.getWriter();
writer.allocate();

final int valueCount = 100;

for (int i = 0; i < valueCount; i++) {
writer.startList();
writer.decimal().writeDecimal(new BigDecimal(i));
writer.decimal().writeDecimal(new BigDecimal(i * 2));
writer.decimal().writeDecimal(new BigDecimal(i * 3));
writer.endList();
}
vector.setValueCount(valueCount);

for (int i = 0; i < valueCount; i++) {
List<BigDecimal> values = (List<BigDecimal>) vector.getObject(i);
assertEquals(3, values.size());
assertEquals(new BigDecimal(i), values.get(0));
assertEquals(new BigDecimal(i * 2), values.get(1));
assertEquals(new BigDecimal(i * 3), values.get(2));
}
}
}

@Test
public void testDecimalIndexCheck() throws Exception {
try (final FixedSizeListVector vector = FixedSizeListVector.empty("vector", /*listSize=*/3, allocator)) {

UnionFixedSizeListWriter writer = vector.getWriter();
writer.allocate();

IllegalStateException e = assertThrows(IllegalStateException.class, () -> {
writer.startList();
writer.decimal().writeDecimal(new BigDecimal(1));
writer.decimal().writeDecimal(new BigDecimal(2));
writer.decimal().writeDecimal(new BigDecimal(3));
writer.decimal().writeDecimal(new BigDecimal(4));
writer.endList();
});
assertEquals("values at index 0 is greater than listSize 3", e.getMessage());
}
}


@Test(expected = IllegalStateException.class)
public void testWriteIllegalData() throws Exception {
try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", 3, allocator)) {
Expand Down