[GitHub] carbondata pull request #1857: [WIP][CARBONDATA-2073][CARBONDATA-1516][Tests...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178035059
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggCreateCommand.scala ---
    @@ -55,12 +57,33 @@ class TestPreAggCreateCommand extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test pre agg create table 2") {
    +    dropDataMaps("PreAggMain", "preagg2")
         sql("create datamap preagg2 on table PreAggMain using 'preaggregate' as select a as a1,sum(b) as udfsum from PreAggMain group by a")
         checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), true, "preaggmain_a")
         checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), true, "preaggmain_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), false, "preaggmain_a1")
         sql("drop datamap preagg2 on table PreAggMain")
       }
     
    +  test("test pre agg create table 3") {
    +    dropDataMaps("PreAggMain", "preagg3")
    +    sql("create datamap preagg3 on table PreAggMain using 'preaggregate' as select a,sum(b) as sum from PreAggMain group by a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), true, "preaggmain_a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), true, "preaggmain_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), false, "preaggmain_sum")
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178035445
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggCreateCommand.scala ---
    @@ -55,12 +57,33 @@ class TestPreAggCreateCommand extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test pre agg create table 2") {
    +    dropDataMaps("PreAggMain", "preagg2")
         sql("create datamap preagg2 on table PreAggMain using 'preaggregate' as select a as a1,sum(b) as udfsum from PreAggMain group by a")
         checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), true, "preaggmain_a")
         checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), true, "preaggmain_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg2"), false, "preaggmain_a1")
         sql("drop datamap preagg2 on table PreAggMain")
       }
     
    +  test("test pre agg create table 3") {
    +    dropDataMaps("PreAggMain", "preagg3")
    +    sql("create datamap preagg3 on table PreAggMain using 'preaggregate' as select a,sum(b) as sum from PreAggMain group by a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), true, "preaggmain_a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), true, "preaggmain_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg3"), false, "preaggmain_sum")
    +    sql("drop datamap preagg3 on table PreAggMain")
    +  }
    +
    +  test("test pre agg create table 4") {
    +    dropDataMaps("PreAggMain", "preagg4")
    +    sql("create datamap preagg4 on table PreAggMain using 'preaggregate' as select a as a1,sum(b) as sum from PreAggMain group by a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg4"), true, "preaggmain_a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg4"), true, "preaggmain_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg4"), false, "preaggmain_a1")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain_preagg4"), false, "preaggmain_sum")
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178035599
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggCreateCommand.scala ---
    @@ -69,10 +92,30 @@ class TestPreAggCreateCommand extends QueryTest with BeforeAndAfterAll {
         sql("drop datamap preagg11 on table PreAggMain1")
       }
     
    +  test("test pre agg create table 6") {
    +    sql("create datamap preagg12 on table PreAggMain1 using 'preaggregate' as select a as a1,sum(b) from PreAggMain1 group by a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain1_preagg12"), true, "preaggmain1_a")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain1_preagg12"), true, "preaggmain1_b_sum")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain1_preagg12"), false, "preaggmain1_a1")
    +    checkExistence(sql("DESCRIBE FORMATTED PreAggMain1_preagg12"), true, "DICTIONARY")
    +    sql("drop datamap preagg12 on table PreAggMain1")
    +  }
    +
    +  test("test pre agg create table 7") {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178036919
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateIUDSuite.scala ---
    @@ -0,0 +1,206 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.integration.spark.testsuite.preaggregate
    +
    +import org.apache.spark.sql.Row
    +import org.apache.spark.util.SparkUtil4Test
    +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.util.SparkQueryTest
    +
    +class TestPreAggregateIUDSuite extends SparkQueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
    +
    +  val testData = s"$resourcesPath/sample.csv"
    +  val p1 = CarbonProperties.getInstance()
    +    .getProperty(CarbonCommonConstants.COMPACTION_SEGMENT_LEVEL_THRESHOLD,
    +      CarbonCommonConstants.DEFAULT_SEGMENT_LEVEL_THRESHOLD)
    +
    +  override def beforeAll(): Unit = {
    +    CarbonProperties.getInstance()
    +      .addProperty(CarbonCommonConstants.ENABLE_AUTO_LOAD_MERGE, "false")
    +      .addProperty(CarbonCommonConstants.COMPACTION_SEGMENT_LEVEL_THRESHOLD,
    +        CarbonCommonConstants.DEFAULT_SEGMENT_LEVEL_THRESHOLD)
    +
    +    SparkUtil4Test.createTaskMockUp(sqlContext)
    +    sql("DROP TABLE IF EXISTS maintable")
    +  }
    +
    +  override protected def afterAll(): Unit = {
    +    CarbonProperties.getInstance()
    +      .addProperty(CarbonCommonConstants.ENABLE_AUTO_LOAD_MERGE,
    +        CarbonCommonConstants.DEFAULT_ENABLE_AUTO_LOAD_MERGE)
    +      .addProperty(CarbonCommonConstants.COMPACTION_SEGMENT_LEVEL_THRESHOLD, p1)
    +    sql("DROP TABLE IF EXISTS y ")
    +    sql("DROP TABLE IF EXISTS maintable")
    +    sql("DROP TABLE IF EXISTS maintbl")
    +    sql("DROP TABLE IF EXISTS main_table")
    +  }
    +
    +  override protected def beforeEach(): Unit = {
    +    sql("DROP TABLE IF EXISTS main_table")
    +    sql("DROP TABLE IF EXISTS segmaintable")
    +  }
    +
    +
    +  test("test pre-aggregate IUD function 1: support insert after create pre aggregate table") {
    --- End diff --
   
    insert scenario is already handled in TestPreAggregateLoad


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178037427
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateIUDSuite.scala ---
    @@ -0,0 +1,206 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.integration.spark.testsuite.preaggregate
    +
    +import org.apache.spark.sql.Row
    +import org.apache.spark.util.SparkUtil4Test
    +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.util.SparkQueryTest
    +
    +class TestPreAggregateIUDSuite extends SparkQueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
    --- End diff --
   
    insert scenario is already handled in TestPreAggregateLoad.
    Same with Update and Delete. No need to add


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178037845
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateLoad.scala ---
    @@ -415,8 +444,661 @@ test("check load and select for avg double datatype") {
         sql(s"LOAD DATA LOCAL INPATH '$testData' into table maintable")
         sql(s"LOAD DATA LOCAL INPATH '$testData' into table maintable")
         val rows = sql("select age,avg(age) from maintable group by age").collect()
    -    sql("create datamap maintbl_douoble on table maintable using 'preaggregate' as select avg(age) from maintable group by age")
    +    sql("create datamap maintbl_double on table maintable using 'preaggregate' as select avg(age) from maintable group by age")
         checkAnswer(sql("select age,avg(age) from maintable group by age"), rows)
    +    sql("drop table if exists maintable ")
    +  }
    +
    +  def testFunction(): Unit = {
    +    // check answer
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_sum"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 70), Row(4, 55)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_avg"),
    +      Seq(Row(1, 31, 1), Row(2, 27, 1), Row(3, 70, 2), Row(4, 55, 2)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_count"),
    +      Seq(Row(1, 1), Row(2, 1), Row(3, 2), Row(4, 2)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_min"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 35), Row(4, 26)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_max"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 35), Row(4, 29)))
    +
    +    testPreAggTable()
    +  }
    +
    +  def testPreAggTable(): Unit = {
    +    // check select and match or not match pre-aggregate table
    +    checkPreAggTable(sql("SELECT id, SUM(age) FROM main_table GROUP BY id"),
    --- End diff --
   
    These scenarios are already covered in TestPreAggregateTableSelection


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178039781
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateIUDSuite.scala ---
    @@ -0,0 +1,206 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.integration.spark.testsuite.preaggregate
    +
    +import org.apache.spark.sql.Row
    +import org.apache.spark.util.SparkUtil4Test
    +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.util.SparkQueryTest
    +
    +class TestPreAggregateIUDSuite extends SparkQueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
    --- End diff --
   
    ok. remove insert.
   
     Where is Update and Delete?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178040942
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateLoad.scala ---
    @@ -415,8 +444,661 @@ test("check load and select for avg double datatype") {
         sql(s"LOAD DATA LOCAL INPATH '$testData' into table maintable")
         sql(s"LOAD DATA LOCAL INPATH '$testData' into table maintable")
         val rows = sql("select age,avg(age) from maintable group by age").collect()
    -    sql("create datamap maintbl_douoble on table maintable using 'preaggregate' as select avg(age) from maintable group by age")
    +    sql("create datamap maintbl_double on table maintable using 'preaggregate' as select avg(age) from maintable group by age")
         checkAnswer(sql("select age,avg(age) from maintable group by age"), rows)
    +    sql("drop table if exists maintable ")
    +  }
    +
    +  def testFunction(): Unit = {
    +    // check answer
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_sum"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 70), Row(4, 55)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_avg"),
    +      Seq(Row(1, 31, 1), Row(2, 27, 1), Row(3, 70, 2), Row(4, 55, 2)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_count"),
    +      Seq(Row(1, 1), Row(2, 1), Row(3, 2), Row(4, 2)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_min"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 35), Row(4, 26)))
    +    checkAnswer(sql(s"SELECT * FROM main_table_preagg_max"),
    +      Seq(Row(1, 31), Row(2, 27), Row(3, 35), Row(4, 29)))
    +
    +    testPreAggTable()
    +  }
    +
    +  def testPreAggTable(): Unit = {
    +    // check select and match or not match pre-aggregate table
    +    checkPreAggTable(sql("SELECT id, SUM(age) FROM main_table GROUP BY id"),
    --- End diff --
   
    test different data type, also can remove if necessary


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4660/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3433/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1857#discussion_r178048887
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/preaggregate/TestPreAggregateIUDSuite.scala ---
    @@ -0,0 +1,206 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.integration.spark.testsuite.preaggregate
    +
    +import org.apache.spark.sql.Row
    +import org.apache.spark.util.SparkUtil4Test
    +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.util.SparkQueryTest
    +
    +class TestPreAggregateIUDSuite extends SparkQueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
    --- End diff --
   
    OK, I find. and remove the test case: TestPreAggregateIUDSuite.scala


---

[qiuchenjian-2]

Github user kunal642 commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    LGTM


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3437/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4663/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4170/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4173/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4174/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

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


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/1857
 
    retest sdv please


---