[carbondata] branch master updated: [CARBONDATA-4002] Fix removal of columns from table schema.

[ak...@apache.org]

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

akashrn5 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 c16747b  [CARBONDATA-4002] Fix removal of columns from table schema.
c16747b is described below

commit c16747ba8e68643df8e53ee912fe11f77ec3ea53
Author: Karan980 <ka...@gmail.com>
AuthorDate: Tue Sep 22 12:42:36 2020 +0530

    [CARBONDATA-4002] Fix removal of columns from table schema.
    
    Why is this PR needed?
    When we change the value of sort_columns property and then perform
    an alter table query to unset long_string_columns. It results in
    removal of columns from table schema.
    
    What changes were proposed in this PR?
    Added fix to avoid removal of columns from table schema.
    
    Does this PR introduce any user interface change?
    No
    
    Is any new testcase added?
    Yes
    
    This closes #3946
---
 .../apache/carbondata/core/util/CarbonUtil.java    | 13 ++++---
 .../org/apache/spark/util/AlterTableUtil.scala     |  4 ++-
 .../longstring/VarcharDataTypesBasicTestCase.scala | 40 ++++++++++++++++++++++
 3 files changed, 49 insertions(+), 8 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 21a34b6..defe7ab 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
@@ -3302,13 +3302,12 @@ public final class CarbonUtil {
     // so just take all other columns at once.
     int otherColumnStartIndex = -1;
     for (int i = 0; i < columns.size(); i++) {
-      if (columns.get(i).getColumnProperties() != null) {
-        String isSortColumn =
-            columns.get(i).getColumnProperties().get(CarbonCommonConstants.SORT_COLUMNS);
-        if ((isSortColumn != null) && (isSortColumn.equalsIgnoreCase("true"))) {
-          // add sort column dimensions
-          sortColumns.add(columns.get(i));
-        }
+      Map<String, String> columnProperties = columns.get(i).getColumnProperties();
+      if (columnProperties != null
+          && columnProperties.get(CarbonCommonConstants.SORT_COLUMNS) != null
+          && columnProperties.get(CarbonCommonConstants.SORT_COLUMNS).equalsIgnoreCase("true")) {
+        // add sort column dimensions
+        sortColumns.add(columns.get(i));
       } else if ((columns.get(i).getData_type() == org.apache.carbondata.format.DataType.ARRAY
           || columns.get(i).getData_type() == org.apache.carbondata.format.DataType.STRUCT
           || columns.get(i).getData_type() == org.apache.carbondata.format.DataType.MAP || (!columns
diff --git a/integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala b/integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
index 3c10571..be8dafb 100644
--- a/integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
@@ -442,7 +442,9 @@ object AlterTableUtil {
         // update schema for long string columns
         updateSchemaForLongStringColumns(thriftTable, longStringColumns.get)
       } else if (propKeys.exists(_.equalsIgnoreCase("long_string_columns") && !set)) {
-        updateSchemaForLongStringColumns(thriftTable, "")
+        if (tblPropertiesMap.exists(prop => prop._1.equalsIgnoreCase("long_string_columns"))) {
+          updateSchemaForLongStringColumns(thriftTable, "")
+        }
       }
       // below map will be used for cache invalidation. As tblProperties map is getting modified
       // in the next few steps the original map need to be retained for any decision making
diff --git a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala
index 8e1b114..d20b44b 100644
--- a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala
+++ b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala
@@ -35,6 +35,9 @@ import org.apache.carbondata.core.metadata.datatype.DataTypes
 import org.apache.carbondata.core.statusmanager.SegmentStatusManager
 import org.apache.carbondata.core.util.CarbonProperties
 
+import scala.collection.mutable
+import scala.collection.JavaConverters._
+
 class VarcharDataTypesBasicTestCase extends QueryTest with BeforeAndAfterEach with BeforeAndAfterAll {
   private val longStringTable = "long_string_table"
   private val inputDir = s"$resourcesPath${File.separator}varchartype${File.separator}"
@@ -434,6 +437,43 @@ class VarcharDataTypesBasicTestCase extends QueryTest with BeforeAndAfterEach wi
 
     sql("DROP TABLE IF EXISTS varchar_complex_table")
   }
+
+  test("check new schema after modifying schema through alter table queries when long_string_column is not present") {
+    sql(
+      s"""
+         | CREATE TABLE if not exists $longStringTable(
+         | id INT, name STRING, description STRING, address STRING, note STRING
+         | ) STORED AS carbondata
+         | TBLPROPERTIES('sort_columns'='id,name')
+         |""".
+        stripMargin)
+    sql(s"alter table long_string_table set tblproperties('sort_columns'='ID','sort_scope'='no_sort')")
+    sql(s"alter table long_string_table unset tblproperties('long_string_columns')")
+    val carbonTable = CarbonMetadata.getInstance().getCarbonTable(CarbonCommonConstants.DATABASE_DEFAULT_NAME,
+      "long_string_table")
+    val columns = carbonTable.getTableInfo.getFactTable.getListOfColumns.asScala.toList
+      .filter(column => column.getColumnName.equalsIgnoreCase("name"))
+    assert(columns != null && columns.size > 0 && columns.head.getColumnName.equals("name"))
+  }
+
+  test("check new schema after modifying schema through alter table queries when long_string_column is present") {
+    sql(
+      s"""
+         | CREATE TABLE if not exists $longStringTable(
+         | id INT, name STRING, description STRING, address STRING, note STRING
+         | ) STORED AS carbondata
+         | TBLPROPERTIES('sort_columns'='id,name')
+         |""".
+        stripMargin)
+    sql(s"alter table long_string_table set tblproperties('long_string_columns'='address')")
+    sql(s"alter table long_string_table set tblproperties('sort_columns'='ID','sort_scope'='no_sort')")
+    sql(s"alter table long_string_table unset tblproperties('long_string_columns')")
+    val carbonTable = CarbonMetadata.getInstance().getCarbonTable(CarbonCommonConstants.DATABASE_DEFAULT_NAME,
+      "long_string_table")
+    val columns = carbonTable.getTableInfo.getFactTable.getListOfColumns.asScala.toList
+      .filter(column => column.getColumnName.equalsIgnoreCase("name"))
+    assert(columns != null && columns.size > 0 && columns.head.getColumnName.equals("name"))
+  }
   
   test("update table with long string column") {
     prepareTable()