Skip to content

ARROW-277: Fix Flatbuffer serialization for Timestamp type #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
StevenMPhillips wants to merge 1 commit into from
Closed

ARROW-277: Fix Flatbuffer serialization for Timestamp type #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion java/vector/src/main/codegen/templates/ArrowType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,16 @@ public byte getTypeType() {

@Override
public int getType(FlatBufferBuilder builder) {
<#list type.fields as field>
<#if field.type == "String">
int ${field.name} = builder.createString(this.${field.name});
<#else>
${field.type} ${field.name} = this.${field.name};
</#if>
</#list>
org.apache.arrow.flatbuf.${type.name}.start${type.name}(builder);
<#list type.fields as field>
org.apache.arrow.flatbuf.${type.name}.add${field.name?cap_first}(builder, <#if field.type == "String">builder.createString(${field.name})<#else>${field.name}</#if>);
org.apache.arrow.flatbuf.${type.name}.add${field.name?cap_first}(builder, ${field.name});
</#list>
return org.apache.arrow.flatbuf.${type.name}.end${type.name}(builder);
}
Expand Down
86 changes: 43 additions & 43 deletions java/vector/src/main/java/org/apache/arrow/vector/types/Types.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ public class Types {
public enum MinorType {
NULL(Null.INSTANCE) {
@Override
public Field getField() {
return NULL_FIELD;
public Field getField(String name) {
return NULL_FIELD.newFieldWithName(name);
Copy link
Member

@julienledem julienledem Aug 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minor type is derived from the Field object.
We should be able to just pass the Field along wherever we pass the minor type and use that instead of trying to reconstruct it from the MinorType. The minorType can always be derived from Field when needed.

}

@Override
Expand All @@ -131,7 +131,7 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
},
MAP(Tuple.INSTANCE) {
@Override
public Field getField() {
public Field getField(String name) {
throw new UnsupportedOperationException("Cannot get simple field for Map type");
}

Expand All @@ -148,8 +148,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {

TINYINT(new Int(8, true)) {
@Override
public Field getField() {
return TINYINT_FIELD;
public Field getField(String name) {
return TINYINT_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -164,8 +164,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // single byte signed integer
SMALLINT(new Int(16, true)) {
@Override
public Field getField() {
return SMALLINT_FIELD;
public Field getField(String name) {
return SMALLINT_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -180,8 +180,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // two byte signed integer
INT(new Int(32, true)) {
@Override
public Field getField() {
return INT_FIELD;
public Field getField(String name) {
return INT_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -196,8 +196,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // four byte signed integer
BIGINT(new Int(64, true)) {
@Override
public Field getField() {
return BIGINT_FIELD;
public Field getField(String name) {
return BIGINT_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -212,8 +212,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // eight byte signed integer
DATE(Date.INSTANCE) {
@Override
public Field getField() {
return DATE_FIELD;
public Field getField(String name) {
return DATE_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -228,8 +228,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // days since 4713bc
TIME(Time.INSTANCE) {
@Override
public Field getField() {
return TIME_FIELD;
public Field getField(String name) {
return TIME_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -244,8 +244,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // time in micros before or after 2000/1/1
TIMESTAMP(new Timestamp("")) {
@Override
public Field getField() {
return TIMESTAMP_FIELD;
public Field getField(String name) {
return TIMESTAMP_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -260,8 +260,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
},
INTERVALDAY(IntervalDay.INSTANCE) {
@Override
public Field getField() {
return INTERVALDAY_FIELD;
public Field getField(String name) {
return INTERVALDAY_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -276,8 +276,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
},
INTERVALYEAR(IntervalYear.INSTANCE) {
@Override
public Field getField() {
return INTERVALYEAR_FIELD;
public Field getField(String name) {
return INTERVALYEAR_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -292,8 +292,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
},
FLOAT4(new FloatingPoint(0)) {
@Override
public Field getField() {
return FLOAT4_FIELD;
public Field getField(String name) {
return FLOAT4_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -308,8 +308,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // 4 byte ieee 754
FLOAT8(new FloatingPoint(1)) {
@Override
public Field getField() {
return FLOAT8_FIELD;
public Field getField(String name) {
return FLOAT8_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -324,8 +324,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // 8 byte ieee 754
BIT(Bool.INSTANCE) {
@Override
public Field getField() {
return BIT_FIELD;
public Field getField(String name) {
return BIT_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -340,8 +340,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // single bit value (boolean)
VARCHAR(Utf8.INSTANCE) {
@Override
public Field getField() {
return VARCHAR_FIELD;
public Field getField(String name) {
return VARCHAR_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -356,8 +356,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // utf8 variable length string
VARBINARY(Binary.INSTANCE) {
@Override
public Field getField() {
return VARBINARY_FIELD;
public Field getField(String name) {
return VARBINARY_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -376,7 +376,7 @@ public ArrowType getType() {
throw new UnsupportedOperationException("Cannot get simple type for Decimal type");
}
@Override
public Field getField() {
public Field getField(String name) {
throw new UnsupportedOperationException("Cannot get simple field for Decimal type");
}

Expand All @@ -392,8 +392,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // variable length binary
UINT1(new Int(8, false)) {
@Override
public Field getField() {
return UINT1_FIELD;
public Field getField(String name) {
return UINT1_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -408,8 +408,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // unsigned 1 byte integer
UINT2(new Int(16, false)) {
@Override
public Field getField() {
return UINT2_FIELD;
public Field getField(String name) {
return UINT2_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -424,8 +424,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // unsigned 2 byte integer
UINT4(new Int(32, false)) {
@Override
public Field getField() {
return UINT8_FIELD;
public Field getField(String name) {
return UINT8_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -440,8 +440,8 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // unsigned 4 byte integer
UINT8(new Int(64, false)) {
@Override
public Field getField() {
return UINT8_FIELD;
public Field getField(String name) {
return UINT8_FIELD.newFieldWithName(name);
}

@Override
Expand All @@ -456,7 +456,7 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
}, // unsigned 8 byte integer
LIST(List.INSTANCE) {
@Override
public Field getField() {
public Field getField(String name) {
throw new UnsupportedOperationException("Cannot get simple field for List type");
}

Expand All @@ -472,7 +472,7 @@ public FieldWriter getNewFieldWriter(ValueVector vector) {
},
UNION(Union.INSTANCE) {
@Override
public Field getField() {
public Field getField(String name) {
throw new UnsupportedOperationException("Cannot get simple field for Union type");
}

Expand All @@ -497,7 +497,7 @@ public ArrowType getType() {
return type;
}

public abstract Field getField();
public abstract Field getField(String name);

public abstract ValueVector getNewVector(String name, BufferAllocator allocator, CallBack callBack, int... precisionScale);

Expand Down
4 changes: 4 additions & 0 deletions java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Field.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,8 @@ public boolean equals(Object obj) {
(this.children.size() == 0 && that.children == null));

}

public Field newFieldWithName(String name) {
return new Field(name, this.nullable, this.type, ImmutableList.copyOf(this.children));
}
}
19 changes: 17 additions & 2 deletions java/vector/src/test/java/org/apache/arrow/vector/pojo/TestConvert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
package org.apache.arrow.vector.pojo;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.flatbuffers.FlatBufferBuilder;
import org.apache.arrow.vector.types.Types.MinorType;
import org.apache.arrow.vector.types.pojo.ArrowType.FloatingPoint;
import org.apache.arrow.vector.types.pojo.ArrowType.Int;
import org.apache.arrow.vector.types.pojo.ArrowType.Tuple;
Expand All @@ -28,6 +30,7 @@
import org.junit.Test;

import java.util.List;
import java.util.Set;

import static org.junit.Assert.assertEquals;

Expand All @@ -36,10 +39,22 @@
*/
public class TestConvert {

private static final Set<MinorType> NO_SIMPLE = ImmutableSet.of(
MinorType.MAP,
MinorType.LIST,
MinorType.UNION,
MinorType.DECIMAL
);

@Test
public void simple() {
Field initialField = new Field("a", true, new Int(32, true), null);
run(initialField);
for (MinorType minorType : MinorType.values()) {
if (NO_SIMPLE.contains(minorType)) {
continue;
}
Field field = minorType.getField(minorType.name());
run(field);
}
}

@Test
Expand Down