[CARBONDATA-3607] Remove batch_sort feature #3499

jackylk · 2019-12-04T15:53:35Z

Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:

Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

CarbonDataQA1 · 2019-12-04T16:11:37Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1092/

CarbonDataQA1 · 2019-12-04T16:12:19Z

Build Failed with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1083/

CarbonDataQA1 · 2019-12-04T16:14:06Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1102/

lamberken · 2019-12-04T17:35:13Z

+1 👍 refactor carbondata project.

CarbonDataQA1 · 2019-12-04T17:39:18Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1086/

CarbonDataQA1 · 2019-12-04T18:44:36Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1105/

CarbonDataQA1 · 2019-12-04T18:45:58Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1095/

CarbonDataQA1 · 2019-12-05T15:47:42Z

Build Failed with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1095/

CarbonDataQA1 · 2019-12-05T15:49:32Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1114/

CarbonDataQA1 · 2019-12-05T15:50:03Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1104/

CarbonDataQA1 · 2019-12-05T15:58:59Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1092/

CarbonDataQA1 · 2019-12-06T08:53:18Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1112/

CarbonDataQA1 · 2019-12-06T09:26:27Z

Build Failed with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1104/

CarbonDataQA1 · 2019-12-06T09:28:47Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1123/

CarbonDataQA1 · 2019-12-06T14:30:40Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1111/

CarbonDataQA1 · 2019-12-06T15:49:23Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1130/

CarbonDataQA1 · 2019-12-06T16:12:52Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1120/

zzcclp · 2019-12-09T14:48:59Z

LGTM, thanks for your contribution.

zzcclp · 2019-12-09T14:53:38Z

After merged #3498 , there are some conflicts, please fix.

ajantha-bhat · 2019-12-09T15:01:09Z

...t/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableWithSortScope.scala

@@ -68,10 +68,10 @@ class TestCreateTableWithSortScope extends QueryTest with BeforeAndAfterAll {
         | stringField STRING
         | )
         | STORED BY 'carbondata'
-         | TBLPROPERTIES('SORT_COLUMNS'='stringField', 'SORT_SCOPE'='BATCH_SORT')
+         | TBLPROPERTIES('SORT_COLUMNS'='stringField', 'SORT_SCOPE'='local_sort')


already this suite has test case local sort, can remove this batch sort test case

please remove test case about table 'tableWithBatchSort':
line 32: sql("DROP TABLE IF EXISTS tableWithBatchSort")
and the code from line 64.

ajantha-bhat · 2019-12-09T15:05:59Z

...test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/BatchSortLoad1TestCase.scala

@@ -1,292 +0,0 @@
-


SDVSuites.scala is still pointing to this file. Need to modify that file also ?

yeah, it also needs to remove 'PartitionTestCase'.

ajantha-bhat · 2019-12-09T15:08:42Z

...pache/carbondata/spark/testsuite/standardpartition/StandardPartitionGlobalSortTestCase.scala

@@ -939,7 +939,7 @@ class StandardPartitionGlobalSortTestCase extends QueryTest with BeforeAndAfterA
    assert(exMessage.getMessage.contains("day is not a valid partition column in table default.partitionnocolumn"))
  }

