You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by in...@apache.org on 2020/10/07 08:18:11 UTC

[carbondata] branch master updated: [CARBONDATA-4017]Fix the insert issue when the column name contains '\' and fix SI creation issue

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

indhumuthumurugesh 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 aeb2aac  [CARBONDATA-4017]Fix the insert issue when the column name contains '\' and fix SI creation issue
aeb2aac is described below

commit aeb2aacef3806eaaf7dadc0503a9b6484de0d678
Author: akashrn5 <ak...@gmail.com>
AuthorDate: Mon Sep 28 19:57:42 2020 +0530

    [CARBONDATA-4017]Fix the insert issue when the column name contains '\' and fix SI creation issue
    
    Why is this PR needed?
    1. when the column name contains the backslash character and the table is created with carbon session,
       insert fails second time. This is because, the .fromGson method takes input string, if that has \,
       it will escape that char, eventhough its valid.
    2. when column name has special characters, SI creation fails in parsing.
    
    What changes were proposed in this PR?
    1. replace the \ char jason string to \\, so that only one character is escaped.
    2. Use CarbonSparkUtil.getRawSchema to get the schema which wrap `` characters around column names and
       helps in parsing.
    
    Does this PR introduce any user interface change?
    No
    
    Is any new testcase added?
    Yes
    
    This closes #3962
---
 .../java/org/apache/carbondata/core/util/CarbonUtil.java     |  4 +++-
 .../testsuite/secondaryindex/TestSIWithSecondryIndex.scala   | 12 ++++++++++++
 .../spark/sql/secondaryindex/command/SICreationCommand.scala | 12 ++++--------
 3 files changed, 19 insertions(+), 9 deletions(-)

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 defe7ab..8d77e05 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
@@ -1881,7 +1881,9 @@ public final class CarbonUtil {
     gsonBuilder.registerTypeAdapter(DataType.class, new DataTypeAdapter());
 
     Gson gson = gsonBuilder.create();
-    TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class);
+    // if the column name contains backslash in the column name, then fromJson will remove that,
+    // so replace like below to keep the "\" in column name and write the proper name in the schema
+    TableInfo tableInfo = gson.fromJson(builder.toString().replace("\\", "\\\\"), TableInfo.class);
 
     // The tableInfo is deserialize from GSON string, need to update the scale and
     // precision if there are any decimal field, because DecimalType is added in Carbon 1.3,
diff --git a/index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala b/index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
index cb5c73f..ea66e75 100644
--- a/index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
+++ b/index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
@@ -423,6 +423,17 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     sql("drop table table2")
   }
 
+  test("test SI creation with special char column") {
+    sql("drop table if exists special_char")
+    sql("create table special_char(`i#d` string, `nam(e` string,`ci)&#@!ty` string,`a\be` int, `ag!e` float, `na^me1` Decimal(8,4)) stored as carbondata")
+    sql("create index special_char_index on table special_char(`nam(e`) as 'carbondata'")
+    sql("insert into special_char values('1','joey','hud', 2, 2.2, 2.3456)")
+    val plan = sql("explain select * from special_char where `nam(e` = 'joey'").collect()(0).toString()
+    assert(plan.contains("special_char_index"))
+    val df = sql("describe formatted special_char_index").collect()
+    assert(df.exists(_.get(0).toString.contains("nam(e")))
+  }
+
   override def afterAll {
     sql("drop index si_altercolumn on table_WithSIAndAlter")
     sql("drop table if exists table_WithSIAndAlter")
@@ -433,5 +444,6 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     sql("drop table if exists uniqdataTable")
     sql("drop table if exists table_drop_columns")
     sql("drop table if exists table_drop_columns_fail")
+    sql("drop table if exists special_char")
   }
 }
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala b/integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala
index c033e8f..3792e89 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala
@@ -53,6 +53,7 @@ import org.apache.carbondata.core.statusmanager.{LoadMetadataDetails, SegmentSta
 import org.apache.carbondata.core.util.{CarbonProperties, CarbonUtil}
 import org.apache.carbondata.core.util.path.CarbonTablePath
 import org.apache.carbondata.events.{CreateTablePostExecutionEvent, CreateTablePreExecutionEvent, OperationContext, OperationListenerBus}
+import org.apache.carbondata.spark.util.CarbonSparkUtil
 
 class ErrorMessage(message: String) extends Exception(message) {
 }
@@ -323,13 +324,8 @@ private[sql] case class CarbonCreateSecondaryIndexCommand(
         IndexType.SI.getIndexProviderName,
         indexTableName,
         indexProperties)
-
-      val cols = tableInfo.getFactTable.getListOfColumns.asScala.filter(!_.isInvisible)
-      val fields = new Array[String](cols.size)
-      cols.foreach(col =>
-        fields(col.getSchemaOrdinal) =
-          col.getColumnName + ' ' + checkAndPrepareDecimal(col))
-
+      val carbonRelation = CarbonSparkUtil.createCarbonRelation(tableInfo, tablePath)
+      val rawSchema = CarbonSparkUtil.getRawSchema(carbonRelation)
       val operationContext = new OperationContext
       val createTablePreExecutionEvent: CreateTablePreExecutionEvent =
         CreateTablePreExecutionEvent(sparkSession, tableIdentifier, Option(tableInfo))
@@ -340,7 +336,7 @@ private[sql] case class CarbonCreateSecondaryIndexCommand(
         try {
           sparkSession.sql(
             s"""CREATE TABLE $databaseName.$indexTableName
-               |(${ fields.mkString(",") })
+               |($rawSchema)
                |USING carbondata OPTIONS (tableName "$indexTableName",
                |dbName "$databaseName", tablePath "$tablePath", path "$tablePath",
                |parentTablePath "${ carbonTable.getTablePath }", isIndexTable "true",