[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/carbondata/pull/2868


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[manishgupta88]

Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229578811
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
@@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final 
File... path)
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < path.length; i++) {
-  deleteRecursive(path[i]);
+  CarbonFile carbonFile = 
FileFactory.getCarbonFile(path[i].getAbsolutePath());
+  boolean delete = carbonFile.delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
--- End diff --

ok


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[manishgupta88]

Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229578798
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
@@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final 
File... path)
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < path.length; i++) {
-  deleteRecursive(path[i]);
+  CarbonFile carbonFile = 
FileFactory.getCarbonFile(path[i].getAbsolutePath());
+  boolean delete = carbonFile.delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
+  }
 }
 return null;
   }
 });
   }
 
-  /**
-   * Recursively delete the files
-   *
-   * @param f File to be deleted
-   * @throws IOException
-   */
-  private static void deleteRecursive(File f) throws IOException {
-if (f.isDirectory()) {
-  File[] files = f.listFiles();
-  if (null != files) {
-for (File c : files) {
-  deleteRecursive(c);
-}
-  }
-}
-if (f.exists() && !f.delete()) {
-  throw new IOException("Error while deleting the folders and files");
-}
-  }
-
   public static void deleteFoldersAndFiles(final CarbonFile... file)
   throws IOException, InterruptedException {
 UserGroupInformation.getLoginUser().doAs(new 
PrivilegedExceptionAction() {
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < file.length; i++) {
-  deleteRecursive(file[i]);
+  boolean delete = file[i].delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
--- End diff --

ok


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[jackylk]

Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229577891
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
@@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final 
File... path)
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < path.length; i++) {
-  deleteRecursive(path[i]);
+  CarbonFile carbonFile = 
FileFactory.getCarbonFile(path[i].getAbsolutePath());
+  boolean delete = carbonFile.delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
+  }
 }
 return null;
   }
 });
   }
 
-  /**
-   * Recursively delete the files
-   *
-   * @param f File to be deleted
-   * @throws IOException
-   */
-  private static void deleteRecursive(File f) throws IOException {
-if (f.isDirectory()) {
-  File[] files = f.listFiles();
-  if (null != files) {
-for (File c : files) {
-  deleteRecursive(c);
-}
-  }
-}
-if (f.exists() && !f.delete()) {
-  throw new IOException("Error while deleting the folders and files");
-}
-  }
-
   public static void deleteFoldersAndFiles(final CarbonFile... file)
   throws IOException, InterruptedException {
 UserGroupInformation.getLoginUser().doAs(new 
PrivilegedExceptionAction() {
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < file.length; i++) {
-  deleteRecursive(file[i]);
+  boolean delete = file[i].delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
--- End diff --

better to print the file location


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[jackylk]

Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229577855
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
@@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final 
File... path)
 
   @Override public Void run() throws Exception {
 for (int i = 0; i < path.length; i++) {
-  deleteRecursive(path[i]);
+  CarbonFile carbonFile = 
FileFactory.getCarbonFile(path[i].getAbsolutePath());
+  boolean delete = carbonFile.delete();
+  if (!delete) {
+throw new IOException("Error while deleting the folders and 
files");
--- End diff --

better to print the file location


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[manishgupta88]

Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229363551
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/LocalCarbonFile.java
 ---
@@ -141,7 +141,12 @@ public boolean renameTo(String changetoName) {
   }
 
   public boolean delete() {
-return file.delete();
+try {
+  return deleteFile(file.getAbsolutePath(), 
FileFactory.getFileType(file.getAbsolutePath()));
+} catch (IOException e) {
+  LOGGER.error("Exception occurred:" + e.getMessage());
--- End diff --

ok


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[manishgupta88]

Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229363490
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/CarbonFile.java
 ---
@@ -62,6 +62,11 @@
 
   boolean renameForce(String changetoName);
 
+  /**
+   * This method will delete the files recursively from file system
+   *
+   * @return
--- End diff --

ok


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[jackylk]

Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229283191
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/LocalCarbonFile.java
 ---
@@ -141,7 +141,12 @@ public boolean renameTo(String changetoName) {
   }
 
   public boolean delete() {
-return file.delete();
+try {
+  return deleteFile(file.getAbsolutePath(), 
FileFactory.getFileType(file.getAbsolutePath()));
+} catch (IOException e) {
+  LOGGER.error("Exception occurred:" + e.getMessage());
--- End diff --

include the exception in the error log


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[jackylk]

Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229283056
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/CarbonFile.java
 ---
@@ -62,6 +62,11 @@
 
   boolean renameForce(String changetoName);
 
+  /**
+   * This method will delete the files recursively from file system
+   *
+   * @return
--- End diff --

complete the comment


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[manishgupta88]

Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229173642
  
--- Diff: 
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SubqueryWithFilterAndSortTestCase.scala
 ---
@@ -64,15 +67,14 @@ class SubqueryWithFilterAndSortTestCase extends 
QueryTest with BeforeAndAfterAll
 dis.close()
   }
   def deleteFile(filePath: String) {
-val file = FileFactory.getCarbonFile(filePath, 
FileFactory.getFileType(filePath))
+val file = new File(filePath)
--- End diff --

Not required. I will remove


---

[GitHub] carbondata pull request #2868: [CARBONDATA-3052] Improve drop table performa...

[xuchuanyin]

Github user xuchuanyin commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2868#discussion_r229146066
  
--- Diff: 
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SubqueryWithFilterAndSortTestCase.scala
 ---
@@ -64,15 +67,14 @@ class SubqueryWithFilterAndSortTestCase extends 
QueryTest with BeforeAndAfterAll
 dis.close()
   }
   def deleteFile(filePath: String) {
-val file = FileFactory.getCarbonFile(filePath, 
FileFactory.getFileType(filePath))
+val file = new File(filePath)
--- End diff --

why is this modification needed?


---