From b7a70bca2cea08da932965a672b74862d2ff346c Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Tue, 28 Nov 2017 16:26:18 -0800 Subject: [PATCH 1/8] removed NonNullableMapVector --- .../vector/complex/NonNullableMapVector.java | 352 ------------------ 1 file changed, 352 deletions(-) delete mode 100644 java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java 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 deleted file mode 100644 index cc3ac4148a8..00000000000 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/NonNullableMapVector.java +++ /dev/null @@ -1,352 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.arrow.vector.complex; - -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -import javax.annotation.Nullable; - -import com.google.common.base.Preconditions; -import com.google.common.collect.Ordering; -import com.google.common.primitives.Ints; - -import io.netty.buffer.ArrowBuf; - -import org.apache.arrow.memory.BufferAllocator; -import org.apache.arrow.vector.*; -import org.apache.arrow.vector.complex.impl.SingleMapReaderImpl; -import org.apache.arrow.vector.complex.reader.FieldReader; -import org.apache.arrow.vector.holders.ComplexHolder; -import org.apache.arrow.vector.types.Types.MinorType; -import org.apache.arrow.vector.types.pojo.ArrowType; -import org.apache.arrow.vector.types.pojo.Field; -import org.apache.arrow.vector.types.pojo.FieldType; -import org.apache.arrow.vector.util.CallBack; -import org.apache.arrow.vector.util.JsonStringHashMap; -import org.apache.arrow.vector.util.TransferPair; - -public class NonNullableMapVector extends AbstractMapVector { - //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(NonNullableMapVector.class); - - public static NonNullableMapVector empty(String name, BufferAllocator allocator) { - FieldType fieldType = new FieldType(false, ArrowType.Struct.INSTANCE, null, null); - return new NonNullableMapVector(name, allocator, fieldType, null); - } - - private final SingleMapReaderImpl reader = new SingleMapReaderImpl(this); - protected final FieldType fieldType; - public int valueCount; - - // deprecated, use FieldType or static constructor instead - @Deprecated - public NonNullableMapVector(String name, BufferAllocator allocator, CallBack callBack) { - this(name, allocator, new FieldType(false, ArrowType.Struct.INSTANCE, null, null), callBack); - } - - public NonNullableMapVector(String name, BufferAllocator allocator, FieldType fieldType, CallBack callBack) { - super(name, allocator, callBack); - this.fieldType = checkNotNull(fieldType); - this.valueCount = 0; - } - - @Override - public FieldReader getReader() { - return reader; - } - - transient private MapTransferPair ephPair; - - public void copyFromSafe(int fromIndex, int thisIndex, NonNullableMapVector from) { - if (ephPair == null || ephPair.from != from) { - ephPair = (MapTransferPair) from.makeTransferPair(this); - } - ephPair.copyValueSafe(fromIndex, thisIndex); - } - - @Override - protected boolean supportsDirectRead() { - return true; - } - - public Iterator fieldNameIterator() { - return getChildFieldNames().iterator(); - } - - @Override - public void setInitialCapacity(int numRecords) { - for (final ValueVector v : (Iterable) this) { - v.setInitialCapacity(numRecords); - } - } - - @Override - public int getBufferSize() { - if (valueCount == 0 || size() == 0) { - return 0; - } - long buffer = 0; - for (final ValueVector v : (Iterable) this) { - buffer += v.getBufferSize(); - } - - return (int) buffer; - } - - @Override - public int getBufferSizeFor(final int valueCount) { - if (valueCount == 0) { - return 0; - } - - long bufferSize = 0; - for (final ValueVector v : (Iterable) this) { - bufferSize += v.getBufferSizeFor(valueCount); - } - - return (int) bufferSize; - } - - @Override - public ArrowBuf getValidityBuffer() { - throw new UnsupportedOperationException(); - } - - @Override - public ArrowBuf getDataBuffer() { - throw new UnsupportedOperationException(); - } - - @Override - public ArrowBuf getOffsetBuffer() { - throw new UnsupportedOperationException(); - } - - @Override - public TransferPair getTransferPair(BufferAllocator allocator) { - return getTransferPair(name, allocator, null); - } - - @Override - public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) { - return new MapTransferPair(this, new NonNullableMapVector(name, allocator, fieldType, callBack), false); - } - - @Override - public TransferPair makeTransferPair(ValueVector to) { - return new MapTransferPair(this, (NonNullableMapVector) to); - } - - @Override - public TransferPair getTransferPair(String ref, BufferAllocator allocator) { - return new MapTransferPair(this, new NonNullableMapVector(ref, allocator, fieldType, callBack), false); - } - - protected static class MapTransferPair implements TransferPair { - private final TransferPair[] pairs; - private final NonNullableMapVector from; - private final NonNullableMapVector to; - - public MapTransferPair(NonNullableMapVector from, NonNullableMapVector to) { - this(from, to, true); - } - - protected MapTransferPair(NonNullableMapVector from, NonNullableMapVector to, boolean allocate) { - this.from = from; - this.to = to; - this.pairs = new TransferPair[from.size()]; - this.to.ephPair = null; - - int i = 0; - FieldVector vector; - for (String child : from.getChildFieldNames()) { - int preSize = to.size(); - vector = from.getChild(child); - if (vector == null) { - continue; - } - //DRILL-1872: we add the child fields for the vector, looking up the field by name. For a map vector, - // the child fields may be nested fields of the top level child. For example if the structure - // of a child field is oa.oab.oabc then we add oa, then add oab to oa then oabc to oab. - // But the children member of a Materialized field is a HashSet. If the fields are added in the - // children HashSet, and the hashCode of the Materialized field includes the hash code of the - // children, the hashCode value of oa changes *after* the field has been added to the HashSet. - // (This is similar to what happens in ScanBatch where the children cannot be added till they are - // read). To take care of this, we ensure that the hashCode of the MaterializedField does not - // include the hashCode of the children but is based only on MaterializedField$key. - final FieldVector newVector = to.addOrGet(child, vector.getField().getFieldType(), vector.getClass()); - if (allocate && to.size() != preSize) { - newVector.allocateNew(); - } - pairs[i++] = vector.makeTransferPair(newVector); - } - } - - @Override - public void transfer() { - for (final TransferPair p : pairs) { - p.transfer(); - } - to.valueCount = from.valueCount; - from.clear(); - } - - @Override - public ValueVector getTo() { - return to; - } - - @Override - public void copyValueSafe(int from, int to) { - for (TransferPair p : pairs) { - p.copyValueSafe(from, to); - } - } - - @Override - public void splitAndTransfer(int startIndex, int length) { - for (TransferPair p : pairs) { - p.splitAndTransfer(startIndex, length); - } - to.setValueCount(length); - } - } - - @Override - public int getValueCapacity() { - if (size() == 0) { - return 0; - } - - final Ordering natural = new Ordering() { - @Override - public int compare(@Nullable ValueVector left, @Nullable ValueVector right) { - return Ints.compare( - checkNotNull(left).getValueCapacity(), - checkNotNull(right).getValueCapacity() - ); - } - }; - - 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<>(); - for (String child : getChildFieldNames()) { - ValueVector v = getChild(child); - if (v != null && index < v.getValueCount()) { - Object value = v.getObject(index); - if (value != null) { - vv.put(child, value); - } - } - } - return vv; - } - - @Override - public boolean isNull(int index) { return false; } - @Override - public int getNullCount() { return 0; } - - public void get(int index, ComplexHolder holder) { - reader.setPosition(index); - holder.reader = reader; - } - - @Override - public int getValueCount() { - return valueCount; - } - - public ValueVector getVectorById(int id) { - return getChildByOrdinal(id); -} - - @Override - public void setValueCount(int valueCount) { - for (final ValueVector v : getChildren()) { - v.setValueCount(valueCount); - } - NonNullableMapVector.this.valueCount = valueCount; - } - - @Override - public void clear() { - for (final ValueVector v : getChildren()) { - v.clear(); - } - valueCount = 0; - } - - @Override - public Field getField() { - List children = new ArrayList<>(); - for (ValueVector child : getChildren()) { - children.add(child.getField()); - } - return new Field(name, fieldType, children); - } - - @Override - public MinorType getMinorType() { - return MinorType.MAP; - } - - @Override - public void close() { - final Collection vectors = getChildren(); - for (final FieldVector v : vectors) { - v.close(); - } - vectors.clear(); - - valueCount = 0; - - super.close(); - } - - public void initializeChildrenFromFields(List children) { - for (Field field : children) { - FieldVector vector = (FieldVector) this.add(field.getName(), field.getFieldType()); - vector.initializeChildrenFromFields(field.getChildren()); - } - } - - public List getChildrenFromFields() { - return getChildren(); - } -} From ce736bad52a6a769580a186f1453872addf488b3 Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Tue, 28 Nov 2017 16:29:10 -0800 Subject: [PATCH 2/8] Combined MapVector classes, is compiling but tests fail --- .../main/codegen/templates/MapWriters.java | 9 - .../arrow/vector/complex/MapVector.java | 211 +++++++++++++++--- .../complex/impl/SingleMapReaderImpl.java | 7 +- .../complex/writer/TestComplexWriter.java | 3 +- 4 files changed, 184 insertions(+), 46 deletions(-) diff --git a/java/vector/src/main/codegen/templates/MapWriters.java b/java/vector/src/main/codegen/templates/MapWriters.java index a5ac1b71704..aeb24ebe13a 100644 --- a/java/vector/src/main/codegen/templates/MapWriters.java +++ b/java/vector/src/main/codegen/templates/MapWriters.java @@ -20,11 +20,7 @@ <#list ["Nullable", "Single"] as mode> <@pp.changeOutputFile name="/org/apache/arrow/vector/complex/impl/${mode}MapWriter.java" /> <#assign index = "idx()"> -<#if mode == "Single"> -<#assign containerClass = "NonNullableMapVector" /> -<#else> <#assign containerClass = "MapVector" /> - <#include "/@includes/license.ftl" /> @@ -50,11 +46,6 @@ public class ${mode}MapWriter extends AbstractFieldWriter { private int initialCapacity; private final Map fields = Maps.newHashMap(); public ${mode}MapWriter(${containerClass} container) { - <#if mode == "Single"> - if (container instanceof MapVector) { - throw new IllegalArgumentException("Invalid container: " + container); - } - this.container = container; this.initialCapacity = 0; for (Field child : container.getField().getChildren()) { 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..a155212d58e 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 @@ -21,12 +21,15 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; +import java.util.Collection; +import java.util.Iterator; import java.util.List; +import java.util.Map; import com.google.common.collect.ObjectArrays; +import com.google.common.collect.Ordering; +import com.google.common.primitives.Ints; import io.netty.buffer.ArrowBuf; import org.apache.arrow.memory.BaseAllocator; import org.apache.arrow.memory.BufferAllocator; @@ -35,16 +38,20 @@ import org.apache.arrow.vector.complex.impl.NullableMapWriter; import org.apache.arrow.vector.holders.ComplexHolder; import org.apache.arrow.vector.ipc.message.ArrowFieldNode; +import org.apache.arrow.vector.types.Types; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.ArrowType.Struct; import org.apache.arrow.vector.types.pojo.DictionaryEncoding; import org.apache.arrow.vector.types.pojo.FieldType; import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.util.CallBack; +import org.apache.arrow.vector.util.JsonStringHashMap; import org.apache.arrow.vector.util.OversizedAllocationException; import org.apache.arrow.vector.util.TransferPair; -public class MapVector extends NonNullableMapVector implements FieldVector { +import javax.annotation.Nullable; + +public class MapVector extends AbstractMapVector implements FieldVector { public static MapVector empty(String name, BufferAllocator allocator) { FieldType fieldType = FieldType.nullable(Struct.INSTANCE); @@ -56,6 +63,8 @@ public static MapVector empty(String name, BufferAllocator allocator) { protected ArrowBuf validityBuffer; private int validityAllocationSizeInBytes; + protected final FieldType fieldType; + public int valueCount; // deprecated, use FieldType or static constructor instead @Deprecated @@ -70,14 +79,25 @@ public MapVector(String name, BufferAllocator allocator, DictionaryEncoding dict } public MapVector(String name, BufferAllocator allocator, FieldType fieldType, CallBack callBack) { - super(name, checkNotNull(allocator), fieldType, callBack); + super(name, checkNotNull(allocator), callBack); + this.fieldType = checkNotNull(fieldType); + this.valueCount = 0; this.validityBuffer = allocator.getEmpty(); this.validityAllocationSizeInBytes = BitVectorHelper.getValidityBufferSize(BaseValueVector.INITIAL_VALUE_ALLOCATION); } + @Override + public Types.MinorType getMinorType() { + return Types.MinorType.MAP; + } + @Override public Field getField() { - Field f = super.getField(); + List children = new ArrayList<>(); + for (ValueVector child : getChildren()) { + children.add(child.getField()); + } + Field f = new Field(name, fieldType, children); FieldType type = new FieldType(true, f.getType(), f.getFieldType().getDictionary(), f.getFieldType().getMetadata()); return new Field(f.getName(), type, f.getChildren()); } @@ -116,6 +136,19 @@ public List getFieldInnerVectors() { throw new UnsupportedOperationException("There are no inner vectors. Use getFieldBuffers"); } + @Override + protected boolean supportsDirectRead() { + return true; + } + + public Iterator fieldNameIterator() { + return getChildFieldNames().iterator(); + } + + public ValueVector getVectorById(int id) { + return getChildByOrdinal(id); + } + @Override public NullableMapReaderImpl getReader() { return reader; @@ -125,6 +158,15 @@ public NullableMapWriter getWriter() { return writer; } + transient private NullableMapTransferPair ephPair; + + public void copyFromSafe(int fromIndex, int thisIndex, MapVector from) { + if (ephPair == null || ephPair.from != from) { + ephPair = (NullableMapTransferPair) from.makeTransferPair(this); + } + ephPair.copyValueSafe(fromIndex, thisIndex); + } + @Override public TransferPair getTransferPair(BufferAllocator allocator) { return new NullableMapTransferPair(this, new MapVector(name, allocator, fieldType, null), false); @@ -145,37 +187,80 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallB return new NullableMapTransferPair(this, new MapVector(ref, allocator, fieldType, callBack), false); } - protected class NullableMapTransferPair extends MapTransferPair { + protected class NullableMapTransferPair implements TransferPair { - private MapVector target; + private final TransferPair[] pairs; + private final MapVector from; + private MapVector to; protected NullableMapTransferPair(MapVector from, MapVector to, boolean allocate) { - super(from, to, allocate); - this.target = to; + this.from = from; + this.to = to; + this.pairs = new TransferPair[from.size()]; + // TODO: ?? this.to.ephPair = null; + + int i = 0; + FieldVector vector; + for (String child : from.getChildFieldNames()) { + int preSize = to.size(); + vector = from.getChild(child); + if (vector == null) { + continue; + } + //DRILL-1872: we add the child fields for the vector, looking up the field by name. For a map vector, + // the child fields may be nested fields of the top level child. For example if the structure + // of a child field is oa.oab.oabc then we add oa, then add oab to oa then oabc to oab. + // But the children member of a Materialized field is a HashSet. If the fields are added in the + // children HashSet, and the hashCode of the Materialized field includes the hash code of the + // children, the hashCode value of oa changes *after* the field has been added to the HashSet. + // (This is similar to what happens in ScanBatch where the children cannot be added till they are + // read). To take care of this, we ensure that the hashCode of the MaterializedField does not + // include the hashCode of the children but is based only on MaterializedField$key. + final FieldVector newVector = to.addOrGet(child, vector.getField().getFieldType(), vector.getClass()); + if (allocate && to.size() != preSize) { + newVector.allocateNew(); + } + pairs[i++] = vector.makeTransferPair(newVector); + } + this.to = to; } @Override public void transfer() { - target.clear(); - target.validityBuffer = validityBuffer.transferOwnership(target.allocator).buffer; - super.transfer(); + to.clear(); + to.validityBuffer = validityBuffer.transferOwnership(to.allocator).buffer; + for (final TransferPair p : pairs) { + p.transfer(); + } + to.valueCount = from.valueCount; + from.clear(); clear(); } + @Override + public ValueVector getTo() { + return to; + } + @Override public void copyValueSafe(int fromIndex, int toIndex) { - while (toIndex >= target.getValidityBufferValueCapacity()) { - target.reallocValidityBuffer(); + while (toIndex >= to.getValidityBufferValueCapacity()) { + to.reallocValidityBuffer(); + } + BitVectorHelper.setValidityBit(to.validityBuffer, toIndex, isSet(fromIndex)); + for (TransferPair p : pairs) { + p.copyValueSafe(fromIndex, toIndex); } - BitVectorHelper.setValidityBit(target.validityBuffer, toIndex, isSet(fromIndex)); - super.copyValueSafe(fromIndex, toIndex); } @Override public void splitAndTransfer(int startIndex, int length) { - target.clear(); - splitAndTransferValidityBuffer(startIndex, length, target); - super.splitAndTransfer(startIndex, length); + to.clear(); + splitAndTransferValidityBuffer(startIndex, length, to); + for (TransferPair p : pairs) { + p.splitAndTransfer(startIndex, length); + } + to.setValueCount(length); } } @@ -252,8 +337,22 @@ private int getValidityBufferValueCapacity() { */ @Override public int getValueCapacity() { + if (size() == 0) { + return 0; + } + + final Ordering natural = new Ordering() { + @Override + public int compare(@Nullable ValueVector left, @Nullable ValueVector right) { + return Ints.compare( + checkNotNull(left).getValueCapacity(), + checkNotNull(right).getValueCapacity() + ); + } + }; + return Math.min(getValidityBufferValueCapacity(), - super.getValueCapacity()); + natural.min(getChildren()).getValueCapacity()); } /** @@ -293,6 +392,12 @@ public ArrowBuf[] getBuffers(boolean clear) { @Override public void close() { clearValidityBuffer(); + final Collection vectors = getChildren(); + for (final FieldVector v : vectors) { + v.close(); + } + vectors.clear(); + valueCount = 0; super.close(); } @@ -302,7 +407,10 @@ public void close() { @Override public void clear() { clearValidityBuffer(); - super.clear(); + for (final ValueVector v : getChildren()) { + v.clear(); + } + valueCount = 0; } /** @@ -320,11 +428,15 @@ private void clearValidityBuffer() { */ @Override public int getBufferSize() { - if (valueCount == 0) { + if (valueCount == 0 || size() == 0) { return 0; } - return super.getBufferSize() + - BitVectorHelper.getValidityBufferSize(valueCount); + long bufferSize = 0; + for (final ValueVector v : (Iterable) this) { + bufferSize += v.getBufferSize(); + } + + return ((int) bufferSize) + BitVectorHelper.getValidityBufferSize(valueCount); } /** @@ -338,14 +450,21 @@ public int getBufferSizeFor(final int valueCount) { if (valueCount == 0) { return 0; } - return super.getBufferSizeFor(valueCount) - + BitVectorHelper.getValidityBufferSize(valueCount); + + long bufferSize = 0; + for (final ValueVector v : (Iterable) this) { + bufferSize += v.getBufferSizeFor(valueCount); + } + + return ((int) bufferSize) + BitVectorHelper.getValidityBufferSize(valueCount); } @Override public void setInitialCapacity(int numRecords) { validityAllocationSizeInBytes = BitVectorHelper.getValidityBufferSize(numRecords); - super.setInitialCapacity(numRecords); + for (final ValueVector v : (Iterable) this) { + v.setInitialCapacity(numRecords); + } } @Override @@ -437,19 +556,34 @@ public ArrowBuf getOffsetBuffer() { throw new UnsupportedOperationException(); } + @Override + public int getValueCount() { + return valueCount; + } + @Override public Object getObject(int index) { if (isSet(index) == 0) { return null; } else { - return super.getObject(index); + Map vv = new JsonStringHashMap<>(); + for (String child : getChildFieldNames()) { + ValueVector v = getChild(child); + if (v != null && index < v.getValueCount()) { + Object value = v.getObject(index); + if (value != null) { + vv.put(child, value); + } + } + } + return vv; } } - @Override public void get(int index, ComplexHolder holder) { holder.isSet = isSet(index); - super.get(index, holder); + reader.setPosition(index); + holder.reader = reader; } public int getNullCount() { @@ -490,7 +624,9 @@ public void setValueCount(int valueCount) { /* realloc the inner buffers if needed */ reallocValidityBuffer(); } - super.setValueCount(valueCount); + for (final ValueVector v : getChildren()) { + v.setValueCount(valueCount); + } this.valueCount = valueCount; } @@ -498,6 +634,19 @@ public void reset() { valueCount = 0; } + @Override + public void initializeChildrenFromFields(List children) { + for (Field field : children) { + FieldVector vector = (FieldVector) this.add(field.getName(), field.getFieldType()); + vector.initializeChildrenFromFields(field.getChildren()); + } + } + + @Override + public List getChildrenFromFields() { + return getChildren(); + } + @Override @Deprecated public Accessor getAccessor() { diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/SingleMapReaderImpl.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/SingleMapReaderImpl.java index 0341b622e0d..5cb4e919678 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/SingleMapReaderImpl.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/SingleMapReaderImpl.java @@ -24,7 +24,6 @@ import org.apache.arrow.vector.ValueVector; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.NonNullableMapVector; import org.apache.arrow.vector.complex.reader.FieldReader; import org.apache.arrow.vector.complex.writer.BaseWriter.MapWriter; import org.apache.arrow.vector.types.Types.MinorType; @@ -35,10 +34,10 @@ @SuppressWarnings("unused") public class SingleMapReaderImpl extends AbstractFieldReader { - private final NonNullableMapVector vector; + private final MapVector vector; private final Map fields = Maps.newHashMap(); - public SingleMapReaderImpl(NonNullableMapVector vector) { + public SingleMapReaderImpl(MapVector vector) { this.vector = vector; } @@ -89,7 +88,7 @@ public MinorType getMinorType() { @Override public boolean isSet() { - return true; + return !vector.isNull(idx()); } @Override diff --git a/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java b/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java index bd8489eb20f..178b464af17 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java @@ -34,7 +34,6 @@ import org.apache.arrow.vector.IntVector; import org.apache.arrow.vector.complex.ListVector; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.NonNullableMapVector; import org.apache.arrow.vector.complex.UnionVector; import org.apache.arrow.vector.complex.impl.ComplexWriterImpl; import org.apache.arrow.vector.complex.impl.SingleMapReaderImpl; @@ -849,7 +848,7 @@ public void testSingleMapWriter1() { /* initialize a SingleMapWriter with empty MapVector and then lazily * create all vectors with expected initialCapacity. */ - NonNullableMapVector parent = NonNullableMapVector.empty("parent", allocator); + MapVector parent = MapVector.empty("parent", allocator); SingleMapWriter singleMapWriter = new SingleMapWriter(parent); int initialCapacity = 1024; From a3934df92cf8c929e057ab425934b5fd4d3e9950 Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Tue, 28 Nov 2017 18:32:33 -0800 Subject: [PATCH 3/8] fixed bug when MapWriter was created before fieldType assigned --- .../main/java/org/apache/arrow/vector/complex/MapVector.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a155212d58e..a0358e649f4 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 @@ -59,7 +59,7 @@ public static MapVector empty(String name, BufferAllocator allocator) { } private final NullableMapReaderImpl reader = new NullableMapReaderImpl(this); - private final NullableMapWriter writer = new NullableMapWriter(this); + private NullableMapWriter writer; protected ArrowBuf validityBuffer; private int validityAllocationSizeInBytes; @@ -84,6 +84,7 @@ public MapVector(String name, BufferAllocator allocator, FieldType fieldType, Ca this.valueCount = 0; this.validityBuffer = allocator.getEmpty(); this.validityAllocationSizeInBytes = BitVectorHelper.getValidityBufferSize(BaseValueVector.INITIAL_VALUE_ALLOCATION); + this.writer = new NullableMapWriter(this); // NOTE: fieldType must be set before this } @Override From dfc2fe9cc86f2230da8c6e6c6b01923f52d9a91f Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Tue, 28 Nov 2017 18:49:33 -0800 Subject: [PATCH 4/8] fixed ComplexWriterTest that needed validity vector to be allocated --- .../apache/arrow/vector/complex/writer/TestComplexWriter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java b/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java index 178b464af17..5671f3c6d2f 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java @@ -853,6 +853,7 @@ public void testSingleMapWriter1() { int initialCapacity = 1024; singleMapWriter.setInitialCapacity(initialCapacity); + singleMapWriter.allocate(); IntWriter intWriter = singleMapWriter.integer("intField"); BigIntWriter bigIntWriter = singleMapWriter.bigInt("bigIntField"); From 12084ed8725b092b32aaee1aab0459e5b1098dd6 Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 29 Nov 2017 11:01:23 -0800 Subject: [PATCH 5/8] also need to properly set index when starting SingleMapWriter now --- java/vector/src/main/codegen/templates/MapWriters.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java/vector/src/main/codegen/templates/MapWriters.java b/java/vector/src/main/codegen/templates/MapWriters.java index aeb24ebe13a..5caffdd2d7e 100644 --- a/java/vector/src/main/codegen/templates/MapWriters.java +++ b/java/vector/src/main/codegen/templates/MapWriters.java @@ -188,10 +188,7 @@ public void setPosition(int index) { @Override public void start() { - <#if mode == "Single"> - <#else> container.setIndexDefined(idx()); - } @Override From db31b5eb15c940f519a87ae678a937f6275cdb10 Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 29 Nov 2017 11:26:22 -0800 Subject: [PATCH 6/8] some cleanup in MapVector --- .../org/apache/arrow/vector/complex/MapVector.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 a0358e649f4..11720c378fb 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 @@ -58,8 +58,8 @@ public static MapVector empty(String name, BufferAllocator allocator) { return new MapVector(name, allocator, fieldType, null); } - private final NullableMapReaderImpl reader = new NullableMapReaderImpl(this); - private NullableMapWriter writer; + private final NullableMapReaderImpl reader; + private final NullableMapWriter writer; protected ArrowBuf validityBuffer; private int validityAllocationSizeInBytes; @@ -84,6 +84,7 @@ public MapVector(String name, BufferAllocator allocator, FieldType fieldType, Ca this.valueCount = 0; this.validityBuffer = allocator.getEmpty(); this.validityAllocationSizeInBytes = BitVectorHelper.getValidityBufferSize(BaseValueVector.INITIAL_VALUE_ALLOCATION); + this.reader = new NullableMapReaderImpl(this); this.writer = new NullableMapWriter(this); // NOTE: fieldType must be set before this } @@ -192,13 +193,13 @@ protected class NullableMapTransferPair implements TransferPair { private final TransferPair[] pairs; private final MapVector from; - private MapVector to; + private final MapVector to; protected NullableMapTransferPair(MapVector from, MapVector to, boolean allocate) { this.from = from; this.to = to; this.pairs = new TransferPair[from.size()]; - // TODO: ?? this.to.ephPair = null; + this.to.ephPair = null; int i = 0; FieldVector vector; @@ -223,7 +224,6 @@ protected NullableMapTransferPair(MapVector from, MapVector to, boolean allocate } pairs[i++] = vector.makeTransferPair(newVector); } - this.to = to; } @Override From 396de5e1a35d1082e59689748777c2a9d11dea5b Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 29 Nov 2017 11:51:59 -0800 Subject: [PATCH 7/8] made valueCount private --- .../java/org/apache/arrow/vector/complex/MapVector.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 11720c378fb..00bdc3d3d7d 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 @@ -58,13 +58,12 @@ public static MapVector empty(String name, BufferAllocator allocator) { return new MapVector(name, allocator, fieldType, null); } - private final NullableMapReaderImpl reader; - private final NullableMapWriter writer; - protected ArrowBuf validityBuffer; private int validityAllocationSizeInBytes; - protected final FieldType fieldType; - public int valueCount; + private final FieldType fieldType; + private int valueCount; + private final NullableMapReaderImpl reader; + private final NullableMapWriter writer; // deprecated, use FieldType or static constructor instead @Deprecated From 59647f9229e62dab52ea9d459825d95a26f2600d Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 29 Nov 2017 16:06:04 -0800 Subject: [PATCH 8/8] removed duplicate imports and unnecessary casting --- .../apache/arrow/vector/complex/impl/ComplexWriterImpl.java | 3 +-- .../arrow/vector/complex/impl/NullableMapReaderImpl.java | 5 ++--- .../arrow/vector/complex/impl/TestPromotableWriter.java | 1 - .../test/java/org/apache/arrow/vector/ipc/BaseFileTest.java | 1 - .../test/java/org/apache/arrow/vector/ipc/TestArrowFile.java | 1 - .../test/java/org/apache/arrow/vector/ipc/TestJSONFile.java | 1 - 6 files changed, 3 insertions(+), 9 deletions(-) diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java index 970b90ef510..71bd9ae1c1a 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java @@ -20,7 +20,6 @@ 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.StateTool; import org.apache.arrow.vector.complex.writer.BaseWriter.ComplexWriter; import org.apache.arrow.vector.types.pojo.Field; @@ -131,7 +130,7 @@ public MapWriter directMap() { switch (mode) { case INIT: - mapRoot = nullableMapWriterFactory.build((MapVector) container); + mapRoot = nullableMapWriterFactory.build(container); mapRoot.setPosition(idx()); mode = Mode.MAP; break; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableMapReaderImpl.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableMapReaderImpl.java index acf155af71d..d3e0eee971d 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableMapReaderImpl.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableMapReaderImpl.java @@ -19,7 +19,6 @@ package org.apache.arrow.vector.complex.impl; -import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.complex.writer.BaseWriter.MapWriter; import org.apache.arrow.vector.types.pojo.Field; @@ -29,8 +28,8 @@ public class NullableMapReaderImpl extends SingleMapReaderImpl { private MapVector nullableMapVector; public NullableMapReaderImpl(MapVector vector) { - super((MapVector) vector); - this.nullableMapVector = (MapVector) vector; + super(vector); + this.nullableMapVector = vector; } @Override diff --git a/java/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestPromotableWriter.java b/java/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestPromotableWriter.java index 38b78424dcc..464dbed4b59 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestPromotableWriter.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestPromotableWriter.java @@ -25,7 +25,6 @@ import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.vector.DirtyRootAllocator; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.complex.UnionVector; import org.apache.arrow.vector.complex.writer.BaseWriter.MapWriter; import org.apache.arrow.vector.types.pojo.ArrowType; 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..79b388482f7 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 @@ -38,7 +38,6 @@ 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; diff --git a/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowFile.java b/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowFile.java index 0cfc9ba919d..d2dea052362 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowFile.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowFile.java @@ -43,7 +43,6 @@ import org.apache.arrow.vector.VectorUnloader; import org.apache.arrow.vector.complex.FixedSizeListVector; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.dictionary.DictionaryProvider.MapDictionaryProvider; import org.apache.arrow.vector.ipc.message.ArrowBlock; import org.apache.arrow.vector.ipc.message.ArrowBuffer; diff --git a/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestJSONFile.java b/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestJSONFile.java index 625717048bf..27b377bf43b 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestJSONFile.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/ipc/TestJSONFile.java @@ -25,7 +25,6 @@ import org.apache.arrow.vector.FieldVector; import org.apache.arrow.vector.VectorSchemaRoot; import org.apache.arrow.vector.complex.MapVector; -import org.apache.arrow.vector.complex.MapVector; import org.apache.arrow.vector.dictionary.DictionaryProvider; import org.apache.arrow.vector.dictionary.DictionaryProvider.MapDictionaryProvider; import org.apache.arrow.vector.types.pojo.Schema;