arrow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From w...@apache.org
Subject [arrow] branch master updated: ARROW-1663: [Java] use consistent name for null and not-null in FixedSizeLis…
Date Sun, 05 Nov 2017 23:57:16 GMT
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 b25b243  ARROW-1663: [Java] use consistent name for null and not-null in FixedSizeLis…
b25b243 is described below

commit b25b2433d67d797e9cd461377eb3798e60de2727
Author: Yuliya Feldman <yuliya@dremio.com>
AuthorDate: Sun Nov 5 18:57:05 2017 -0500

    ARROW-1663: [Java] use consistent name for null and not-null in FixedSizeLis…
    
    …t, add backward compatibility while deserializing schema that was generated before
this JIRA checkin
    
    Author: Yuliya Feldman <yuliya@dremio.com>
    
    Closes #1193 from yufeldman/ARROW-1663 and squashes the following commits:
    
    7f9bd34f [Yuliya Feldman] ARROW-1663: Addressing code review comments
    600d379b [Yuliya Feldman] ARROW-1663: Addressing code review comments
    93f527b0 [Yuliya Feldman] ARROW-1663: Addressing code review comments
    7cfa22b0 [Yuliya Feldman] ARROW-1663: use consistent name for null and not-null in FixedSizeListVector
and ListVector, add backward compatibility while deserializing schema that was generated before
this JIRA checkin
    58d6e9c9 [Yuliya Feldman] ARROW-1663: use consistent name for null and not-null in FixedSizeList,
add backward compatibility while deserializing schema that was generated before this JIRA
checkin
---
 .../java/org/apache/arrow/vector/ZeroVector.java   |  6 ++--
 .../apache/arrow/vector/complex/ListVector.java    |  5 ---
 .../org/apache/arrow/vector/types/pojo/Field.java  | 27 ++++++++++++++-
 .../arrow/vector/TestFixedSizeListVector.java      | 14 ++++++++
 .../org/apache/arrow/vector/TestListVector.java    |  7 ++--
 .../org/apache/arrow/vector/pojo/TestConvert.java  | 40 ++++++++++++++++++++++
 6 files changed, 85 insertions(+), 14 deletions(-)

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 b267b20..5ac0037 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
@@ -18,6 +18,8 @@
 
 package org.apache.arrow.vector;
 
