You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ku...@apache.org on 2018/05/02 08:37:40 UTC

carbondata git commit: [CARBONDATA-2419] sortColumns Order we are getting wrong as we set for external table is fixed

Repository: carbondata
Updated Branches:
  refs/heads/master 3202cf517 -> 93724ec3a


[CARBONDATA-2419] sortColumns Order we are getting wrong as we set for external table is fixed

Problem : sortColumns propert was being added during addition of column in builder.
Solution : sortColumns property is set explicitly instead of during addition of column in builder.

This closes #2249


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

Branch: refs/heads/master
Commit: 93724ec3a13bd68808ff8c8e5fc223f8eefef2b2
Parents: 3202cf5
Author: rahulforallp <ra...@knoldus.in>
Authored: Sun Apr 29 19:38:01 2018 +0530
Committer: kunal642 <ku...@gmail.com>
Committed: Wed May 2 14:06:19 2018 +0530

----------------------------------------------------------------------
 .../schema/table/TableSchemaBuilder.java        | 13 ++++----
 .../schema/table/TableSchemaBuilderSuite.java   |  7 ++--
 .../TestNonTransactionalCarbonTable.scala       | 35 +++++++++++++++++++-
 .../sdk/file/CarbonWriterBuilder.java           | 13 ++++++--
 4 files changed, 57 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/carbondata/blob/93724ec3/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java b/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java
index 2c29be0..731fea8 100644
--- a/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java
+++ b/core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java
@@ -103,7 +103,11 @@ public class TableSchemaBuilder {
     return schema;
   }
 
