marton-bod commented on a change in pull request #2191:
URL: https://github.com/apache/iceberg/pull/2191#discussion_r569313944



##########
File path: 
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerLocalScan.java
##########
@@ -253,6 +253,7 @@ public void testCreateTableWithColumnSpecification() throws 
IOException {
         testTables.locationForCreateTableSQL(identifier);
     runCreateAndReadTest(identifier, createSql, 
HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
         PartitionSpec.unpartitioned(), data);
+    shell.executeStatement("DROP TABLE " + identifier);

Review comment:
       > We already have a test case for testCreateDropTable, so I think this 
part is covered. Do I miss something? Maybe we should create one where we add 
some data as well, if it is relevant
   
   That test case does not detect the difference of behaviour between Hive2/3 
and Hive4 with regards to purge=TRUE, because if we create a table and do not 
do any additional snapshot-creating operations on the table, then 
`metadata.snapshots()` will give back an empty collection, hence the 
deleteFiles operation will not run during `dropTableData` (and you won't get 
FileNotFoundExc for Hive4). That's why I added the DROP TABLE to some tests 
where additional operations (like inserts) also take place.
   
   On the other hand, not sure if `metadata.snapshots()` giving back an empty 
collection in this case is the right behaviour.
   
   > Could we create specific test cases instead for dropping the Iceberg table 
backing Hive table and then dropping the Hive table? (should only run for 
non-HiveCatalog tests)
   
   Yes, will do. 




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