Skip to content
Merged
Prev Previous commit
Next Next commit
Address code review comments
  • Loading branch information
nkollar committed Sep 7, 2018
commit fca77abc7b901259d1c3efeed597391f333e4bc1
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ List<Type> mergeFields(GroupType toMerge, boolean strict) {
throw new IncompatibleSchemaModificationException("repetition constraint is more restrictive: can not merge type " + fieldToMerge + " into " + type);
}
if (type.getLogicalTypeAnnotation() != null && !type.getLogicalTypeAnnotation().equals(fieldToMerge.getLogicalTypeAnnotation())) {
throw new IncompatibleSchemaModificationException("cannot merge original type " + fieldToMerge.getLogicalTypeAnnotation() + " into " + type.getLogicalTypeAnnotation());
throw new IncompatibleSchemaModificationException("cannot merge logical type " + fieldToMerge.getLogicalTypeAnnotation() + " into " + type.getLogicalTypeAnnotation());
}
merged = type.union(fieldToMerge, strict);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ PrimitiveComparator<?> comparator(LogicalTypeAnnotation logicalType) {
return logicalType.accept(new LogicalTypeAnnotation.LogicalTypeAnnotationVisitor<PrimitiveComparator>() {
@Override
public Optional<PrimitiveComparator> visit(LogicalTypeAnnotation.IntLogicalTypeAnnotation intLogicalType) {
if (intLogicalType.getBitWidth() != 64) {
return empty();
}
return intLogicalType.isSigned() ?
of(PrimitiveComparator.SIGNED_INT64_COMPARATOR) : of(PrimitiveComparator.UNSIGNED_INT64_COMPARATOR);
}
Expand All @@ -113,18 +110,12 @@ public Optional<PrimitiveComparator> visit(LogicalTypeAnnotation.DecimalLogicalT

@Override
public Optional<PrimitiveComparator> visit(LogicalTypeAnnotation.TimeLogicalTypeAnnotation timeLogicalType) {
if (timeLogicalType.getUnit() == MICROS) {
return of(PrimitiveComparator.SIGNED_INT64_COMPARATOR);
}
return empty();
return of(PrimitiveComparator.SIGNED_INT64_COMPARATOR);
}

@Override
public Optional<PrimitiveComparator> visit(LogicalTypeAnnotation.TimestampLogicalTypeAnnotation timestampLogicalType) {
if (timestampLogicalType.getUnit() == MICROS || timestampLogicalType.getUnit() == MILLIS) {
return of(PrimitiveComparator.SIGNED_INT64_COMPARATOR);
}
return empty();
return of(PrimitiveComparator.SIGNED_INT64_COMPARATOR);
}
}).orElseThrow(() -> new ShouldNeverHappenException("No comparator logic implemented for INT64 logical type: " + logicalType));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void testMergeSchema() {
t9.union(t10);
fail("moving from BINARY (UTF8) to BINARY");
} catch (IncompatibleSchemaModificationException e) {
assertEquals("cannot merge original type null into STRING", e.getMessage());
assertEquals("cannot merge logical type null into STRING", e.getMessage());
}

MessageType t11 = Types.buildMessage()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public FieldSchema convertBOOLEAN(PrimitiveTypeName primitiveTypeName)
@Override
public FieldSchema convertBINARY(PrimitiveTypeName primitiveTypeName)
throws FrontendException {
if (logicalTypeAnnotation != null && logicalTypeAnnotation instanceof LogicalTypeAnnotation.StringLogicalTypeAnnotation) {
if (logicalTypeAnnotation instanceof LogicalTypeAnnotation.StringLogicalTypeAnnotation) {
return new FieldSchema(fieldName, null, DataType.CHARARRAY);
} else {
return new FieldSchema(fieldName, null, DataType.BYTEARRAY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ public SimpleRecordConverter(GroupType schema, String name, SimpleRecordConverte
}

private Converter createConverter(Type field) {
LogicalTypeAnnotation otype = field.getLogicalTypeAnnotation();
LogicalTypeAnnotation ltype = field.getLogicalTypeAnnotation();

if (field.isPrimitive()) {
if (otype != null) {
return otype.accept(new LogicalTypeAnnotation.LogicalTypeAnnotationVisitor<Converter>() {
if (ltype != null) {
return ltype.accept(new LogicalTypeAnnotation.LogicalTypeAnnotationVisitor<Converter>() {
@Override
public Optional<Converter> visit(LogicalTypeAnnotation.StringLogicalTypeAnnotation stringLogicalType) {
return of(new StringConverter(field.getName()));
Expand All @@ -74,8 +74,8 @@ public Optional<Converter> visit(LogicalTypeAnnotation.DecimalLogicalTypeAnnotat
}

GroupType groupType = field.asGroupType();
if (otype != null) {
return otype.accept(new LogicalTypeAnnotation.LogicalTypeAnnotationVisitor<Converter>() {
if (ltype != null) {
return ltype.accept(new LogicalTypeAnnotation.LogicalTypeAnnotationVisitor<Converter>() {
@Override
public Optional<Converter> visit(LogicalTypeAnnotation.MapLogicalTypeAnnotation mapLogicalType) {
return of(new SimpleMapRecordConverter(groupType, field.getName(), SimpleRecordConverter.this));
Expand Down