-  public TableSchemaBuilder addColumn(StructField field, boolean isSortColumn) {
+  public void setSortColumns(List<ColumnSchema> sortColumns) {
+    this.sortColumns = sortColumns;
+  }
+
+  public ColumnSchema addColumn(StructField field, boolean isSortColumn) {
     Objects.requireNonNull(field);
     checkRepeatColumnName(field);
     ColumnSchema newColumn = new ColumnSchema();
@@ -138,10 +142,7 @@ public class TableSchemaBuilder {
       newColumn.setPrecision(decimalType.getPrecision());
       newColumn.setScale(decimalType.getScale());
     }
-    if (isSortColumn) {
-      sortColumns.add(newColumn);
-      newColumn.setSortColumn(true);
-    } else {
+    if (!isSortColumn) {
       otherColumns.add(newColumn);
     }
     if (newColumn.isDimensionColumn()) {
@@ -156,7 +157,7 @@ public class TableSchemaBuilder {
         }
       }
     }
-    return this;
+    return newColumn;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/carbondata/blob/93724ec3/core/src/test/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilderSuite.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilderSuite.java b/core/src/test/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilderSuite.java
index 34fa920..e9dce94 100644
--- a/core/src/test/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilderSuite.java
+++ b/core/src/test/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilderSuite.java
@@ -17,6 +17,7 @@
 
 package org.apache.carbondata.core.metadata.schema.table;
 
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.carbondata.core.metadata.datatype.DataTypes;
@@ -37,7 +38,8 @@ public class TableSchemaBuilderSuite {
   @Test
   public void testBuilder() {
     TableSchemaBuilder builder = TableSchema.builder();
-    builder.addColumn(new StructField("a", DataTypes.INT), true);
+    ColumnSchema columnSchema = builder.addColumn(new StructField("a", DataTypes.INT), true);
+    builder.setSortColumns(Arrays.asList(columnSchema));
     builder.addColumn(new StructField("b", DataTypes.DOUBLE), false);
     TableSchema schema = builder.build();
     Assert.assertEquals(2, schema.getListOfColumns().size());
@@ -49,7 +51,8 @@ public class TableSchemaBuilderSuite {
   @Test(expected = IllegalArgumentException.class)
   public void testRepeatedColumn() {
     TableSchemaBuilder builder = TableSchema.builder();
-    builder.addColumn(new StructField("a", DataTypes.INT), true);
+    ColumnSchema columnSchema = builder.addColumn(new StructField("a", DataTypes.INT), true);
+    builder.setSortColumns(Arrays.asList(columnSchema));
     builder.addColumn(new StructField("a", DataTypes.DOUBLE), false);
     TableSchema schema = builder.build();
   }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/93724ec3/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 a6af4a6..d8e5374 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
@@ -94,9 +94,20 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll {
     buildTestData(3, false, options)
   }
 
+  def buildTestDataWithSortColumns(): Any = {
+    FileUtils.deleteDirectory(new File(writerPath))
+    buildTestData(3, false, null, List("age", "name"))
+  }
 
-  // prepare sdk writer output
   def buildTestData(rows: Int, persistSchema: Boolean, options: util.Map[String, String]): Any = {
+    buildTestData(rows, persistSchema, options, List("name"))
+  }
+
+  // prepare sdk writer output
+  def buildTestData(rows: Int,
+      persistSchema: Boolean,
+      options: util.Map[String, String],
+      sortColumns: List[String]): Any = {
     val schema = new StringBuilder()
       .append("[ \n")
       .append("   {\"name\":\"string\"},\n")
@@ -111,6 +122,7 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll {
         if (persistSchema) {
           builder.persistSchemaFile(true)
           builder.withSchema(Schema.parseJson(schema))
+            .sortBy(sortColumns.toArray)
             .outputPath(writerPath)
             .isTransactionalTable(false)
             .uniqueIdentifier(System.currentTimeMillis)
@@ -119,12 +131,14 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll {
           if (options != null) {
             builder.withSchema(Schema.parseJson(schema)).outputPath(writerPath)
               .isTransactionalTable(false)
+              .sortBy(sortColumns.toArray)
               .uniqueIdentifier(
                 System.currentTimeMillis).withBlockSize(2).withLoadOptions(options)
               .buildWriterForCSVInput()
           } else {
             builder.withSchema(Schema.parseJson(schema)).outputPath(writerPath)
               .isTransactionalTable(false)
+              .sortBy(sortColumns.toArray)
               .uniqueIdentifier(
                 System.currentTimeMillis).withBlockSize(2)
               .buildWriterForCSVInput()
@@ -179,6 +193,25 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll {
     sql("DROP TABLE IF EXISTS sdkOutputTable")
   }
 
+  test("test create external table with sort columns") {
+    buildTestDataWithSortColumns()
+    assert(new File(writerPath).exists())
+    sql("DROP TABLE IF EXISTS sdkOutputTable")
+
+    // with partition
+    sql(
+      s"""CREATE EXTERNAL TABLE sdkOutputTable(name string) PARTITIONED BY (age int) STORED BY
+         |'carbondata' LOCATION
+         |'$writerPath' """.stripMargin)
+
+    checkExistence(sql("describe formatted sdkOutputTable"), true, "age,name")
+
+    sql("DROP TABLE sdkOutputTable")
+    // drop table should not delete the files
+    assert(new File(writerPath).exists())
+    cleanTestData()
+  }
+
   test("test create External Table with Schema with partition, should ignore schema and partition")
   {
     buildTestDataSingleFile()

http://git-wip-us.apache.org/repos/asf/carbondata/blob/93724ec3/store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
----------------------------------------------------------------------
diff --git a/store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java b/store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
index 3d5c77c..68bc3ab 100644
--- a/store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
+++ b/store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
@@ -40,6 +40,7 @@ import org.apache.carbondata.core.metadata.schema.table.CarbonTable;
 import org.apache.carbondata.core.metadata.schema.table.TableInfo;
 import org.apache.carbondata.core.metadata.schema.table.TableSchema;
 import org.apache.carbondata.core.metadata.schema.table.TableSchemaBuilder;
+import org.apache.carbondata.core.metadata.schema.table.column.ColumnSchema;
 import org.apache.carbondata.core.util.path.CarbonTablePath;
 import org.apache.carbondata.core.writer.ThriftWriter;
 import org.apache.carbondata.processing.loading.model.CarbonLoadModel;
@@ -378,6 +379,7 @@ public class CarbonWriterBuilder {
     } else {
       sortColumnsList = Arrays.asList(sortColumns);
     }
+    ColumnSchema[] sortColumnsSchemaList = new ColumnSchema[sortColumnsList.size()];
     for (Field field : schema.getFields()) {
       if (null != field) {
         if (field.getChildren() != null && field.getChildren().size() > 0) {
@@ -391,11 +393,18 @@ public class CarbonWriterBuilder {
           DataType complexType = DataTypes.createStructType(structFieldsArray);
           tableSchemaBuilder.addColumn(new StructField(field.getFieldName(), complexType), false);
         } else {
-          tableSchemaBuilder.addColumn(new StructField(field.getFieldName(), field.getDataType()),
-              sortColumnsList.contains(field.getFieldName()));
+          int isSortColumn = sortColumnsList.indexOf(field.getFieldName());
+          ColumnSchema columnSchema = tableSchemaBuilder
+              .addColumn(new StructField(field.getFieldName(), field.getDataType()),
+                  isSortColumn > -1);
+          if (isSortColumn > -1) {
+            columnSchema.setSortColumn(true);
+            sortColumnsSchemaList[isSortColumn] = columnSchema;
+          }
         }
       }
     }
+    tableSchemaBuilder.setSortColumns(Arrays.asList(sortColumnsSchemaList));
     String tableName;
     String dbName;
     if (isTransactionalTable) {