pvary commented on a change in pull request #1607:
URL: https://github.com/apache/iceberg/pull/1607#discussion_r528672097



##########
File path: 
mr/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerBaseTest.java
##########
@@ -512,4 +770,14 @@ protected void createHiveTable(String tableName, String 
location) {
   protected String locationForCreateTable(String tempDirName, String 
tableName) {
     return null;
   }
+
+  private List<Record> createTableWithGeneratedRecords(Schema schema, int 
numRecords, long seed, String tableName)
+          throws IOException {
+    List<Record> records = TestHelper.generateRandomRecords(schema, 
numRecords, seed);
+    createTable(tableName, schema, records);
+    // sanity check, fetch all rows
+    List<Object[]> allRows = shell.executeStatement("SELECT * from default." + 
tableName);
+    Assert.assertEquals("Number of rows doesn't match expected.", 
records.size(), allRows.size());

Review comment:
       Do we need this test? This adds an extra sql statement which could be 
costly when run 24 times. I think that if the number of rows are not correct 
when preparing the table, then the tests will fail anyway.
   Or did I miss something?




----------------------------------------------------------------
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]

Reply via email to