You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by aj...@apache.org on 2020/12/04 11:46:33 UTC

[carbondata] branch master updated: [CARBONDATA-4029] [CARBONDATA-3908] Issue while adding segments through alter add segment command

This is an automated email from the ASF dual-hosted git repository.

ajantha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/carbondata.git


The following commit(s) were added to refs/heads/master by this push:
     new d8799ff  [CARBONDATA-4029] [CARBONDATA-3908] Issue while adding segments through alter add segment command
d8799ff is described below

commit d8799ffac16f4fd660c53d9693d1c1f51f84a550
Author: akkio-97 <ak...@gmail.com>
AuthorDate: Mon Nov 2 21:28:08 2020 +0530

    [CARBONDATA-4029] [CARBONDATA-3908] Issue while adding segments through alter add segment command
    
    Why is this PR needed?
    While adding segments into carbon table using "alter add segment" command- if the HDFS location path has " / " file separator in the end, then it leads to NPE during full scan.
    
    What changes were proposed in this PR?
    Have removed this extra file separator.
    
    Does this PR introduce any user interface change?
    No
    
    Is any new testcase added?
    No
    
    This closes #4001
---
 .../sql/execution/command/management/CarbonAddLoadCommand.scala   | 8 +++++++-
 .../spark/testsuite/addsegment/AddSegmentTestCase.scala           | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
index 8434d9a..5fc16d5 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
@@ -34,6 +34,7 @@ import org.apache.spark.sql.types.StructType
 
 import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
 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.exception.ConcurrentOperationException
 import org.apache.carbondata.core.index.{IndexStoreManager, Segment}
@@ -89,8 +90,13 @@ case class CarbonAddLoadCommand(
       throw new ConcurrentOperationException(carbonTable, "insert overwrite", "add segment")
     }
 
-    val inputPath = options.getOrElse(
+    var givenPath = options.getOrElse(
       "path", throw new UnsupportedOperationException("PATH is mandatory"))
+    // remove file separator if already present
+    if (givenPath.charAt(givenPath.length - 1) == CarbonCommonConstants.FILE_SEPARATOR) {
+      givenPath = givenPath.substring(0, givenPath.length - 1)
+    }
+    val inputPath = givenPath
 
     // If a path is already added then we should block the adding of the same path again.
     val allSegments = SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
diff --git a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/addsegment/AddSegmentTestCase.scala b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/addsegment/AddSegmentTestCase.scala
index dddafd4..76f6ee8 100644
--- a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/addsegment/AddSegmentTestCase.scala
+++ b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/addsegment/AddSegmentTestCase.scala
@@ -63,6 +63,7 @@ class AddSegmentTestCase extends QueryTest with BeforeAndAfterAll {
     val table = CarbonEnv.getCarbonTable(None, "addsegment1") (sqlContext.sparkSession)
     val path = CarbonTablePath.getSegmentPath(table.getTablePath, "1")
     val newPath = storeLocation + "/" + "addsegtest"
+    val newPathWithLineSeparator = storeLocation + "/" + "addsegtest/"
     copy(path, newPath)
     sql("delete from table addsegment1 where segment.id in (1)")
     sql("clean files for table addsegment1")
@@ -72,6 +73,11 @@ class AddSegmentTestCase extends QueryTest with BeforeAndAfterAll {
       .collect()
     checkAnswer(sql("select count(*) from addsegment1"), Seq(Row(20)))
     checkAnswer(sql("select count(empname) from addsegment1"), Seq(Row(20)))
+
+    sql(s"alter table addsegment1 add segment options('path'='$newPathWithLineSeparator', " +
+        s"'format'='carbon')")
+      .collect()
+    checkAnswer(sql("select count(*) from addsegment1"), Seq(Row(30)))
     FileFactory.deleteAllFilesOfDir(new File(newPath))
   }