[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1321#discussion_r138832604
 
    --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -874,6 +884,8 @@ private[sql] case class DescribeCommandFormatted(
         results ++= Seq(("CARBON Store Path: ", relation.tableMeta.storePath, ""))
         val carbonTable = relation.tableMeta.carbonTable
         results ++= Seq(("Table Block Size : ", carbonTable.getBlockSizeInMB + " MB", ""))
    +    results ++= Seq(("SORT_SCOPE", carbonTable.getTableInfo.getFactTable
    +      .getTableProperties.getOrDefault("sort_scope", ""), ""))
    --- End diff --
   
    Should give a default sort_scope instead of ""


---

[qiuchenjian-2]

Github user chenerlu commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1321#discussion_r138852185
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -344,6 +345,13 @@ case class CreateTable(cm: TableModel, createDSTable: Boolean = true) extends Ru
     
         val tableInfo: TableInfo = TableNewProcessor(cm)
     
    +    // Add validation for sort scope when create table
    +    val sortScope = tableInfo.getFactTable.getTableProperties.get("sort_scope")
    +    if (null != sortScope && !CarbonUtil.isValidSortOption(sortScope)) {
    --- End diff --
   
    ok, I will provide default value instead of null, then I will remove this


---

[qiuchenjian-2]

Github user chenerlu commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1321#discussion_r138852303
 
    --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -172,6 +173,13 @@ case class CreateTable(cm: TableModel) extends RunnableCommand {
     
         val tableInfo: TableInfo = TableNewProcessor(cm)
     
    +    // Add validation for sort scope when create table
    +    val sortScope = tableInfo.getFactTable.getTableProperties.get("sort_scope")
    +    if (null != sortScope && !CarbonUtil.isValidSortOption(sortScope)) {
    +      throw new InvalidConfigurationException("The sort scope " + sortScope
    --- End diff --
   
    For this, I just keep same with error message which is already exists.


---

[qiuchenjian-2]

Github user chenerlu commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1321#discussion_r138853538
 
    --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -432,7 +440,9 @@ case class LoadTable(
           val dateFormat = options.getOrElse("dateformat", null)
           ValidateUtil.validateDateFormat(dateFormat, table, tableName)
           val maxColumns = options.getOrElse("maxcolumns", null)
    -      val sortScope = options.getOrElse("sort_scope", null)
    +
    +      val tableProperties = table.getTableInfo.getFactTable.getTableProperties
    +      val sortScope = if (null == tableProperties) null else tableProperties.get("sort_scope")
    --- End diff --
   
    I use CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT as its default value.


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/761/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/770/



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/145/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/19/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/774/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/147/



---

[qiuchenjian-2]

Github user QACarbonData commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    test


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/778/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/154/



---

[qiuchenjian-2]

Github user chenerlu commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/783/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/155/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/784/



---

[qiuchenjian-2]

Github user chenerlu commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1321
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/57/



---