You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2019/07/11 07:09:31 UTC
[arrow] branch master updated: ARROW-5842: [Java] Revise the
semantic of lastSet in ListVector
This is an automated email from the ASF dual-hosted git repository.
ravindra 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 f569bf6 ARROW-5842: [Java] Revise the semantic of lastSet in ListVector
f569bf6 is described below
commit f569bf691e56b4cf5c1352c0ac67519ec8843e47
Author: liyafan82 <fa...@foxmail.com>
AuthorDate: Thu Jul 11 12:39:04 2019 +0530
ARROW-5842: [Java] Revise the semantic of lastSet in ListVector
The lastSet member in ListVector seems misleading. According to the name, it should refers to the last index that is actually set. However, from the context of the code, it actually means the next index that will be set.
We fix this problem, and make it consistent with the lastSet in BaseVariableWidthVector.
Author: liyafan82 <fa...@foxmail.com>
Closes #4797 from liyafan82/fly_0704_lastset and squashes the following commits:
6043263d9 <liyafan82> Revise external semantics
56a9ef336 <liyafan82> Revise the semantic of lastSet in ListVector
---
.../apache/arrow/vector/complex/ListVector.java | 26 +++++++++++++---------
.../org/apache/arrow/vector/TestListVector.java | 12 +++++-----
.../org/apache/arrow/vector/TestMapVector.java | 4 ++--
.../org/apache/arrow/vector/TestVectorReset.java | 2 +-
.../org/apache/arrow/vector/ipc/BaseFileTest.java | 2 +-
5 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
index abbb2f6..af5333c 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
@@ -72,6 +72,10 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
private CallBack callBack;
private final FieldType fieldType;
private int validityAllocationSizeInBytes;
+
+ /**
+ * The maximum index that is actually set.
+ */
private int lastSet;
/**
@@ -105,7 +109,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
this.fieldType = checkNotNull(fieldType);
this.callBack = callBack;
this.validityAllocationSizeInBytes = getValidityBufferSizeFromCount(INITIAL_VALUE_ALLOCATION);
- this.lastSet = 0;
+ this.lastSet = -1;
}
@Override
@@ -200,7 +204,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
validityAllocationSizeInBytes = validityBuffer.capacity();
offsetAllocationSizeInBytes = offsetBuffer.capacity();
- lastSet = fieldNode.getLength();
+ lastSet = fieldNode.getLength() - 1;
valueCount = fieldNode.getLength();
}
@@ -465,7 +469,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
splitAndTransferValidityBuffer(startIndex, length, to);
/* splitAndTransfer data buffer */
dataTransferPair.splitAndTransfer(startPoint, sliceLength);
- to.lastSet = length;
+ to.lastSet = length - 1;
to.setValueCount(length);
}
@@ -580,14 +584,14 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
public void clear() {
super.clear();
validityBuffer = releaseBuffer(validityBuffer);
- lastSet = 0;
+ lastSet = -1;
}
@Override
public void reset() {
super.reset();
validityBuffer.setZero(0, validityBuffer.capacity());
- lastSet = 0;
+ lastSet = -1;
}
/**
@@ -720,7 +724,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
reallocValidityAndOffsetBuffers();
}
BitVectorHelper.setValidityBitToOne(validityBuffer, index);
- lastSet = index + 1;
+ lastSet = index;
}
/**
@@ -733,13 +737,13 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
while (index >= getValidityAndOffsetValueCapacity()) {
reallocValidityAndOffsetBuffers();
}
- for (int i = lastSet; i <= index; i++) {
+ for (int i = lastSet + 1; i <= index; i++) {
final int currentOffset = offsetBuffer.getInt(i * OFFSET_WIDTH);
offsetBuffer.setInt((i + 1) * OFFSET_WIDTH, currentOffset);
}
BitVectorHelper.setValidityBitToOne(validityBuffer, index);
- lastSet = index + 1;
- return offsetBuffer.getInt(lastSet * OFFSET_WIDTH);
+ lastSet = index;
+ return offsetBuffer.getInt((lastSet + 1) * OFFSET_WIDTH);
}
/**
@@ -766,7 +770,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
/* check if validity and offset buffers need to be re-allocated */
reallocValidityAndOffsetBuffers();
}
- for (int i = lastSet; i < valueCount; i++) {
+ for (int i = lastSet + 1; i < valueCount; i++) {
/* fill the holes with offsets */
final int currentOffset = offsetBuffer.getInt(i * OFFSET_WIDTH);
offsetBuffer.setInt((i + 1) * OFFSET_WIDTH, currentOffset);
@@ -774,7 +778,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
}
/* valueCount for the data vector is the current end offset */
final int childValueCount = (valueCount == 0) ? 0 :
- offsetBuffer.getInt(lastSet * OFFSET_WIDTH);
+ offsetBuffer.getInt((lastSet + 1) * OFFSET_WIDTH);
/* set the value count of data vector and this will take care of
* checking whether data buffer needs to be reallocated.
*/
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestListVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestListVector.java
index 68102b1..96372c0 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestListVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestListVector.java
@@ -135,7 +135,7 @@ public class TestListVector {
BigIntVector dataVector = (BigIntVector) listVector.getDataVector();
/* check current lastSet */
- assertEquals(Integer.toString(0), Integer.toString(listVector.getLastSet()));
+ assertEquals(Integer.toString(-1), Integer.toString(listVector.getLastSet()));
int index = 0;
int offset = 0;
@@ -165,7 +165,7 @@ public class TestListVector {
offsetBuffer.setInt((index + 1) * ListVector.OFFSET_WIDTH, 8);
/* check current lastSet */
- assertEquals(Integer.toString(0), Integer.toString(listVector.getLastSet()));
+ assertEquals(Integer.toString(-1), Integer.toString(listVector.getLastSet()));
/* set lastset and arbitrary valuecount for list vector.
*
@@ -206,7 +206,7 @@ public class TestListVector {
* [15, 16, 17]
* }
*/
- listVector.setLastSet(3);
+ listVector.setLastSet(2);
listVector.setValueCount(10);
/* (3+2+3)/10 */
@@ -307,7 +307,7 @@ public class TestListVector {
listVector.setValueCount(5);
- assertEquals(5, listVector.getLastSet());
+ assertEquals(4, listVector.getLastSet());
/* get offset buffer */
final ArrowBuf offsetBuffer = listVector.getOffsetBuffer();
@@ -501,7 +501,7 @@ public class TestListVector {
listWriter.endList();
- assertEquals(2, listVector.getLastSet());
+ assertEquals(1, listVector.getLastSet());
listVector.setValueCount(2);
@@ -635,7 +635,7 @@ public class TestListVector {
listWriter.endList();
- assertEquals(2, listVector.getLastSet());
+ assertEquals(1, listVector.getLastSet());
listVector.setValueCount(2);
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java
index a16390c..55a0a41 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java
@@ -324,7 +324,7 @@ public class TestMapVector {
mapVector.setValueCount(5);
- assertEquals(5, mapVector.getLastSet());
+ assertEquals(4, mapVector.getLastSet());
/* get offset buffer */
final ArrowBuf offsetBuffer = mapVector.getOffsetBuffer();
@@ -552,7 +552,7 @@ public class TestMapVector {
mapWriter.endMap();
- assertEquals(2, mapVector.getLastSet());
+ assertEquals(1, mapVector.getLastSet());
mapWriter.setValueCount(2);
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReset.java b/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReset.java
index f8643b8..93ebf6c 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReset.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReset.java
@@ -108,7 +108,7 @@ public class TestVectorReset {
variableList.endValue(0, 0);
variableList.setValueCount(1);
resetVectorAndVerify(variableList, variableList.getBuffers(false));
- assertEquals(0, variableList.getLastSet());
+ assertEquals(-1, variableList.getLastSet());
// FixedSizeListVector
fixedList.allocateNewSafe();
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/ipc/BaseFileTest.java b/java/vector/src/test/java/org/apache/arrow/vector/ipc/BaseFileTest.java
index 83d15d1..21b9c44 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/ipc/BaseFileTest.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/ipc/BaseFileTest.java
@@ -643,7 +643,7 @@ public class BaseFileTest {
}
// ListVector lastSet should be the index of last value + 1
- Assert.assertEquals(listVector.getLastSet(), count);
+ Assert.assertEquals(listVector.getLastSet(), count - 1);
// VarBinaryVector lastSet should be the index of last value
VarBinaryVector binaryVector = (VarBinaryVector) listVector.getChildrenFromFields().get(0);