+import static org.apache.arrow.vector.complex.BaseRepeatedValueVector.DATA_VECTOR_NAME;
+
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -39,8 +41,6 @@ import io.netty.buffer.ArrowBuf;
 public class ZeroVector implements FieldVector {
   public final static ZeroVector INSTANCE = new ZeroVector();
 
-  private final String name = "[DEFAULT]";
-
   private final TransferPair defaultPair = new TransferPair() {
     @Override
     public void transfer() {
@@ -109,7 +109,7 @@ public class ZeroVector implements FieldVector {
 
   @Override
   public Field getField() {
-    return new Field(name, FieldType.nullable(new Null()), null);
+    return new Field(DATA_VECTOR_NAME, FieldType.nullable(new Null()), null);
   }
 
   @Override
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 6511efc..ea28a60 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
@@ -39,7 +39,6 @@ import org.apache.arrow.vector.BufferBacked;
 import org.apache.arrow.vector.FieldVector;
 import org.apache.arrow.vector.UInt4Vector;
 import org.apache.arrow.vector.ValueVector;
-import org.apache.arrow.vector.VarCharVector;
 import org.apache.arrow.vector.ZeroVector;
 import org.apache.arrow.vector.complex.impl.ComplexCopier;
 import org.apache.arrow.vector.complex.impl.UnionListReader;
@@ -49,7 +48,6 @@ import org.apache.arrow.vector.complex.writer.FieldWriter;
 import org.apache.arrow.vector.schema.ArrowFieldNode;
 import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.pojo.ArrowType;
-import org.apache.arrow.vector.types.pojo.ArrowType.Null;
 import org.apache.arrow.vector.types.pojo.DictionaryEncoding;
 import org.apache.arrow.vector.types.pojo.Field;
 import org.apache.arrow.vector.types.pojo.FieldType;
@@ -324,9 +322,6 @@ public class ListVector extends BaseRepeatedValueVector implements FieldVector,
 
   @Override
   public Field getField() {
-    if (getDataVector() instanceof ZeroVector) {
-      return new Field(name, fieldType, ImmutableList.of(new Field(DATA_VECTOR_NAME, FieldType.nullable(Null.INSTANCE),
null)));
-    }
     return new Field(name, fieldType, ImmutableList.of(getDataVector().getField()));
   }
 
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Field.java b/java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Field.java
index 48e71a9..eba149b 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Field.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Field.java
@@ -20,6 +20,7 @@ package org.apache.arrow.vector.types.pojo;
 
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.arrow.vector.complex.BaseRepeatedValueVector.DATA_VECTOR_NAME;
 import static org.apache.arrow.vector.types.pojo.ArrowType.getTypeForField;
 
 import java.util.Iterator;
@@ -39,6 +40,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.flatbuffers.FlatBufferBuilder;
 
 import org.apache.arrow.flatbuf.KeyValue;
+import org.apache.arrow.flatbuf.Type;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.FieldVector;
 import org.apache.arrow.vector.schema.TypeLayout;
@@ -121,7 +123,9 @@ public class Field {
     }
     ImmutableList.Builder<Field> childrenBuilder = ImmutableList.builder();
     for (int i = 0; i < field.childrenLength(); i++) {
-      childrenBuilder.add(convertField(field.children(i)));
+      Field childField = convertField(field.children(i));
+      childField = mutateOriginalNameIfNeeded(field, childField);
+      childrenBuilder.add(childField);
     }
     List<Field> children = childrenBuilder.build();
     ImmutableMap.Builder<String, String> metadataBuilder = ImmutableMap.builder();
@@ -134,6 +138,27 @@ public class Field {
     return new Field(name, nullable, type, dictionary, children, new TypeLayout(layout.build()),
metadata);
   }
 
+  /**
+   * Helper method to ensure backward compatibility with schemas generated prior to ARROW-1347,
ARROW-1663
+   * @param field
+   * @param originalChildField original field which name might be mutated
+   * @return original or mutated field
+   */
+  private static Field mutateOriginalNameIfNeeded(org.apache.arrow.flatbuf.Field field, Field
originalChildField) {
+    if ((field.typeType() == Type.List || field.typeType() == Type.FixedSizeList)
+        && originalChildField.getName().equals("[DEFAULT]")) {
+      return
+        new Field(DATA_VECTOR_NAME,
+          originalChildField.isNullable(),
+          originalChildField.getType(),
+          originalChildField.getDictionary(),
+          originalChildField.getChildren(),
+          originalChildField.getTypeLayout(),
+          originalChildField.getMetadata());
+    }
+    return originalChildField;
+  }
+
   public void validate() {
     TypeLayout expectedLayout = TypeLayout.getTypeLayout(getType());
     if (!expectedLayout.equals(typeLayout)) {
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java
b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java
index 5677f25..43d9387 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestFixedSizeListVector.java
@@ -26,6 +26,7 @@ import org.apache.arrow.vector.complex.ListVector;
 import org.apache.arrow.vector.complex.impl.UnionFixedSizeListReader;
 import org.apache.arrow.vector.complex.impl.UnionListReader;
 import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.types.Types;
 import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.pojo.ArrowType;
 import org.apache.arrow.vector.types.pojo.FieldType;
@@ -220,4 +221,17 @@ public class TestFixedSizeListVector {
       }
     }
   }
+
+  @Test
+  public void testConsistentChildName() throws Exception {
+    try (FixedSizeListVector listVector = FixedSizeListVector.empty("sourceVector", 2, allocator))
{
+      String emptyListStr = listVector.getField().toString();
+      Assert.assertTrue(emptyListStr.contains(ListVector.DATA_VECTOR_NAME));
+
+      listVector.addOrGetVector(FieldType.nullable(Types.MinorType.INT.getType()));
+      String emptyVectorStr = listVector.getField().toString();
+      Assert.assertTrue(emptyVectorStr.contains(ListVector.DATA_VECTOR_NAME));
+    }
+  }
+
 }
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 1c9b574..59e1646 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
@@ -26,12 +26,9 @@ import static org.junit.Assert.assertTrue;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.complex.ListVector;
 import org.apache.arrow.vector.complex.impl.UnionListWriter;
-import org.apache.arrow.vector.complex.impl.UnionListReader;
 import org.apache.arrow.vector.complex.reader.FieldReader;
-import org.apache.arrow.vector.complex.writer.FieldWriter;
-import org.apache.arrow.vector.holders.NullableBigIntHolder;
 import org.apache.arrow.vector.types.Types;
-import org.apache.arrow.vector.types.Types.*;
+import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.pojo.FieldType;
 import org.apache.arrow.vector.util.TransferPair;
 import org.junit.After;
@@ -635,7 +632,7 @@ public class TestListVector {
       String emptyListStr = listVector.getField().toString();
       assertTrue(emptyListStr.contains(ListVector.DATA_VECTOR_NAME));
 
-      listVector.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
+      listVector.addOrGetVector(FieldType.nullable(Types.MinorType.INT.getType()));
       String emptyVectorStr = listVector.getField().toString();
       assertTrue(emptyVectorStr.contains(ListVector.DATA_VECTOR_NAME));
     }
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/pojo/TestConvert.java b/java/vector/src/test/java/org/apache/arrow/vector/pojo/TestConvert.java
index f98aeac..f6f1ad2 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/pojo/TestConvert.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/pojo/TestConvert.java
@@ -21,13 +21,19 @@ package org.apache.arrow.vector.pojo;
 import static org.apache.arrow.vector.types.FloatingPointPrecision.DOUBLE;
 import static org.apache.arrow.vector.types.FloatingPointPrecision.SINGLE;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
+import java.nio.ByteBuffer;
 import java.util.HashMap;
 import java.util.Map;
 
 import com.google.common.collect.ImmutableList;
 import com.google.flatbuffers.FlatBufferBuilder;
 
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.complex.FixedSizeListVector;
+import org.apache.arrow.vector.complex.ListVector;
 import org.apache.arrow.vector.types.TimeUnit;
 import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.UnionMode;
@@ -65,6 +71,40 @@ public class TestConvert {
   }
 
   @Test
+  public void list() throws Exception {
+    ImmutableList.Builder<Field> childrenBuilder = ImmutableList.builder();
+    try (BufferAllocator allocator = new RootAllocator(Long.MAX_VALUE);
+         ListVector writeVector = ListVector.empty("list", allocator);
+         FixedSizeListVector writeFixedVector = FixedSizeListVector.empty("fixedlist", 5,
allocator)) {
+      Field listVectorField = writeVector.getField();
+      childrenBuilder.add(listVectorField);
+      Field listFixedVectorField = writeFixedVector.getField();
+      childrenBuilder.add(listFixedVectorField);
+    }
+
+    Field initialField = new Field("a", FieldType.nullable(Struct.INSTANCE), childrenBuilder.build());
+    ImmutableList.Builder<Field> parentBuilder = ImmutableList.builder();
+    parentBuilder.add(initialField);
+    FlatBufferBuilder builder = new FlatBufferBuilder();
+    builder.finish(initialField.getField(builder));
+    org.apache.arrow.flatbuf.Field flatBufField = org.apache.arrow.flatbuf.Field.getRootAsField(builder.dataBuffer());
+    Field finalField = Field.convertField(flatBufField);
+    assertEquals(initialField, finalField);
+    assertFalse(finalField.toString().contains("[DEFAULT]"));
+
+    Schema initialSchema = new Schema(parentBuilder.build());
+    String jsonSchema = initialSchema.toJson();
+    String modifiedSchema = jsonSchema.replace("$data$", "[DEFAULT]");
+
+    Schema tempSchema = Schema.fromJSON(modifiedSchema);
+    FlatBufferBuilder schemaBuilder = new FlatBufferBuilder();
+    org.apache.arrow.vector.types.pojo.Schema schema = new org.apache.arrow.vector.types.pojo.Schema(tempSchema.getFields());
+    schemaBuilder.finish(schema.getSchema(schemaBuilder));
+    Schema finalSchema = Schema.deserialize(ByteBuffer.wrap(schemaBuilder.sizedByteArray()));
+    assertFalse(finalSchema.toString().contains("[DEFAULT]"));
+  }
+
+  @Test
   public void schema() {
     ImmutableList.Builder<Field> childrenBuilder = ImmutableList.builder();
     childrenBuilder.add(new Field("child1", FieldType.nullable(Utf8.INSTANCE), null));

-- 
To stop receiving notification emails like this one, please contact
['"commits@arrow.apache.org" <commits@arrow.apache.org>'].

Mime
View raw message