rdblue commented on a change in pull request #1740:
URL: https://github.com/apache/iceberg/pull/1740#discussion_r520192582
##########
File path:
mr/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerBaseTest.java
##########
@@ -211,6 +212,11 @@ public void testJoinTables() throws IOException {
Assert.assertArrayEquals(new Object[] {0L, "Alice", 100L, 11.11d},
rows.get(0));
Assert.assertArrayEquals(new Object[] {0L, "Alice", 101L, 22.22d},
rows.get(1));
Assert.assertArrayEquals(new Object[] {1L, "Bob", 102L, 33.33d},
rows.get(2));
+
+ joinTables("decimaltable", "decimal_col", Types.DecimalType.of(3, 1));
+ joinTables("timestamptable", "timestamp_col",
Types.TimestampType.withZone());
+ joinTables("binarytable", "binary_col", Types.BinaryType.get());
+ joinTables("datetable", "date_col", Types.DateType.get());
Review comment:
Could you rewrite this as a new test case with a list of types to test?
We consider it a best practice to start new test cases rather than adding to
existing, complete cases. Each test method is run independently so you see more
of the failures that way. By making longer test methods with more than one
case, failures can prevent other tests from even running.
I think it would be fine to use a loop over types in a single new case,
since most of the code is the same for these.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
