RussellSpitzer commented on code in PR #11324:
URL: https://github.com/apache/iceberg/pull/11324#discussion_r1826235415
##########
core/src/test/java/org/apache/iceberg/TestTableMetadata.java:
##########
@@ -1687,6 +1687,44 @@ public void testV3TimestampNanoTypeSupport() {
3);
}
+ @Test
+ public void testV3VariantTypeSupport() {
Review Comment:
I know this is copying a lot of tests in this class but we should start
future proofing a bit more imho. We also have tests around this sort of thing
in TestSchema.java. I think it is probably ok to just keep all of our schema
validation tests there, but it wouldn't hurt to have some redundancy here as
well.
Refactoring the whole suite can come in another pr but I think we should
build a templated test that's something like
```java
@ParameterizedTest
@ValueSource(types = {Types.TimetstampNanos, Types.Variant, ....})
testTypeSupport(Type type) {
Schema schemaWithType = new Schema(
Types.NestedField.required(1, "id", Types.LongType.get()),
Types.NestedField.optional(2, type.name, type),
Types.NestedField.optional(3, "arr", Types.ListType.ofRequired(4,
type)),
Types.NestedField.required(5, "struct",
Types.StructType.of(
Types.NestedField.optional(6, "inner_" + type.name, type),
Types.NestedField.required(7, "data",
Types.StringType.get()))),
Types.NestedField.optional(8, "struct_arr",
Types.StructType.of(
Types.NestedField.optional(9, "ts", type))));
//Psuedo code here
from 0 -> MIN_FORMAT_VERSION.get(type)
fail to make metadata
from MIN_FORMAT_VERSION.get(type) -> SUPPORTED_TABLE_VERSION
succeed
}
```
The most important part about this is that we wouldn't have to continually
update tests every time a new valid metadata version is added. It also would be
much easier to test type compatibility. (I'm thinking that Geo is going to need
the exact same thing soon)
For this PR I think it is enough to write a parameterized version for just
Variant, then we could raise another PR to add in nanos and remove the
redundant tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