-  ignore("data loading with default partition in static partition table with batchsort") {
+  ignore("data loading with default partition in static partition table with local_sort") {
    sql("DROP TABLE IF EXISTS partitiondefaultbatchsort")
    sql(
      """


Table name is still a batch sort, better to rename for readability.
looked for "batchsort" in the code. only this file and TestCreateTableWithSortScope has this problem

CarbonDataQA1 · 2019-12-09T19:52:59Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1133/

CarbonDataQA1 · 2019-12-09T21:05:28Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1142/

CarbonDataQA1 · 2019-12-09T21:14:59Z

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1152/

zzcclp · 2019-12-10T01:34:10Z

...-test/src/test/scala/org/apache/carbondata/spark/testsuite/sortcolumns/TestSortColumns.scala

@@ -447,7 +447,7 @@ class TestSortColumns extends QueryTest with BeforeAndAfterAll {
  def setLoadingProperties(offheap: String, unsafe: String, useBatch: String): Unit = {
    CarbonProperties.getInstance().addProperty(CarbonCommonConstants.ENABLE_OFFHEAP_SORT, offheap)
    if (useBatch.equalsIgnoreCase("true")) {


use unsafe.equalsIgnoreCase("true") instead of useBatch.equalsIgnoreCase("true")

zzcclp · 2019-12-10T01:35:54Z

...pache/carbondata/spark/testsuite/standardpartition/StandardPartitionGlobalSortTestCase.scala

@@ -939,7 +939,7 @@ class StandardPartitionGlobalSortTestCase extends QueryTest with BeforeAndAfterA
    assert(exMessage.getMessage.contains("day is not a valid partition column in table default.partitionnocolumn"))
  }

-  ignore("data loading with default partition in static partition table with batchsort") {
+  ignore("data loading with default partition in static partition table with local_sort") {
    sql("DROP TABLE IF EXISTS partitiondefaultbatchsort")


rename 'partitiondefaultbatchsort' to 'partitiondefaultlocalsort'

zzcclp · 2019-12-10T01:36:17Z

...pache/carbondata/spark/testsuite/standardpartition/StandardPartitionGlobalSortTestCase.scala

@@ -948,7 +948,7 @@ class StandardPartitionGlobalSortTestCase extends QueryTest with BeforeAndAfterA
        |  projectcode int, projectenddate Timestamp,attendance int,
        |  utilization int, doj Timestamp, empname String)
        | PARTITIONED BY (projectjoindate Timestamp, salary decimal)
-        | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='BATCH_SORT')
+        | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='local_sort')
      """.stripMargin)
    sql(s"""LOAD DATA local inpath '$resourcesPath/data.csv' INTO TABLE partitiondefaultbatchsort OPTIONS('DELIMITER'= ',', 'QUOTECHAR'= '"')""")


rename 'partitiondefaultbatchsort' to 'partitiondefaultlocalsort'

zzcclp · 2019-12-10T01:36:27Z

...pache/carbondata/spark/testsuite/standardpartition/StandardPartitionGlobalSortTestCase.scala

@@ -948,7 +948,7 @@ class StandardPartitionGlobalSortTestCase extends QueryTest with BeforeAndAfterA
        |  projectcode int, projectenddate Timestamp,attendance int,
        |  utilization int, doj Timestamp, empname String)
        | PARTITIONED BY (projectjoindate Timestamp, salary decimal)
-        | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='BATCH_SORT')
+        | STORED BY 'org.apache.carbondata.format' TBLPROPERTIES('SORT_SCOPE'='local_sort')
      """.stripMargin)
    sql(s"""LOAD DATA local inpath '$resourcesPath/data.csv' INTO TABLE partitiondefaultbatchsort OPTIONS('DELIMITER'= ',', 'QUOTECHAR'= '"')""")
    checkAnswer(sql("select count(*) from partitiondefaultbatchsort"), Seq(Row(10)))


rename 'partitiondefaultbatchsort' to 'partitiondefaultlocalsort'

CarbonDataQA1 · 2019-12-12T07:12:06Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1160/

CarbonDataQA1 · 2019-12-12T07:17:58Z

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1178/

CarbonDataQA1 · 2019-12-12T07:29:01Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1169/

zzcclp · 2019-12-12T07:40:46Z

LGTM

jackylk closed this Dec 5, 2019

jackylk reopened this Dec 5, 2019

jackylk force-pushed the remove_batch_sort branch from c7d0ef5 to 7d764a8 Compare December 5, 2019 15:20

ajantha-bhat reviewed Dec 9, 2019

View reviewed changes

jackylk force-pushed the remove_batch_sort branch from 5a2a6c6 to 82c371a Compare December 9, 2019 19:30

zzcclp reviewed Dec 10, 2019

View reviewed changes

remove batch sort option

2e5f9e5

jackylk force-pushed the remove_batch_sort branch from 82c371a to 2e5f9e5 Compare December 12, 2019 05:41

asfgit closed this in 2c0a1e0 Dec 12, 2019

[CARBONDATA-3607] Remove batch_sort feature #3499

[CARBONDATA-3607] Remove batch_sort feature #3499

Conversation

jackylk commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

lamberken commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

CarbonDataQA1 commented Dec 4, 2019

CarbonDataQA1 commented Dec 5, 2019

CarbonDataQA1 commented Dec 5, 2019

CarbonDataQA1 commented Dec 5, 2019

CarbonDataQA1 commented Dec 5, 2019

CarbonDataQA1 commented Dec 6, 2019

CarbonDataQA1 commented Dec 6, 2019

CarbonDataQA1 commented Dec 6, 2019

CarbonDataQA1 commented Dec 6, 2019

CarbonDataQA1 commented Dec 6, 2019

CarbonDataQA1 commented Dec 6, 2019

zzcclp commented Dec 9, 2019

zzcclp commented Dec 9, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA1 commented Dec 9, 2019

CarbonDataQA1 commented Dec 9, 2019

CarbonDataQA1 commented Dec 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA1 commented Dec 12, 2019

CarbonDataQA1 commented Dec 12, 2019

CarbonDataQA1 commented Dec 12, 2019

zzcclp commented Dec 12, 2019

zzcclp commented Dec 9, 2019 •

edited