You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/07/10 19:43:27 UTC

[arrow] branch master updated: ARROW-5897: [Java] Remove duplicated logic in MapVector

This is an automated email from the ASF dual-hosted git repository.

wesm 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 8b6cb3e  ARROW-5897: [Java] Remove duplicated logic in MapVector
8b6cb3e is described below

commit 8b6cb3e9fe305a8cbab2180639343c672f11353d
Author: liyafan82 <fa...@foxmail.com>
AuthorDate: Wed Jul 10 14:43:19 2019 -0500

    ARROW-5897: [Java] Remove duplicated logic in MapVector
    
    Current implementation of MapVector contains much logic duplicated from super classes. We remove it by:
    
    1. Making the default data vector name configurable
    2. Extract a method for creating the reader
    
    Author: liyafan82 <fa...@foxmail.com>
    
    Closes #4842 from liyafan82/fly_0710_map and squashes the following commits:
    
    2e1614776 <liyafan82>  Remove duplicated logic in MapVector
---
 .../vector/complex/BaseRepeatedValueVector.java    |  4 +-
 .../apache/arrow/vector/complex/ListVector.java    | 10 ++--
 .../org/apache/arrow/vector/complex/MapVector.java | 54 ++--------------------
 3 files changed, 13 insertions(+), 55 deletions(-)

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 3ea418e..dbc7236 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
@@ -57,6 +57,8 @@ public abstract class BaseRepeatedValueVector extends BaseValueVector implements
   protected int offsetAllocationSizeInBytes = INITIAL_VALUE_ALLOCATION * OFFSET_WIDTH;
   private final String name;
 
+  protected String defaultDataVectorName = DATA_VECTOR_NAME;
+
   protected BaseRepeatedValueVector(String name, BufferAllocator allocator, CallBack callBack) {
     this(name, allocator, DEFAULT_DATA_VECTOR, callBack);
   }
@@ -277,7 +279,7 @@ public abstract class BaseRepeatedValueVector extends BaseValueVector implements
   public <T extends ValueVector> AddOrGetResult<T> addOrGetVector(FieldType fieldType) {
     boolean created = false;
     if (vector instanceof ZeroVector) {
-      vector = fieldType.createNewSingleVector(DATA_VECTOR_NAME, allocator, callBack);
+      vector = fieldType.createNewSingleVector(defaultDataVectorName, allocator, callBack);
       // returned vector must have the same field
       created = true;
       if (callBack != null &&
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 e8b402c..abbb2f6 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
@@ -101,7 +101,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
   public ListVector(String name, BufferAllocator allocator, FieldType fieldType, CallBack callBack) {
     super(name, allocator, callBack);
     this.validityBuffer = allocator.getEmpty();
-    this.reader = new UnionListReader(this);
+    createReader();
     this.fieldType = checkNotNull(fieldType);
     this.callBack = callBack;
     this.validityAllocationSizeInBytes = getValidityBufferSizeFromCount(INITIAL_VALUE_ALLOCATION);
@@ -547,7 +547,7 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
   /** Initialize the child data vector to field type.  */
   public <T extends ValueVector> AddOrGetResult<T> addOrGetVector(FieldType fieldType) {
     AddOrGetResult<T> result = super.addOrGetVector(fieldType);
-    reader = new UnionListReader(this);
+    createReader();
     return result;
   }
 
@@ -627,13 +627,17 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
   public UnionVector promoteToUnion() {
     UnionVector vector = new UnionVector("$data$", allocator, callBack);
     replaceDataVector(vector);
-    reader = new UnionListReader(this);
+    createReader();
     if (callBack != null) {
       callBack.doWork();
     }
     return vector;
   }
 
+  protected void createReader() {
+    reader = new UnionListReader(this);
+  }
+
   /**
    * Get the element in the list vector at a particular index.
    * @param index position of the element
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 a367150..0fcb19c 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
@@ -24,18 +24,14 @@ import java.util.List;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.AddOrGetResult;
 import org.apache.arrow.vector.FieldVector;
-import org.apache.arrow.vector.ValueVector;
-import org.apache.arrow.vector.ZeroVector;
 import org.apache.arrow.vector.complex.impl.UnionMapReader;
 import org.apache.arrow.vector.complex.impl.UnionMapWriter;
 import org.apache.arrow.vector.types.Types;
 import org.apache.arrow.vector.types.Types.MinorType;
-import org.apache.arrow.vector.types.pojo.ArrowType.ArrowTypeID;
 import org.apache.arrow.vector.types.pojo.ArrowType.Map;
 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.SchemaChangeRuntimeException;
 
 /**
  * A MapVector is used to store entries of key/value pairs. It is a container vector that is
@@ -48,9 +44,7 @@ public class MapVector extends ListVector {
 
   public static final String KEY_NAME = "key";
   public static final String VALUE_NAME = "value";
-
-  // TODO: this is only used for addOrGetVector because ListVector declares it private
-  protected CallBack callBack;
+  public static final String DATA_VECTOR_NAME = "entries";
 
   /**
    * Construct an empty MapVector with no data. Child vectors must be added subsequently.
@@ -74,8 +68,7 @@ public class MapVector extends ListVector {
    */
   public MapVector(String name, BufferAllocator allocator, FieldType fieldType, CallBack callBack) {
     super(name, allocator, fieldType, callBack);
-    this.callBack = callBack;
-    reader = new UnionMapReader(this);
+    defaultDataVectorName = DATA_VECTOR_NAME;
   }
 
   /**
@@ -119,49 +112,8 @@ public class MapVector extends ListVector {
     return (UnionMapReader)reader;
   }
 
-  /**
-   * Add a child vector that will be the list vector, or get the vector if already added.
-   *
-   * @param fieldType The field type of the child vector.
-   * @param <T> Type of the resulting vector.
-   * @return return an AddOrGetResult instance that contains the vector and created flag.
-   */
-  @Override
-  public <T extends ValueVector> AddOrGetResult<T> addOrGetVector(FieldType fieldType) {
-
-    // TODO: can call super method once DATA_VECTOR_NAME is configurable
-    boolean created = false;
-    if (vector instanceof ZeroVector) {
-      vector = fieldType.createNewSingleVector("entries", allocator, callBack);
-      // returned vector must have the same field
-      created = true;
-      if (callBack != null &&
-              // not a schema change if changing from ZeroVector to ZeroVector
-              (fieldType.getType().getTypeID() != ArrowTypeID.Null)) {
-        callBack.doWork();
-      }
-    }
-
-    if (vector.getField().getType().getTypeID() != fieldType.getType().getTypeID()) {
-      final String msg = String.format("Inner vector type mismatch. Requested type: [%s], actual type: [%s]",
-              fieldType.getType().getTypeID(), vector.getField().getType().getTypeID());
-      throw new SchemaChangeRuntimeException(msg);
-    }
-
-    reader = new UnionMapReader(this);
-
-    return new AddOrGetResult<>((T) vector, created);
-  }
-
-  /**
-   * Promote this MapVector to a UnionVector.
-   *
-   * @return the new UnionVector.
-   */
   @Override
-  public UnionVector promoteToUnion() {
-    UnionVector result = super.promoteToUnion();
+  protected void createReader() {
     reader = new UnionMapReader(this);
-    return result;
   }
 }