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 2020/07/14 03:06:59 UTC

[GitHub] [iceberg] JingsongLi opened a new pull request #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

JingsongLi opened a new pull request #1200:
URL: https://github.com/apache/iceberg/pull/1200


   ## What is the purpose of the change
   In `HadoopCatalog.dropNamespace`, throws `RuntimeIOException("Namespace delete failed")` when Namespace is not empty, but according to `SupportsNamespaces.dropNamespace`, should throw `NamespaceNotEmptyException`.
   
   ## Brief change log
   Before delete, check `listStatus` first.


----------------------------------------------------------------
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:
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 #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

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


   


----------------------------------------------------------------
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:
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] rdsr commented on a change in pull request #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

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



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -276,8 +277,11 @@ public boolean dropNamespace(Namespace namespace) {
     }
 
     try {
-      return fs.delete(nsPath, false /* recursive */);
+      if (fs.listStatus(nsPath).length != 0) {

Review comment:
       `listStatus` returns an array which means it lists all the files before returning. Should we use `listFiles` which returns an Iterator. Iterator could be a lot more efficient and we can simply do `listFiles.hasNext`




----------------------------------------------------------------
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:
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] rdsr commented on a change in pull request #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

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



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -276,8 +277,11 @@ public boolean dropNamespace(Namespace namespace) {
     }
 
     try {
-      return fs.delete(nsPath, false /* recursive */);
+      if (fs.listStatus(nsPath).length != 0) {

Review comment:
       `listStatus` returns an array which means it lists all the files before returning. Should we use `listFiles` which returns an Iterator. Iterator could be a lot more efficient as we do not need to list all the underlying files in the dir and we can simply do `listFiles.hasNext`




----------------------------------------------------------------
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:
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] JingsongLi commented on a change in pull request #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

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



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -276,8 +277,11 @@ public boolean dropNamespace(Namespace namespace) {
     }
 
     try {
-      return fs.delete(nsPath, false /* recursive */);
+      if (fs.listStatus(nsPath).length != 0) {

Review comment:
       Good point! Sounds good to me. We can use `listStatusIterator`.




----------------------------------------------------------------
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:
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 #1200: HadoopCatalog.dropNamespace throw NamespaceNotEmptyException when Namespace is not empty

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


   Thanks for fixing this, @JingsongLi!


----------------------------------------------------------------
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:
users@infra.apache.org



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