Repository: carbondata
Updated Branches:
  refs/heads/master 25d949cfa -> 759cb31f6


[CARBONDATA-2954]Fix error when create external table command fired if path 
already exists

Problem : Creating a external table and providing a valid location having some 
empty directory and .carbondata
files was giving "operation not allowed: invalid datapath provided" error.

Solution: It was happening because if the location was having some empty 
directory getFilePathExternalFilePath method in
carbonutil.java was returning null due to the presence of empty directory.So 
made a slight modification to prevent this problem.

This closes #2739


Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo
Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/759cb31f
Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/759cb31f
Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/759cb31f

Branch: refs/heads/master
Commit: 759cb31f64c22b1dd67b1b90e2edb89380f36094
Parents: 25d949c
Author: shardul-cr7 <[email protected]>
Authored: Thu Sep 20 19:42:54 2018 +0530
Committer: kumarvishal09 <[email protected]>
Committed: Mon Sep 24 15:13:55 2018 +0530

----------------------------------------------------------------------
 .../core/metadata/schema/table/CarbonTable.java         |  8 +++++++-
 .../org/apache/carbondata/core/util/CarbonUtil.java     |  9 ++++++++-
 .../createTable/TestNonTransactionalCarbonTable.scala   | 10 ++++++++++
 .../apache/carbondata/sdk/file/CarbonReaderTest.java    | 12 ++++++++++++
 4 files changed, 37 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/carbondata/blob/759cb31f/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java
 
b/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java
index c606063..3d04cca 100644
--- 
a/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java
+++ 
b/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java
@@ -261,7 +261,13 @@ public class CarbonTable implements Serializable {
     CarbonFile[] carbonFiles = tablePath.listFiles();
     for (CarbonFile carbonFile : carbonFiles) {
       if (carbonFile.isDirectory()) {
-        return getFirstIndexFile(carbonFile);
+        // if the list has directories that doesn't contain index files,
+        // continue checking other files/directories in the list.
+        if (getFirstIndexFile(carbonFile) == null) {
+          continue;
+        } else {
+          return getFirstIndexFile(carbonFile);
+        }
       } else if 
(carbonFile.getName().endsWith(CarbonTablePath.INDEX_FILE_EXT)) {
         return carbonFile;
       }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/759cb31f/core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java 
b/core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java
index 5a85b14..03054bf 100644
--- a/core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java
+++ b/core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java
@@ -2230,9 +2230,16 @@ public final class CarbonUtil {
       if (dataFile.getName().endsWith(CarbonCommonConstants.FACT_FILE_EXT)) {
         return dataFile.getAbsolutePath();
       } else if (dataFile.isDirectory()) {
-        return getFilePathExternalFilePath(dataFile.getAbsolutePath(), 
configuration);
+        // if the list has directories that doesn't contain data files,
+        // continue checking other files/directories in the list.
+        if (getFilePathExternalFilePath(dataFile.getAbsolutePath(), 
configuration) == null) {
+          continue;
+        } else {
+          return getFilePathExternalFilePath(dataFile.getAbsolutePath(), 
configuration);
+        }
       }
     }
+    //returning null only if the path doesn't have data files.
     return null;
   }
 

http://git-wip-us.apache.org/repos/asf/carbondata/blob/759cb31f/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
----------------------------------------------------------------------
diff --git 
a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
 
b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
index b80a2f2..f6d12ab 100644
--- 
a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
+++ 
b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
@@ -740,6 +740,16 @@ class TestNonTransactionalCarbonTable extends QueryTest 
with BeforeAndAfterAll {
   test("test create External Table With Schema, should ignore the schema 
provided") {
     buildTestDataSingleFile()
     assert(new File(writerPath).exists())
+
+    val path1 = writerPath + "/0testdir"
+    val path2 = writerPath + "/testdir"
+
+    FileFactory.getCarbonFile(path1, FileFactory.getFileType(path1))
+    FileFactory.mkdirs(path1, FileFactory.getFileType(path1))
+
+    FileFactory.getCarbonFile(path2, FileFactory.getFileType(path2))
+    FileFactory.mkdirs(path2, FileFactory.getFileType(path2))
+
     sql("DROP TABLE IF EXISTS sdkOutputTable")
 
     // with schema

http://git-wip-us.apache.org/repos/asf/carbondata/blob/759cb31f/store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java
----------------------------------------------------------------------
diff --git 
a/store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java 
b/store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java
index dedf30e..e324aca 100644
--- 
a/store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java
+++ 
b/store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java
@@ -27,6 +27,7 @@ import 
org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
 import org.apache.carbondata.common.logging.LogService;
 import org.apache.carbondata.common.logging.LogServiceFactory;
 import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.datastore.impl.FileFactory;
 import org.apache.carbondata.core.metadata.datatype.DataTypes;
 import 
org.apache.carbondata.core.metadata.schema.table.DiskBasedDMSchemaStorageProvider;
 import org.apache.carbondata.core.scan.expression.ColumnExpression;
@@ -201,6 +202,17 @@ public class CarbonReaderTest extends TestCase {
     String path = "./testWriteFiles";
     FileUtils.deleteDirectory(new File(path));
 
+    String path1 = path + "/0testdir";
+    String path2 = path + "/testdir";
+
+    FileUtils.deleteDirectory(new File(path));
+
+    FileFactory.getCarbonFile(path1, FileFactory.getFileType(path1));
+    FileFactory.mkdirs(path1, FileFactory.getFileType(path1));
+
+    FileFactory.getCarbonFile(path2, FileFactory.getFileType(path2));
+    FileFactory.mkdirs(path2, FileFactory.getFileType(path2));
+
     Field[] fields = new Field[2];
     fields[0] = new Field("name", DataTypes.STRING);
     fields[1] = new Field("age", DataTypes.INT);

Reply via email to