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 <[email protected]>
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()