You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by chenerlu <gi...@git.apache.org> on 2017/09/04 12:57:14 UTC
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
GitHub user chenerlu opened a pull request:
https://github.com/apache/carbondata/pull/1321
[CARBONDATA-1438] Unify the sort column and sort scope in create table command
Background:
In order to improve the ease of usage for users, unify the sort column and sort scope in create table command.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chenerlu/incubator-carbondata pr-1438
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1321.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1321
----
commit c20caf973a70e6c6dc94d3cb26bf22404646e8eb
Author: chenerlu <ch...@huawei.com>
Date: 2017-09-04T12:54:55Z
Unify the sort column and sort scope in create table command
----
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r139300959
--- Diff: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -433,10 +442,23 @@ 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) {
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT
+ } else {
+ tableProperties.getOrDefault("sort_scope",
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT)
+ }
+
--- End diff --
Don't hardcode the sort scope here, you should get from carbon properties. Please use the code like below.
```
val sortScopeDefault = CarbonProperties.getInstance().
getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SORT_SCOPE,
CarbonProperties.getInstance().getProperty(CarbonCommonConstants.LOAD_SORT_SCOPE,
CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT))
val sortScope = if (null == tableProperties) {
sortScopeDefault
} else {
tableProperties.getOrDefault("sort_scope", sortScopeDefault)
}
```
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/682/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/818/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
please retest this
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/675/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/12/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r137441759
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -639,6 +639,23 @@ case class LoadTable(
val carbonProperty: CarbonProperties = CarbonProperties.getInstance()
carbonProperty.addProperty("zookeeper.enable.lock", "false")
val optionsFinal = getFinalOptions(carbonProperty)
+ val tableProperties = relation.tableMeta.carbonTable.getTableInfo
+ .getFactTable.getTableProperties
+
+ optionsFinal.put("sort_scope", tableProperties.getOrDefault("sort_scope",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SORT_SCOPE,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_SORT_SCOPE,
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT))))
+
+ optionsFinal.put("batch_sort_size_inmb", tableProperties.getOrDefault("batch_sort_size_inmb",
--- End diff --
Yes, this is only needed for batch sort, but I think if users specify this parameter in global sort, it is better to ignore it.
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r138832339
--- 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 --
If tableProperties is null, it need to use default sort scope, right?
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
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
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/812/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/689/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r136828909
--- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestGlobalSortDataLoad.scala ---
@@ -109,23 +118,22 @@ class TestGlobalSortDataLoad extends QueryTest with BeforeAndAfterEach with Befo
}
// ----------------------------------- Configuration Validity -----------------------------------
- test("Don't support GLOBAL_SORT on partitioned table") {
+ ignore("Don't support GLOBAL_SORT on partitioned table") {
--- End diff --
why ignore this testcase?
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder1/22/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/826/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/128/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/184/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/828/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r139301875
--- Diff: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -433,10 +442,23 @@ 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) {
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT
+ } else {
+ tableProperties.getOrDefault("sort_scope",
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT)
+ }
+
--- End diff --
ok, already update.
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/195/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r136829860
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -556,21 +556,21 @@ case class LoadTable(
carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_DATEFORMAT,
CarbonLoadOptionConstants.CARBON_OPTIONS_DATEFORMAT_DEFAULT)))
- optionsFinal.put("global_sort_partitions", options.getOrElse("global_sort_partitions",
- carbonProperty
- .getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_GLOBAL_SORT_PARTITIONS, null)))
+// optionsFinal.put("global_sort_partitions", options.getOrElse("global_sort_partitions",
--- End diff --
delete unused code
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/185/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/61/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder1/21/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder1/20/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
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 ""
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r137441815
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -639,6 +639,23 @@ case class LoadTable(
val carbonProperty: CarbonProperties = CarbonProperties.getInstance()
carbonProperty.addProperty("zookeeper.enable.lock", "false")
val optionsFinal = getFinalOptions(carbonProperty)
+ val tableProperties = relation.tableMeta.carbonTable.getTableInfo
+ .getFactTable.getTableProperties
+
+ optionsFinal.put("sort_scope", tableProperties.getOrDefault("sort_scope",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SORT_SCOPE,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_SORT_SCOPE,
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT))))
+
+ optionsFinal.put("batch_sort_size_inmb", tableProperties.getOrDefault("batch_sort_size_inmb",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_BATCH_SORT_SIZE_INMB,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_BATCH_SORT_SIZE_INMB,
+ CarbonCommonConstants.LOAD_BATCH_SORT_SIZE_INMB_DEFAULT))))
+
+ optionsFinal.put("global_sort_partitions", tableProperties.getOrDefault("global_sort_partitions",
--- End diff --
Same as batch sort size I think.
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
please retest it
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/740/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
please retest this
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/122/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/676/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/664/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/811/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/72/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r138830655
--- 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 --
null != sortScope can be removed, it is checked inside CarbonUtil.isValidSortOption
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/74/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/59/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/117/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/829/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/196/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
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.
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by gvramana <gi...@git.apache.org>.
Github user gvramana commented on the issue:
https://github.com/apache/carbondata/pull/1321
Unification for simplicity is ok. Ex: SORT_SCOPE provided in create table is default SORT_SCOPE for that table.
However we have scenarios where SORT_SCOPE needs to be overridden in load command for data load speed/streaming cases. So that compaction will bring back the scope to table level default value.
Option 1:
We can decide if delete load time SORT_SCOPE and bring it back later when it is fully supported.
Option 2:
Keep the syntax in data load as optional parameter and currently validate to allow only Valid values.
like Data load level BATCH_SORT when table level BATCH_SORT or LOCAL_SORT is valid, as current compaction can support this.
currently all other cases dataload level SORT_SCOPE should be same as table level SORT_SCOPE.
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
Github user chenerlu commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r137442307
--- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestGlobalSortDataLoad.scala ---
@@ -318,12 +329,12 @@ class TestGlobalSortDataLoad extends QueryTest with BeforeAndAfterEach with Befo
| charField CHAR(5),
| floatField FLOAT
| )
- | STORED BY 'org.apache.carbondata.format'
+ | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='GLOBAL_SORT')
""".stripMargin)
sql(
s"""
| LOAD DATA LOCAL INPATH '$path' INTO TABLE carbon_globalsort_difftypes
- | OPTIONS('SORT_SCOPE'='GLOBAL_SORT',
+ | OPTIONS(
| 'FILEHEADER'='shortField,intField,bigintField,doubleField,stringField,timestampField,decimalField,dateField,charField,floatField')
""".stripMargin)
--- End diff --
ok
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/827/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r136829721
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -639,6 +639,23 @@ case class LoadTable(
val carbonProperty: CarbonProperties = CarbonProperties.getInstance()
carbonProperty.addProperty("zookeeper.enable.lock", "false")
val optionsFinal = getFinalOptions(carbonProperty)
+ val tableProperties = relation.tableMeta.carbonTable.getTableInfo
+ .getFactTable.getTableProperties
+
+ optionsFinal.put("sort_scope", tableProperties.getOrDefault("sort_scope",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SORT_SCOPE,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_SORT_SCOPE,
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT))))
+
+ optionsFinal.put("batch_sort_size_inmb", tableProperties.getOrDefault("batch_sort_size_inmb",
--- End diff --
This is needed only if using batch sort
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder1/14/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by chenerlu <gi...@git.apache.org>.
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.
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/124/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/678/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/813/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/73/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/181/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/123/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/517/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/194/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by QACarbonData <gi...@git.apache.org>.
Github user QACarbonData commented on the issue:
https://github.com/apache/carbondata/pull/1321
test
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/187/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/1321
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/182/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r138831288
--- 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 --
suggest to change to
```
s"Passing invalid sort scope '$sortScope', valid sort scopes are 'NO_SORT', 'BATCH_SORT', 'LOCAL_SORT' or 'GLOBAL_SORT' "
```
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/63/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
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/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r136829772
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
@@ -639,6 +639,23 @@ case class LoadTable(
val carbonProperty: CarbonProperties = CarbonProperties.getInstance()
carbonProperty.addProperty("zookeeper.enable.lock", "false")
val optionsFinal = getFinalOptions(carbonProperty)
+ val tableProperties = relation.tableMeta.carbonTable.getTableInfo
+ .getFactTable.getTableProperties
+
+ optionsFinal.put("sort_scope", tableProperties.getOrDefault("sort_scope",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SORT_SCOPE,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_SORT_SCOPE,
+ CarbonCommonConstants.LOAD_SORT_SCOPE_DEFAULT))))
+
+ optionsFinal.put("batch_sort_size_inmb", tableProperties.getOrDefault("batch_sort_size_inmb",
+ carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_BATCH_SORT_SIZE_INMB,
+ carbonProperty.getProperty(CarbonCommonConstants.LOAD_BATCH_SORT_SIZE_INMB,
+ CarbonCommonConstants.LOAD_BATCH_SORT_SIZE_INMB_DEFAULT))))
+
+ optionsFinal.put("global_sort_partitions", tableProperties.getOrDefault("global_sort_partitions",
--- End diff --
This is needed only if using global sort
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1321
LGTM
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/197/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/815/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:
https://github.com/apache/carbondata/pull/1321
retest this please
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r138830621
--- 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)) {
--- End diff --
`null != sortScope` can be removed, it is checked inside `CarbonUtil.isValidSortOption`
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1321
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/816/
---
[GitHub] carbondata pull request #1321: [CARBONDATA-1438] Unify the sort column and s...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1321#discussion_r136830112
--- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestGlobalSortDataLoad.scala ---
@@ -318,12 +329,12 @@ class TestGlobalSortDataLoad extends QueryTest with BeforeAndAfterEach with Befo
| charField CHAR(5),
| floatField FLOAT
| )
- | STORED BY 'org.apache.carbondata.format'
+ | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='GLOBAL_SORT')
""".stripMargin)
sql(
s"""
| LOAD DATA LOCAL INPATH '$path' INTO TABLE carbon_globalsort_difftypes
- | OPTIONS('SORT_SCOPE'='GLOBAL_SORT',
+ | OPTIONS(
| 'FILEHEADER'='shortField,intField,bigintField,doubleField,stringField,timestampField,decimalField,dateField,charField,floatField')
""".stripMargin)
--- End diff --
Add a new testcase to test using SORT_SCOPE in LOAD DATA statement, it should fail
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/58/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1321
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/129/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by CarbonDataQA <gi...@git.apache.org>.
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/71/
---
[GitHub] carbondata issue #1321: [CARBONDATA-1438] Unify the sort column and sort sco...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1321
@gvramana
I suggest we consider these two requirements separately:
1. Streaming ingestion to carbon table
2. Want to override the sort scope after table is created.
For 1, I think we should probably have another table property in the create table to indicate this table is streaming ingestable. And we should have separate API instead of SQL to do the streaming ingest. Anyway, I think it is better to design after carbon 1.2 in streaming ingest branch.
For 2, I feel we should not allow this, at least for carbon 1.2, because this will open up many complexity. Not just loading, but also compaction a lot of validation and configuration to make it work for different level. We should allow this only if all combination scenario has been tested.
In one word, we should remove the SORT_SCOPE option in LOAD DATA statement for carbondata 1.2. If there are requirement for it in future, we always can add it later.
---