You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/09/13 17:45:45 UTC

[GitHub] [iceberg] itachi-sharingan opened a new pull request #3097: Hadoop drop table to return false if table does not exist

itachi-sharingan opened a new pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097


   closes #3051 


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


[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706169396



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -247,26 +247,25 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
   @Override
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     if (!isValidIdentifier(identifier)) {
-      throw new NoSuchTableException("Invalid identifier: %s", identifier);
+      LOG.error("Invalid identifier: %s", identifier);
+      return false;
     }
 
     Path tablePath = new Path(defaultWarehouseLocation(identifier));
     TableOperations ops = newTableOps(identifier);
-    TableMetadata lastMetadata;
-    if (purge && ops.current() != null) {
-      lastMetadata = ops.current();
-    } else {
-      lastMetadata = null;
+    TableMetadata lastMetadata = ops.current();
+    if (lastMetadata == null) {
+      LOG.error("Not an iceberg table: %s", identifier);
+      return false;

Review comment:
       I would prefer keeping the returns in the same if block as the other return statement. I tend to always lean towards single or very closely located exits from functions so it's clear that you either return this or that.
   
   Like if you had it as
   ```
   if (lastMetadata == null) {
     LOG.error(....)
     return false
   } else {
     return fs.delete(...)
   }
   ```




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


[GitHub] [iceberg] itachi-sharingan commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706384135



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -247,26 +247,25 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
   @Override
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     if (!isValidIdentifier(identifier)) {
-      throw new NoSuchTableException("Invalid identifier: %s", identifier);

Review comment:
       sure, have rolled back my changes for this section.




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


[GitHub] [iceberg] itachi-sharingan commented on pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#issuecomment-918414354


   @rdblue have fixed the failing CI.
   @RussellSpitzer have added the check for non existent directory drop returning false in the same test.
   Pls check. 
   
   Moreover, can someone please suggest what would be a good issue to pick up next?


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


[GitHub] [iceberg] rdblue commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706859413



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -252,21 +252,19 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) {
 
     Path tablePath = new Path(defaultWarehouseLocation(identifier));
     TableOperations ops = newTableOps(identifier);
-    TableMetadata lastMetadata;
-    if (purge && ops.current() != null) {
-      lastMetadata = ops.current();
-    } else {
-      lastMetadata = null;
-    }
-
+    TableMetadata lastMetadata = ops.current();
     try {
-      if (purge && lastMetadata != null) {
-        // 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);
+      if (lastMetadata == null) {
+        LOG.error("Not an iceberg table: %s", identifier);

Review comment:
       This is not an error. The API states that if the table doesn't exist, then the catalog should return false. If you want to add a debug log, that's fine. But this should not be info or error level.




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


[GitHub] [iceberg] itachi-sharingan commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706879865



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
##########
@@ -237,6 +239,24 @@ public void testDropTable() throws Exception {
     Assert.assertFalse(fs.isDirectory(new Path(metaLocation)));
   }
 
+  @Test
+  public void testDropNonIcebergTable() throws Exception {
+    Configuration conf = new Configuration();
+    String warehousePath = temp.newFolder().getAbsolutePath();
+    HadoopCatalog catalog = new HadoopCatalog(conf, warehousePath);
+    TableIdentifier testTable = TableIdentifier.of("db", "ns1", "ns2", "tbl");
+    String metaLocation = catalog.defaultWarehouseLocation(testTable);
+
+    FileSystem fs = Util.getFs(new Path(metaLocation), conf);
+    File tablePath = new File(warehousePath + "/db/ns1/ns2/tbl");
+    tablePath.mkdirs();

Review comment:
       Makes sense, have made the change.




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


[GitHub] [iceberg] itachi-sharingan commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706384601



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -247,26 +247,25 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
   @Override
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     if (!isValidIdentifier(identifier)) {
-      throw new NoSuchTableException("Invalid identifier: %s", identifier);
+      LOG.error("Invalid identifier: %s", identifier);
+      return false;
     }
 
     Path tablePath = new Path(defaultWarehouseLocation(identifier));
     TableOperations ops = newTableOps(identifier);
-    TableMetadata lastMetadata;
-    if (purge && ops.current() != null) {
-      lastMetadata = ops.current();
-    } else {
-      lastMetadata = null;
+    TableMetadata lastMetadata = ops.current();
+    if (lastMetadata == null) {
+      LOG.error("Not an iceberg table: %s", identifier);
+      return false;

Review comment:
       Have tried doing something similar in the latest commit, please review.




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


[GitHub] [iceberg] itachi-sharingan edited a comment on pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan edited a comment on pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#issuecomment-918414354


   @rdblue have fixed the failing CI.
   @RussellSpitzer have added the check for non existent directory drop returning false in the same test.
   Hence, I am closing this pr.
   Moreover, can someone please suggest what would be a good issue to pick up next?


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


[GitHub] [iceberg] rdblue commented on pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#issuecomment-918379126


   @itachi-sharingan, could you add the test that Russell suggested and fix the failing CI checks? Otherwise, I think this is about ready to go.


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


[GitHub] [iceberg] rdblue merged pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097


   


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


[GitHub] [iceberg] rdblue commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r707507344



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -247,26 +247,25 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
   @Override
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     if (!isValidIdentifier(identifier)) {
-      throw new NoSuchTableException("Invalid identifier: %s", identifier);
+      LOG.error("Invalid identifier: %s", identifier);
+      return false;
     }
 
     Path tablePath = new Path(defaultWarehouseLocation(identifier));
     TableOperations ops = newTableOps(identifier);
-    TableMetadata lastMetadata;
-    if (purge && ops.current() != null) {
-      lastMetadata = ops.current();
-    } else {
-      lastMetadata = null;
+    TableMetadata lastMetadata = ops.current();
+    if (lastMetadata == null) {
+      LOG.error("Not an iceberg table: %s", identifier);
+      return false;

Review comment:
       Looks like this is fixed to me.




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


[GitHub] [iceberg] itachi-sharingan closed pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan closed pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097


   


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


[GitHub] [iceberg] rdblue commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706859589



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
##########
@@ -237,6 +239,24 @@ public void testDropTable() throws Exception {
     Assert.assertFalse(fs.isDirectory(new Path(metaLocation)));
   }
 
+  @Test
+  public void testDropNonIcebergTable() throws Exception {
+    Configuration conf = new Configuration();
+    String warehousePath = temp.newFolder().getAbsolutePath();
+    HadoopCatalog catalog = new HadoopCatalog(conf, warehousePath);
+    TableIdentifier testTable = TableIdentifier.of("db", "ns1", "ns2", "tbl");
+    String metaLocation = catalog.defaultWarehouseLocation(testTable);
+
+    FileSystem fs = Util.getFs(new Path(metaLocation), conf);
+    File tablePath = new File(warehousePath + "/db/ns1/ns2/tbl");
+    tablePath.mkdirs();
+    Assert.assertTrue(fs.isDirectory(new Path(metaLocation)));
+
+    Assert.assertFalse(catalog.dropTable(testTable));
+    Assert.assertTrue(fs.isDirectory(new Path(metaLocation)));
+    FileUtils.deleteDirectory(new File(warehousePath + "/db"));

Review comment:
       No need to delete this. The temp folder will handle it.




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


[GitHub] [iceberg] rdblue commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706859729



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
##########
@@ -237,6 +239,24 @@ public void testDropTable() throws Exception {
     Assert.assertFalse(fs.isDirectory(new Path(metaLocation)));
   }
 
+  @Test
+  public void testDropNonIcebergTable() throws Exception {
+    Configuration conf = new Configuration();
+    String warehousePath = temp.newFolder().getAbsolutePath();
+    HadoopCatalog catalog = new HadoopCatalog(conf, warehousePath);
+    TableIdentifier testTable = TableIdentifier.of("db", "ns1", "ns2", "tbl");
+    String metaLocation = catalog.defaultWarehouseLocation(testTable);
+
+    FileSystem fs = Util.getFs(new Path(metaLocation), conf);
+    File tablePath = new File(warehousePath + "/db/ns1/ns2/tbl");
+    tablePath.mkdirs();

Review comment:
       This shouldn't mix Hadoop FileSystem calls with the Java File API. Please only use the Hadoop calls based on metadataLocation. That way you don't need to assume a location for the table.




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


[GitHub] [iceberg] itachi-sharingan commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706879798



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
##########
@@ -237,6 +239,24 @@ public void testDropTable() throws Exception {
     Assert.assertFalse(fs.isDirectory(new Path(metaLocation)));
   }
 
+  @Test
+  public void testDropNonIcebergTable() throws Exception {
+    Configuration conf = new Configuration();
+    String warehousePath = temp.newFolder().getAbsolutePath();
+    HadoopCatalog catalog = new HadoopCatalog(conf, warehousePath);
+    TableIdentifier testTable = TableIdentifier.of("db", "ns1", "ns2", "tbl");
+    String metaLocation = catalog.defaultWarehouseLocation(testTable);
+
+    FileSystem fs = Util.getFs(new Path(metaLocation), conf);
+    File tablePath = new File(warehousePath + "/db/ns1/ns2/tbl");
+    tablePath.mkdirs();
+    Assert.assertTrue(fs.isDirectory(new Path(metaLocation)));
+
+    Assert.assertFalse(catalog.dropTable(testTable));
+    Assert.assertTrue(fs.isDirectory(new Path(metaLocation)));
+    FileUtils.deleteDirectory(new File(warehousePath + "/db"));

Review comment:
       ok




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


[GitHub] [iceberg] rdblue commented on pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#issuecomment-918428118


   Thanks, @itachi-sharingan! I reopened this so that I could merge it. There's no need to close a PR when it is finished.
   
   If you're looking for another contribution, #2488 is a good one! Thanks for contributing!


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


[GitHub] [iceberg] itachi-sharingan commented on pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#issuecomment-917117088


   Have added test for the other case of exit because of non iceberg table, please review.


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


[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706166892



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -247,26 +247,25 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
   @Override
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     if (!isValidIdentifier(identifier)) {
-      throw new NoSuchTableException("Invalid identifier: %s", identifier);

Review comment:
       This is actually a dead code block for this class. If you check isValidIdentifier it is ```return true```. I think we should just leave this bit as is for now and maybe clean it up in a future 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


[GitHub] [iceberg] itachi-sharingan commented on a change in pull request #3097: Hadoop drop table to return false if table does not exist

Posted by GitBox <gi...@apache.org>.
itachi-sharingan commented on a change in pull request #3097:
URL: https://github.com/apache/iceberg/pull/3097#discussion_r706879695



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -252,21 +252,19 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) {
 
     Path tablePath = new Path(defaultWarehouseLocation(identifier));
     TableOperations ops = newTableOps(identifier);
-    TableMetadata lastMetadata;
-    if (purge && ops.current() != null) {
-      lastMetadata = ops.current();
-    } else {
-      lastMetadata = null;
-    }
-
+    TableMetadata lastMetadata = ops.current();
     try {
-      if (purge && lastMetadata != null) {
-        // 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);
+      if (lastMetadata == null) {
+        LOG.error("Not an iceberg table: %s", identifier);

Review comment:
       The reason I thought of putting it in error was calling iceberg api drop on a table which is not an iceberg table looked like an error but wasn't sure of this. But now I agree with you, so have made the log as debug now.




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