From 8f2df78a674f13f08fcc2b09605e68a5cfcc6d64 Mon Sep 17 00:00:00 2001 From: tianchen Date: Wed, 16 Oct 2019 15:16:57 +0800 Subject: [PATCH 1/4] ARROW-XXXX: [Java] UnionFixedSizeListWriter decimal type should check writer index --- .../templates/UnionFixedSizeListWriter.java | 11 ++++++++++- .../arrow/vector/TestFixedSizeListVector.java | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java b/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java index e7f3e3935d2..461e1abbbe0 100644 --- a/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java +++ b/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java @@ -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 exceed 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 exceed 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 exceed listSize %s", idx(), listSize)); + } writer.writeDecimal(value); writer.setPosition(writer.idx() + 1); } @@ -197,7 +206,7 @@ public void writeDecimal(BigDecimal value) { @Override public void write${name}(<#list fields as field>${field.type} ${field.name}<#if field_has_next>, ) { if (writer.idx() >= (idx() + 1) * listSize) { - throw new IllegalStateException(String.format("values at index %s is greater than listSize %s", idx(), listSize)); + throw new IllegalStateException(String.format("values at index %s exceed listSize %s", idx(), listSize)); } writer.write${name}(<#list fields as field>${field.name}<#if field_has_next>, ); writer.setPosition(writer.idx() + 1); diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java index 6843daf5cf0..bc230f2d841 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.math.BigDecimal; import java.util.Arrays; import org.apache.arrow.memory.BufferAllocator; @@ -265,6 +266,23 @@ public void testUnionFixedSizeListWriter() throws Exception { } } + @Test(expected = IllegalStateException.class) + public void testDecimalIndexCheck() throws Exception { + try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", 3, allocator)) { + + UnionFixedSizeListWriter writer1 = vector1.getWriter(); + writer1.allocate(); + + writer1.startList(); + writer1.decimal().writeDecimal(new BigDecimal(1)); + writer1.decimal().writeDecimal(new BigDecimal(2)); + writer1.decimal().writeDecimal(new BigDecimal(3)); + writer1.decimal().writeDecimal(new BigDecimal(4)); + writer1.endList(); + } + } + + @Test(expected = IllegalStateException.class) public void testWriteIllegalData() throws Exception { try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", 3, allocator)) { @@ -285,6 +303,7 @@ public void testWriteIllegalData() throws Exception { assertTrue(Arrays.equals(values1, realValue1)); int[] realValue2 = convertListToIntArray((JsonStringArrayList) vector1.getObject(1)); assertTrue(Arrays.equals(values2, realValue2)); + System.out.println("hehe"); } } From 5b66dcb5eec2282aa17b7c4fde462d4f75e92a29 Mon Sep 17 00:00:00 2001 From: tianchen Date: Mon, 11 Nov 2019 20:40:11 +0800 Subject: [PATCH 2/4] fix test --- .../arrow/vector/TestFixedSizeListVector.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java index bc230f2d841..8cb5a187f92 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java @@ -19,6 +19,7 @@ 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; @@ -266,19 +267,22 @@ public void testUnionFixedSizeListWriter() throws Exception { } } - @Test(expected = IllegalStateException.class) + @Test public void testDecimalIndexCheck() throws Exception { try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", 3, allocator)) { UnionFixedSizeListWriter writer1 = vector1.getWriter(); writer1.allocate(); - writer1.startList(); - writer1.decimal().writeDecimal(new BigDecimal(1)); - writer1.decimal().writeDecimal(new BigDecimal(2)); - writer1.decimal().writeDecimal(new BigDecimal(3)); - writer1.decimal().writeDecimal(new BigDecimal(4)); - writer1.endList(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> { + writer1.startList(); + writer1.decimal().writeDecimal(new BigDecimal(1)); + writer1.decimal().writeDecimal(new BigDecimal(2)); + writer1.decimal().writeDecimal(new BigDecimal(3)); + writer1.decimal().writeDecimal(new BigDecimal(4)); + writer1.endList(); + }); + assertEquals("values at index 0 exceed listSize 3", e.getMessage()); } } @@ -303,7 +307,6 @@ public void testWriteIllegalData() throws Exception { assertTrue(Arrays.equals(values1, realValue1)); int[] realValue2 = convertListToIntArray((JsonStringArrayList) vector1.getObject(1)); assertTrue(Arrays.equals(values2, realValue2)); - System.out.println("hehe"); } } From a17dded56a0e0bd2b1600081744e00be416732ef Mon Sep 17 00:00:00 2001 From: tianchen Date: Tue, 12 Nov 2019 15:15:12 +0800 Subject: [PATCH 3/4] resolve comments --- .../main/codegen/templates/UnionFixedSizeListWriter.java | 8 ++++---- .../org/apache/arrow/vector/TestFixedSizeListVector.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java b/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java index 461e1abbbe0..013d60e1db7 100644 --- a/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java +++ b/java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java @@ -175,7 +175,7 @@ public void end() { @Override public void write(DecimalHolder holder) { if (writer.idx() >= (idx() + 1) * listSize) { - throw new IllegalStateException(String.format("values at index %s exceed listSize %s", idx(), 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); @@ -183,7 +183,7 @@ public void write(DecimalHolder holder) { public void writeDecimal(int start, ArrowBuf buffer) { if (writer.idx() >= (idx() + 1) * listSize) { - throw new IllegalStateException(String.format("values at index %s exceed listSize %s", idx(), 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); @@ -191,7 +191,7 @@ public void writeDecimal(int start, ArrowBuf buffer) { public void writeDecimal(BigDecimal value) { if (writer.idx() >= (idx() + 1) * listSize) { - throw new IllegalStateException(String.format("values at index %s exceed listSize %s", idx(), 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); @@ -206,7 +206,7 @@ public void writeDecimal(BigDecimal value) { @Override public void write${name}(<#list fields as field>${field.type} ${field.name}<#if field_has_next>, ) { if (writer.idx() >= (idx() + 1) * listSize) { - throw new IllegalStateException(String.format("values at index %s exceed listSize %s", idx(), listSize)); + throw new IllegalStateException(String.format("values at index %s is greater than listSize %s", idx(), listSize)); } writer.write${name}(<#list fields as field>${field.name}<#if field_has_next>, ); writer.setPosition(writer.idx() + 1); diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java index 8cb5a187f92..b2d6b8bf648 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java @@ -269,7 +269,7 @@ public void testUnionFixedSizeListWriter() throws Exception { @Test public void testDecimalIndexCheck() throws Exception { - try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", 3, allocator)) { + try (final FixedSizeListVector vector1 = FixedSizeListVector.empty("vector", /*listSize=*/3, allocator)) { UnionFixedSizeListWriter writer1 = vector1.getWriter(); writer1.allocate(); @@ -282,7 +282,7 @@ public void testDecimalIndexCheck() throws Exception { writer1.decimal().writeDecimal(new BigDecimal(4)); writer1.endList(); }); - assertEquals("values at index 0 exceed listSize 3", e.getMessage()); + assertEquals("values at index 0 is greater than listSize 3", e.getMessage()); } } From 1268aec6a8e2c24275519c427f73c723718da511 Mon Sep 17 00:00:00 2001 From: tianchen Date: Fri, 15 Nov 2019 16:52:06 +0800 Subject: [PATCH 4/4] add write decimal tests --- .../arrow/vector/TestFixedSizeListVector.java | 47 +++++++++++++++---- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java index b2d6b8bf648..df3ba85a100 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java @@ -23,6 +23,7 @@ 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; @@ -267,20 +268,48 @@ public void testUnionFixedSizeListWriter() throws Exception { } } + @Test + 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 values = (List) 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 vector1 = FixedSizeListVector.empty("vector", /*listSize=*/3, allocator)) { + try (final FixedSizeListVector vector = FixedSizeListVector.empty("vector", /*listSize=*/3, allocator)) { - UnionFixedSizeListWriter writer1 = vector1.getWriter(); - writer1.allocate(); + UnionFixedSizeListWriter writer = vector.getWriter(); + writer.allocate(); IllegalStateException e = assertThrows(IllegalStateException.class, () -> { - writer1.startList(); - writer1.decimal().writeDecimal(new BigDecimal(1)); - writer1.decimal().writeDecimal(new BigDecimal(2)); - writer1.decimal().writeDecimal(new BigDecimal(3)); - writer1.decimal().writeDecimal(new BigDecimal(4)); - writer1.endList(); + 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()); }