From 3423ad4f49a7fbf75ca4b2ba76ed9b6e7729aecf Mon Sep 17 00:00:00 2001 From: Li Jin Date: Wed, 29 Nov 2017 14:09:06 -0500 Subject: [PATCH] Remove accessor and mutator interface --- .../codegen/templates/ComplexWriters.java | 6 +- .../main/codegen/templates/UnionVector.java | 12 -- .../arrow/vector/BaseDataValueVector.java | 2 +- .../arrow/vector/BaseFixedWidthVector.java | 17 +-- .../apache/arrow/vector/BaseValueVector.java | 63 ----------- .../arrow/vector/BaseVariableWidthVector.java | 12 -- .../org/apache/arrow/vector/ValueVector.java | 107 +++++------------- .../arrow/vector/VariableWidthVector.java | 12 -- .../apache/arrow/vector/VectorSchemaRoot.java | 2 +- .../org/apache/arrow/vector/ZeroVector.java | 45 -------- .../complex/BaseRepeatedValueVector.java | 12 -- .../vector/complex/FixedSizeListVector.java | 12 -- .../arrow/vector/complex/MapVector.java | 12 -- .../vector/complex/NonNullableMapVector.java | 10 -- .../vector/complex/RepeatedValueVector.java | 39 ------- .../impl/UnionFixedSizeListReader.java | 2 +- .../arrow/vector/TestVectorReAlloc.java | 2 +- .../apache/arrow/vector/ipc/BaseFileTest.java | 2 - 18 files changed, 41 insertions(+), 328 deletions(-) diff --git a/java/vector/src/main/codegen/templates/ComplexWriters.java b/java/vector/src/main/codegen/templates/ComplexWriters.java index 98672d54cc3..4a1545a173d 100644 --- a/java/vector/src/main/codegen/templates/ComplexWriters.java +++ b/java/vector/src/main/codegen/templates/ComplexWriters.java @@ -79,17 +79,17 @@ protected int idx() { public void write(${minor.class?cap_first}Holder h) { mutator.addSafe(idx(), h); - vector.getMutator().setValueCount(idx()+1); + vector.setValueCount(idx()+1); } public void write(${minor.class?cap_first}Holder h) { mutator.addSafe(idx(), h); - vector.getMutator().setValueCount(idx()+1); + vector.setValueCount(idx()+1); } public void write${minor.class}(<#list fields as field>${field.type} ${field.name}<#if field_has_next>, ) { mutator.addSafe(idx(), <#list fields as field>${field.name}<#if field_has_next>, ); - vector.getMutator().setValueCount(idx()+1); + vector.setValueCount(idx()+1); } public void setPosition(int idx) { diff --git a/java/vector/src/main/codegen/templates/UnionVector.java b/java/vector/src/main/codegen/templates/UnionVector.java index 501933f8fbc..60cd24dcc44 100644 --- a/java/vector/src/main/codegen/templates/UnionVector.java +++ b/java/vector/src/main/codegen/templates/UnionVector.java @@ -416,18 +416,6 @@ public void copyValueSafe(int from, int to) { } } - @Override - @Deprecated - public Accessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from UNION"); - } - - @Override - @Deprecated - public Mutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing to UNION"); - } - @Override public FieldReader getReader() { if (reader == null) { diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java index 6d9eb1db03a..80675135e3c 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java @@ -99,7 +99,7 @@ public ArrowBuf[] getBuffers(boolean clear) { @Override public int getBufferSize() { - if (getAccessor().getValueCount() == 0) { + if (getValueCount() == 0) { return 0; } return data.writerIndex(); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java index cc056904ae8..77026d4e5a1 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java @@ -91,19 +91,6 @@ public BaseFixedWidthVector(final String name, final BufferAllocator allocator, * the top class as of now is not a good idea. */ - - @Override - @Deprecated - public Mutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing to vector"); - } - - @Override - @Deprecated - public Accessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from vector"); - } - /** * Get the memory address of buffer that manages the validity * (NULL or NON-NULL nature) of elements in the vector. @@ -711,6 +698,7 @@ private void splitAndTransferValidityBuffer(int startIndex, int length, * * @return the number of null elements. */ + @Override public int getNullCount() { return BitVectorHelper.getNullCount(validityBuffer, valueCount); } @@ -721,6 +709,7 @@ public int getNullCount() { * * @return valueCount for the vector */ + @Override public int getValueCount() { return valueCount; } @@ -730,6 +719,7 @@ public int getValueCount() { * * @param valueCount value count to set */ + @Override public void setValueCount(int valueCount) { this.valueCount = valueCount; final int currentValueCapacity = getValueCapacity(); @@ -789,6 +779,7 @@ public boolean isSafe(int index) { * @param index position of element * @return true if element at given index is null, false otherwise */ + @Override public boolean isNull(int index) { return (isSet(index) == 0); } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java index fc0ab3e5915..6418ea4f8a0 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java @@ -68,42 +68,6 @@ public TransferPair getTransferPair(BufferAllocator allocator) { return getTransferPair(name, allocator); } - public abstract static class BaseAccessor implements ValueVector.Accessor { - protected BaseAccessor() { - } - - @Override - public boolean isNull(int index) { - return false; - } - - @Override - // override this in case your implementation is faster, see BitVector - public int getNullCount() { - int nullCount = 0; - for (int i = 0; i < getValueCount(); i++) { - if (isNull(i)) { - nullCount++; - } - } - return nullCount; - } - } - - public abstract static class BaseMutator implements ValueVector.Mutator { - protected BaseMutator() { - } - - @Override - public void generateTestData(int values) { - } - - //TODO: consider making mutator stateless(if possible) on another issue. - @Override - public void reset() { - } - } - @Override public Iterator iterator() { return Collections.emptyIterator(); @@ -136,33 +100,6 @@ protected ArrowBuf releaseBuffer(ArrowBuf buffer) { return buffer; } - @Override - public int getValueCount() { - return getAccessor().getValueCount(); - } - - @Override - public void setValueCount(int valueCount) { - getMutator().setValueCount(valueCount); - } - - @Override - - public Object getObject(int index) { - return getAccessor().getObject(index); - } - - @Override - - public int getNullCount() { - return getAccessor().getNullCount(); - } - - @Override - public boolean isNull(int index) { - return getAccessor().isNull(index); - } - /* number of bytes for the validity buffer for the given valueCount */ protected static int getValidityBufferSizeFromCount(final int valueCount) { return (int) Math.ceil(valueCount / 8.0); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java index b515586bc9f..98820a2367b 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java @@ -90,18 +90,6 @@ public BaseVariableWidthVector(final String name, final BufferAllocator allocato * the top class as of now is not a good idea. */ - @Override - @Deprecated - public VariableWidthMutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing into vector"); - } - - @Override - @Deprecated - public VariableWidthAccessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from vector"); - } - /** * Get buffer that manages the validity (NULL or NON-NULL nature) of * elements in the vector. Consider it as a buffer for internal bit vector diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java index 0d3acf1646f..e77c1b12703 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java @@ -135,18 +135,6 @@ public interface ValueVector extends Closeable, Iterable { */ TransferPair makeTransferPair(ValueVector target); - /** - * @return an {@link org.apache.arrow.vector.ValueVector.Accessor accessor} that is used to read from this vector - * instance. - */ - Accessor getAccessor(); - - /** - * @return an {@link org.apache.arrow.vector.ValueVector.Mutator mutator} that is used to write to this vector - * instance. - */ - Mutator getMutator(); - /** * @return a {@link org.apache.arrow.vector.complex.reader.FieldReader field reader} that supports reading values * from this vector. @@ -160,7 +148,7 @@ public interface ValueVector extends Closeable, Iterable { /** * Returns the number of bytes that is used by this vector if it holds the given number - * of values. The result will be the same as if Mutator.setValueCount() were called, followed + * of values. The result will be the same as if setValueCount() were called, followed * by calling getBufferSize(), but without any of the closing side-effects that setValueCount() * implies wrt finishing off the population of a vector. Some operations might wish to use * this to determine how much memory has been used by a vector so far, even though it is @@ -182,91 +170,56 @@ public interface ValueVector extends Closeable, Iterable { */ ArrowBuf[] getBuffers(boolean clear); - /** - * An abstraction that is used to read from this vector instance. - */ - interface Accessor { - /** - * Get the Java Object representation of the element at the specified position. Useful for testing. - * - * @param index Index of the value to get - * @return the friendly java type - */ - Object getObject(int index); - - /** - * @return the number of values that is stored in this vector. - */ - int getValueCount(); - - /** - * @param index the index to check for nullity - * @return true if the value at the given index is null, false otherwise. - */ - boolean isNull(int index); - - /** - * @return the number of null values - */ - int getNullCount(); - } - - /** - * An abstraction that is used to write into this vector instance. - */ - interface Mutator { - /** - * Sets the number of values that is stored in this vector to the given value count. - * - * @param valueCount value count to set. - */ - void setValueCount(int valueCount); - - /** - * Resets the mutator to pristine state. - */ - void reset(); - - /** - * @param values the number of values to generate - * @deprecated this has nothing to do with value vector abstraction and should be removed. - */ - @Deprecated - void generateTestData(int values); - } - /** * Gets the underlying buffer associated with validity vector * * @return buffer */ - public ArrowBuf getValidityBuffer(); + ArrowBuf getValidityBuffer(); /** * Gets the underlying buffer associated with data vector * * @return buffer */ - public ArrowBuf getDataBuffer(); + ArrowBuf getDataBuffer(); /** * Gets the underlying buffer associated with offset vector * * @return buffer */ - public ArrowBuf getOffsetBuffer(); + ArrowBuf getOffsetBuffer(); - /* temporarily add these methods here until we remove other vectors - * (non-nullable) which are under ValueVector hierarchy and still - * use the mutator/accessor interfaces. + /** + * Gets the number of values + * @return */ - public int getValueCount(); + int getValueCount(); - public void setValueCount(int valueCount); + /** + * Set number of values in the vector + * @return + */ + void setValueCount(int valueCount); - public Object getObject(int index); + /** + * Get friendly type object from the vector + * @param index + * @return + */ + Object getObject(int index); - public int getNullCount(); + /** + * Returns number of null elements in the vector + * @return + */ + int getNullCount(); - public boolean isNull(int index); + /** + * Check whether an element in the vector is null + * @param index + * @return + */ + boolean isNull(int index); } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VariableWidthVector.java index 04c00b7c834..593d4dceaf7 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/VariableWidthVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/VariableWidthVector.java @@ -35,17 +35,5 @@ public interface VariableWidthVector extends ValueVector { */ int getByteCapacity(); - VariableWidthMutator getMutator(); - - VariableWidthAccessor getAccessor(); - - interface VariableWidthAccessor extends Accessor { - int getValueLength(int index); - } - int getCurrentSizeInBytes(); - - interface VariableWidthMutator extends Mutator { - void setValueLengthSafe(int index, int length); - } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java b/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java index 196393fb958..3fd33d66d4e 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java @@ -140,7 +140,7 @@ public String contentToTSVString() { for (int i = 0; i < rowCount; i++) { row.clear(); for (FieldVector v : fieldVectors) { - row.add(v.getAccessor().getObject(i)); + row.add(v.getObject(i)); } printRow(sb, row); } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java index 0ab3a7b6843..962a1c94707 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java @@ -60,41 +60,6 @@ public void copyValueSafe(int from, int to) { } }; - private final Accessor defaultAccessor = new Accessor() { - @Override - public Object getObject(int index) { - return null; - } - - @Override - public int getValueCount() { - return 0; - } - - @Override - public boolean isNull(int index) { - return true; - } - - @Override - public int getNullCount() { - return 0; - } - }; - - private final Mutator defaultMutator = new Mutator() { - @Override - public void setValueCount(int valueCount) { - } - - @Override - public void reset() { - } - - @Override - public void generateTestData(int values) { - } - }; public ZeroVector() { } @@ -186,16 +151,6 @@ public TransferPair makeTransferPair(ValueVector target) { return defaultPair; } - @Override - public Accessor getAccessor() { - return defaultAccessor; - } - - @Override - public Mutator getMutator() { - return defaultMutator; - } - @Override public FieldReader getReader() { return NullReader.INSTANCE; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/BaseRepeatedValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/BaseRepeatedValueVector.java index 4648d078949..a9221f2f6ea 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/BaseRepeatedValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/BaseRepeatedValueVector.java @@ -277,16 +277,4 @@ public void setValueCount(int valueCount) { offsetBuffer.getInt(valueCount * OFFSET_WIDTH); vector.setValueCount(childValueCount); } - - @Override - @Deprecated - public RepeatedAccessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from LIST."); - } - - @Override - @Deprecated - public RepeatedMutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing to LIST"); - } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java index 774a10dbfb9..93a8127cfd3 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java @@ -150,18 +150,6 @@ public List getFieldInnerVectors() { throw new UnsupportedOperationException("There are no inner vectors. Use getFieldBuffers"); } - @Override - @Deprecated - public Accessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from vector"); - } - - @Override - @Deprecated - public Mutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing to vector"); - } - @Override public UnionFixedSizeListReader getReader() { return reader; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java index 075ae83ea4d..72cc29f30ca 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java @@ -497,16 +497,4 @@ public void setValueCount(int valueCount) { public void reset() { valueCount = 0; } - - @Override - @Deprecated - public Accessor getAccessor() { - throw new UnsupportedOperationException("Accessor is not supported for reading from Nullable MAP"); - } - - @Override - @Deprecated - public Mutator getMutator() { - throw new UnsupportedOperationException("Mutator is not supported for writing to Nullable MAP"); - } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java index cc3ac4148a8..1cd7b70d7bf 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java @@ -252,16 +252,6 @@ public int compare(@Nullable ValueVector left, @Nullable ValueVector right) { return natural.min(getChildren()).getValueCapacity(); } - @Override - public Accessor getAccessor() { - throw new UnsupportedOperationException("accessor is not needed for MAP"); - } - - @Override - public Mutator getMutator() { - throw new UnsupportedOperationException("mutator is not needed for MAP"); - } - @Override public Object getObject(int index) { Map vv = new JsonStringHashMap<>(); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/RepeatedValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/RepeatedValueVector.java index 91147c663f2..36401172994 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/RepeatedValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/RepeatedValueVector.java @@ -41,43 +41,4 @@ public interface RepeatedValueVector extends ValueVector { * @return the underlying data vector or null if none exists. */ ValueVector getDataVector(); - - @Override - RepeatedAccessor getAccessor(); - - @Override - RepeatedMutator getMutator(); - - interface RepeatedAccessor extends ValueVector.Accessor { - /** - * The result includes empty, null valued cells. - * - * @return total number of cells that vector contains. - */ - int getInnerValueCount(); - - - /** - * @param index the index of the value for which we want the size - * @return number of cells that the value at the given index contains. - */ - int getInnerValueCountAt(int index); - - /** - * @param index value index - * @return true if the value at the given index is empty, false otherwise. - */ - boolean isEmpty(int index); - } - - interface RepeatedMutator extends ValueVector.Mutator { - - /** - * Starts a new value that is a container of cells. - * - * @param index index of new value to start - * @return index into the child vector - */ - int startNewValue(int index); - } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/UnionFixedSizeListReader.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/UnionFixedSizeListReader.java index 4ad2f6f5fad..56ae379dca2 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/UnionFixedSizeListReader.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/UnionFixedSizeListReader.java @@ -80,7 +80,7 @@ public void read(int index, UnionHolder holder) { } } holder.reader = data.getReader(); - holder.isSet = vector.getAccessor().isNull(idx()) ? 0 : 1; + holder.isSet = vector.isNull(idx()) ? 0 : 1; } @Override diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReAlloc.java b/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReAlloc.java index c0df4881f38..1b13c2ed919 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReAlloc.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestVectorReAlloc.java @@ -130,7 +130,7 @@ public void testMapType() { assertEquals(512, vector.getValueCapacity()); try { - vector.getAccessor().getObject(513); + vector.getObject(513); Assert.fail("Expected out of bounds exception"); } catch (Exception e) { // ok 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 3514acaa242..f52c697f6bd 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 @@ -34,11 +34,9 @@ import org.apache.arrow.vector.TimeMilliVector; import org.apache.arrow.vector.VarBinaryVector; import org.apache.arrow.vector.VarCharVector; -import org.apache.arrow.vector.ValueVector.Accessor; import org.apache.arrow.vector.VectorSchemaRoot; import org.apache.arrow.vector.complex.ListVector; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.complex.impl.ComplexWriterImpl; import org.apache.arrow.vector.complex.impl.UnionListWriter; import org.apache.arrow.vector.complex.reader.FieldReader;