From commits-return-10736-archive-asf-public=cust-asf.ponee.io@carbondata.apache.org Wed May 2 10:37:42 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id D499818065D for ; Wed, 2 May 2018 10:37:41 +0200 (CEST) Received: (qmail 56115 invoked by uid 500); 2 May 2018 08:37:40 -0000 Mailing-List: contact commits-help@carbondata.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@carbondata.apache.org Delivered-To: mailing list commits@carbondata.apache.org Received: (qmail 56106 invoked by uid 99); 2 May 2018 08:37:40 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 02 May 2018 08:37:40 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 57793E09E9; Wed, 2 May 2018 08:37:40 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: kunalkapoor@apache.org To: commits@carbondata.apache.org Message-Id: <744ac8b7de6b4b89b4e1e4340674ac5d@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: carbondata git commit: [CARBONDATA-2419] sortColumns Order we are getting wrong as we set for external table is fixed Date: Wed, 2 May 2018 08:37:40 +0000 (UTC) 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 Authored: Sun Apr 29 19:38:01 2018 +0530 Committer: kunal642 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 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) {