Skip to content

Commit 824fc58

Browse files
fix(core): create precision time protobuf (#742)
The io.substrait.type.proto.TypeProtoConverter.Types#wrap(Object) method was missing a branch to handle PrecisionTime. This caused an UnsupportedOperationException to the be thrown when creating a PrecisionTime protobuf object. This change adds the missing branch to the wrap method, and extends type pojo/proto conversion tests to include PrecisionTime. Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>
1 parent ff3607d commit 824fc58

2 files changed

Lines changed: 47 additions & 45 deletions

File tree

core/src/main/java/io/substrait/type/proto/TypeProtoConverter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ protected Type wrap(final Object o) {
233233
return bldr.setFixedBinary((Type.FixedBinary) o).build();
234234
} else if (o instanceof Type.Decimal) {
235235
return bldr.setDecimal((Type.Decimal) o).build();
236+
} else if (o instanceof Type.PrecisionTime) {
237+
return bldr.setPrecisionTime((Type.PrecisionTime) o).build();
236238
} else if (o instanceof Type.PrecisionTimestamp) {
237239
return bldr.setPrecisionTimestamp((Type.PrecisionTimestamp) o).build();
238240
} else if (o instanceof Type.PrecisionTimestampTZ) {

core/src/test/java/io/substrait/type/proto/TestTypeRoundtrip.java

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,59 +6,59 @@
66
import io.substrait.extension.SimpleExtension;
77
import io.substrait.type.Type;
88
import io.substrait.type.TypeCreator;
9+
import java.util.stream.Stream;
10+
import org.junit.jupiter.api.DisplayName;
911
import org.junit.jupiter.params.ParameterizedTest;
10-
import org.junit.jupiter.params.provider.ValueSource;
12+
import org.junit.jupiter.params.provider.MethodSource;
1113

1214
class TestTypeRoundtrip {
1315

14-
private ExtensionCollector lookup = new ExtensionCollector();
15-
private TypeProtoConverter typeProtoConverter = new TypeProtoConverter(lookup);
16-
17-
private ProtoTypeConverter protoTypeConverter =
16+
private static ExtensionCollector lookup = new ExtensionCollector();
17+
private static TypeProtoConverter typeProtoConverter = new TypeProtoConverter(lookup);
18+
private static ProtoTypeConverter protoTypeConverter =
1819
new ProtoTypeConverter(lookup, SimpleExtension.ExtensionCollection.builder().build());
1920

20-
@ParameterizedTest
21-
@ValueSource(booleans = {true, false})
22-
void roundtrip(boolean n) {
23-
t(creator(n).BOOLEAN);
24-
t(creator(n).I8);
25-
t(creator(n).I16);
26-
t(creator(n).I32);
27-
t(creator(n).I64);
28-
t(creator(n).FP32);
29-
t(creator(n).FP64);
30-
t(creator(n).STRING);
31-
t(creator(n).BINARY);
32-
t(creator(n).TIME);
33-
t(creator(n).DATE);
34-
t(creator(n).TIMESTAMP);
35-
t(creator(n).TIMESTAMP_TZ);
36-
t(creator(n).INTERVAL_YEAR);
37-
t(creator(n).UUID);
38-
t(creator(n).fixedChar(25));
39-
t(creator(n).varChar(35));
40-
t(creator(n).fixedBinary(45));
41-
t(creator(n).decimal(34, 3));
42-
t(creator(n).intervalDay(6));
43-
t(creator(n).intervalCompound(3));
44-
t(creator(n).precisionTimestamp(1));
45-
t(creator(n).precisionTimestampTZ(2));
46-
t(creator(n).map(creator(n).I8, creator(n).I16));
47-
t(creator(n).list(creator(n).TIME));
48-
t(creator(n).struct(creator(n).TIME, creator(n).TIMESTAMP, creator(n).TIMESTAMP_TZ));
21+
static Stream<Type> types() {
22+
return Stream.of(true, false)
23+
.map(nullable -> nullable ? TypeCreator.NULLABLE : TypeCreator.REQUIRED)
24+
.flatMap(
25+
creator ->
26+
Stream.of(
27+
creator.BOOLEAN,
28+
creator.I8,
29+
creator.I16,
30+
creator.I32,
31+
creator.I64,
32+
creator.FP32,
33+
creator.FP64,
34+
creator.STRING,
35+
creator.BINARY,
36+
creator.TIME,
37+
creator.DATE,
38+
creator.TIMESTAMP,
39+
creator.TIMESTAMP_TZ,
40+
creator.INTERVAL_YEAR,
41+
creator.UUID,
42+
creator.fixedChar(25),
43+
creator.varChar(35),
44+
creator.fixedBinary(45),
45+
creator.decimal(34, 3),
46+
creator.intervalDay(6),
47+
creator.intervalCompound(3),
48+
creator.precisionTime(3),
49+
creator.precisionTimestamp(1),
50+
creator.precisionTimestampTZ(2),
51+
creator.map(creator.I8, creator.I16),
52+
creator.list(creator.TIME),
53+
creator.struct(creator.TIME, creator.TIMESTAMP, creator.TIMESTAMP_TZ)));
4954
}
5055

51-
/*
52-
* Test a type pojo -> proto -> pojo roundtrip.
53-
*
54-
* @param type
55-
*/
56-
private void t(Type type) {
56+
@ParameterizedTest
57+
@MethodSource("types")
58+
@DisplayName("pojo -> proto -> pojo round trip")
59+
void roundtripFromType(Type type) {
5760
io.substrait.proto.Type converted = type.accept(typeProtoConverter);
58-
assertEquals(type, protoTypeConverter.from(converted));
59-
}
60-
61-
private TypeCreator creator(boolean nullable) {
62-
return nullable ? TypeCreator.NULLABLE : TypeCreator.REQUIRED;
61+
Type actual = protoTypeConverter.from(converted);
62+
assertEquals(type, actual);
6363
}
6464
}

0 commit comments

Comments
 (0)