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

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

rdblue commented on code in PR #5283:
URL: https://github.com/apache/iceberg/pull/5283#discussion_r922717324


##########
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:
   The version hint is a hint. If any metadata file exists, then the table will still exist.
   
   I think that the original version is correct. For Hadoop tables, dropping a table means deleting its directory. The confusion here is one reason why Hadoop tables are not recommended for production use.



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