You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by em...@apache.org on 2019/07/20 05:08:52 UTC
[arrow] branch master updated: ARROW-5973: [Java] Variable width
vectors' get methods should return null when the underlying data is null
This is an automated email from the ASF dual-hosted git repository.
emkornfield pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 9644853 ARROW-5973: [Java] Variable width vectors' get methods should return null when the underlying data is null
9644853 is described below
commit 96448532a653dacf8250c771878e20a606d3526f
Author: liyafan82 <fa...@foxmail.com>
AuthorDate: Fri Jul 19 22:08:00 2019 -0700
ARROW-5973: [Java] Variable width vectors' get methods should return null when the underlying data is null
For variable-width vectors (VarCharVector and VarBinaryVector), when the validity bit is not set, it means the underlying data is null, so the get method should return null.
However, the current implementation throws an IllegalStateException when NULL_CHECKING_ENABLED is set, or returns an empty array when the flag is clear.
Maybe the purpose of this design is to be consistent with fixed-width vectors. However, the scenario is different: fixed-width vectors (e.g. IntVector) throw an IllegalStateException, simply because the primitive types are non-nullable.
Author: liyafan82 <fa...@foxmail.com>
Closes #4901 from liyafan82/fly_0717_varget and squashes the following commits:
8fe83f751 <liyafan82> Variable width vectors' get methods should return null when the underlying data is null
---
.../apache/arrow/vector/FixedSizeBinaryVector.java | 15 ++--------
.../org/apache/arrow/vector/VarBinaryVector.java | 14 ++--------
.../org/apache/arrow/vector/VarCharVector.java | 17 ++++--------
.../arrow/vector/TestFixedSizeBinaryVector.java | 6 ++++
.../org/apache/arrow/vector/TestValueVector.java | 32 ++++++++++++----------
5 files changed, 35 insertions(+), 49 deletions(-)
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/FixedSizeBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/FixedSizeBinaryVector.java
index 61bd57c..b7cce56 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/FixedSizeBinaryVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/FixedSizeBinaryVector.java
@@ -17,8 +17,6 @@
package org.apache.arrow.vector;
-import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
-
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.impl.FixedSizeBinaryReaderImpl;
import org.apache.arrow.vector.complex.reader.FieldReader;
@@ -114,8 +112,8 @@ public class FixedSizeBinaryVector extends BaseFixedWidthVector {
*/
public byte[] get(int index) {
assert index >= 0;
- if (NULL_CHECKING_ENABLED && isSet(index) == 0) {
- throw new IllegalStateException("Value at index is null");
+ if (isSet(index) == 0) {
+ return null;
}
final byte[] dst = new byte[byteWidth];
valueBuffer.getBytes(index * byteWidth, dst, 0, byteWidth);
@@ -148,14 +146,7 @@ public class FixedSizeBinaryVector extends BaseFixedWidthVector {
*/
@Override
public byte[] getObject(int index) {
- assert index >= 0;
- if (isSet(index) == 0) {
- return null;
- } else {
- final byte[] dst = new byte[byteWidth];
- valueBuffer.getBytes(index * byteWidth, dst, 0, byteWidth);
- return dst;
- }
+ return get(index);
}
public int getByteWidth() {
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java
index bd76f3c..14c6c9a 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java
@@ -17,8 +17,6 @@
package org.apache.arrow.vector;
-import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
-
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.impl.VarBinaryReaderImpl;
import org.apache.arrow.vector.complex.reader.FieldReader;
@@ -109,8 +107,8 @@ public class VarBinaryVector extends BaseVariableWidthVector {
*/
public byte[] get(int index) {
assert index >= 0;
- if (NULL_CHECKING_ENABLED && isSet(index) == 0) {
- throw new IllegalStateException("Value at index is null");
+ if (isSet(index) == 0) {
+ return null;
}
final int startOffset = getStartOffset(index);
final int dataLength =
@@ -127,13 +125,7 @@ public class VarBinaryVector extends BaseVariableWidthVector {
* @return byte array for non-null element, null otherwise
*/
public byte[] getObject(int index) {
- byte[] b;
- try {
- b = get(index);
- } catch (IllegalStateException e) {
- return null;
- }
- return b;
+ return get(index);
}
/**
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java
index 7a914d9..1f21a57 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java
@@ -17,8 +17,6 @@
package org.apache.arrow.vector;
-import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
-
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.impl.VarCharReaderImpl;
import org.apache.arrow.vector.complex.reader.FieldReader;
@@ -106,8 +104,8 @@ public class VarCharVector extends BaseVariableWidthVector {
*/
public byte[] get(int index) {
assert index >= 0;
- if (NULL_CHECKING_ENABLED && isSet(index) == 0) {
- throw new IllegalStateException("Value at index is null");
+ if (isSet(index) == 0) {
+ return null;
}
final int startOffset = getStartOffset(index);
final int dataLength =
@@ -124,15 +122,12 @@ public class VarCharVector extends BaseVariableWidthVector {
* @return Text object for non-null element, null otherwise
*/
public Text getObject(int index) {
- Text result = new Text();
- byte[] b;
- try {
- b = get(index);
- } catch (IllegalStateException e) {
+ byte[] b = get(index);
+ if (b == null) {
return null;
+ } else {
+ return new Text(b);
}
- result.set(b);
- return result;
}
/**
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeBinaryVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeBinaryVector.java
index 14476a1..d834d03 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeBinaryVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeBinaryVector.java
@@ -258,4 +258,10 @@ public class TestFixedSizeBinaryVector {
vector.setSafe(0, largeNullableHolder);
vector.setSafe(0, largeBuf);
}
+
+ @Test
+ public void testGetNull() {
+ vector.setNull(0);
+ assertNull(vector.get(0));
+ }
}
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
index 0c1ae54..f8f9c76 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
@@ -906,13 +906,7 @@ public class TestValueVector {
// Ensure null value throws.
boolean b = false;
- try {
- vector.get(8);
- } catch (IllegalStateException e) {
- b = true;
- } finally {
- assertTrue(b);
- }
+ assertNull(vector.get(8));
}
}
@@ -942,14 +936,7 @@ public class TestValueVector {
assertArrayEquals(Arrays.copyOfRange(STR3, 2, STR3.length), vector.get(6));
// Ensure null value throws.
- boolean b = false;
- try {
- vector.get(7);
- } catch (IllegalStateException e) {
- b = true;
- } finally {
- assertTrue(b);
- }
+ assertNull(vector.get(7));
}
}
@@ -2173,4 +2160,19 @@ public class TestValueVector {
buf.close();
}
}
+
+ @Test
+ public void testGetNullFromVariableWidthVector() {
+ try (VarCharVector varCharVector = new VarCharVector("varcharvec", allocator);
+ VarBinaryVector varBinaryVector = new VarBinaryVector("varbinary", allocator)) {
+ varCharVector.allocateNew(10, 1);
+ varBinaryVector.allocateNew(10, 1);
+
+ varCharVector.setNull(0);
+ varBinaryVector.setNull(0);
+
+ assertNull(varCharVector.get(0));
+ assertNull(varBinaryVector.get(0));
+ }
+ }
}