[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #5283: [WIP] Core: Fix drop table without purge for hadoop catalog

[GitBox <gi...@apache.org>]

ajantha-bhat commented on code in PR #5283:
URL: https://github.com/apache/iceberg/pull/5283#discussion_r922976027


##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java:
##########
@@ -254,8 +254,11 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) {
           // Since the data files and the metadata files may store in different locations,
           // so it has to call dropTableData to force delete the data file.
           CatalogUtil.dropTableData(ops.io(), lastMetadata);
+          return fs.delete(tablePath, true /* recursive */);
+        } else {
+          // just drop the version-hint.txt file

Review Comment:
   @rdblue: yeah. That's exactly why test cases are failing. 
   
   Also, it is odd to me that `HadoopCatalog` extends `BaseMetastoreCatalog` (as it should not be a metastore catalog. should be file system catalog). 
   
   I will mostly close this PR. 
   
   We are working on a catalog migration API (any catalog to any catalog and of course we will contribute it here). API is simple. But adding testcase with cross catalog is more work. 
   While adding API, after catalog migration, we call drop table with purge = false on source catalog. At that time Hadoop catalog was cleaning the migrated table's data too. Hence opened this PR.



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org