[
https://issues.apache.org/jira/browse/ARROW-1663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16239776#comment-16239776
]
ASF GitHub Bot commented on ARROW-1663:
---------------------------------------
wesm closed pull request #1193: ARROW-1663: [Java] use consistent name for null
and not-null in FixedSizeLis…
URL: https://github.com/apache/arrow/pull/1193
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 b267b2087..5ac00375f 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 @@
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 void clear() {
@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 6511efcb7..ea28a6061 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.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.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 int getBufferSize() {
@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 48e71a976..eba149bf7 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 @@
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.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 static Field
convertField(org.apache.arrow.flatbuf.Field 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 static Field
convertField(org.apache.arrow.flatbuf.Field 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 5677f2566..43d9387b1 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.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 void testTransferPair() {
}
}
}
+
+ @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 1c9b57499..59e1646e8 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 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 void testConsistentChildName() throws Exception {
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 f98aeac8c..f6f1ad221 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 @@
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 void complex() {
}
@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));
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> [Java] Follow up on ARROW-1347 and make schema backward compatible
> ------------------------------------------------------------------
>
> Key: ARROW-1663
> URL: https://issues.apache.org/jira/browse/ARROW-1663
> Project: Apache Arrow
> Issue Type: Bug
> Components: Java - Vectors
> Reporter: Yuliya Feldman
> Assignee: Yuliya Feldman
> Labels: pull-request-available
> Fix For: 0.8.0
>
>
> ARROW-1347 covered ListVector to have name of the field $data$ instead of
> [DEFAULT]
> We left FixedSizeListVector behind.
> Another case is backward compatibility - if schema was created before
> ARROW-1347 was in place application may still suffer from side effects as it
> would not be updated based on new code.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
