[GitHub] [carbondata] Indhumathi27 opened a new pull request #3517: [WIP] Support creating MV datamap without giving filter columns in projection and bug fixes[

[GitBox]

Indhumathi27 opened a new pull request #3517: [WIP] Support creating MV datamap without giving filter columns in projection and bug fixes[
URL: https://github.com/apache/carbondata/pull/3517
 
 
   
   
   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.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360389519
 
 

 ##########
 File path: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/rewrite/Utils.scala
 ##########
 @@ -274,7 +274,44 @@ object Utils extends PredicateHelper {
           None
         }
 
-        derivative.getOrElse { matchable = false; avg_q }
+        if (derivative.isEmpty) {
 
 Review comment:
   please add comment here

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360390631
 
 

 ##########
 File path: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/MVCreateTestCase.scala
 ##########
 @@ -1345,7 +1346,31 @@ class MVCreateTestCase extends QueryTest with BeforeAndAfterAll {
     FileFactory.deleteAllFilesOfDir(new File(newPath))
   }
 
-
+  test("test join query with & without filter columns in projection") {
+    sql("drop table if exists t1")
+    sql("drop table if exists t2")
+    sql("drop datamap if exists mv1")
+    sql("drop datamap if exists mv2")
+    sql("create table t1(userId string,score int) stored by 'carbondata'")
+    sql("create table t2(userId string,age int,sex string) stored by 'carbondata'")
+    sql("insert into t1 values(1,100),(2,500)")
+    sql("insert into t2 values(1,20,'f'),(2,30,'m')")
+    var result  = sql("select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    sql("create datamap mv1 using 'mv' as select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    var df = sql("select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    TestUtil.verifyMVDataMap(df.queryExecution.analyzed, "mv1")
+    checkAnswer(df, result)
+    intercept[ProcessMetaDataException] {
+      sql("alter table t1 drop columns(userId)")
+    }.getMessage.contains("Column name cannot be dropped because it exists in mv datamap: mv1")
+    sql("drop datamap if exists mv1")
+    sql("create datamap mv2 using 'mv' as select avg(t1.score),t2.age,t2.sex,sum(t1.userId),sum(t2.userId) from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
 
 Review comment:
   `sum(t1.userId),sum(t2.userId)` is strange, can you remove them

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360391048
 
 

 ##########
 File path: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala
 ##########
 @@ -159,8 +161,9 @@ object MVHelper {
     tableProperties.put(CarbonCommonConstants.DATAMAP_NAME, dataMapSchema.getDataMapName)
     tableProperties.put(CarbonCommonConstants.PARENT_TABLES, parentTables.asScala.mkString(","))
 
-    val fieldRelationMap = mvUtil.getFieldsAndDataMapFieldsFromPlan(
-      logicalPlan, queryString, sparkSession)
+    val finalModularPlan: ModularPlan = new SQLBuilder(modularPlan).SQLizer.execute(modularPlan)
 
 Review comment:
   ```suggestion
       val finalModularPlan = new SQLBuilder(modularPlan).SQLizer.execute(modularPlan)
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

Indhumathi27 commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360630552
 
 

 ##########
 File path: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/rewrite/Utils.scala
 ##########
 @@ -274,7 +274,44 @@ object Utils extends PredicateHelper {
           None
         }
 
-        derivative.getOrElse { matchable = false; avg_q }
+        if (derivative.isEmpty) {
 
 Review comment:
   added

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

Indhumathi27 commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360630559
 
 

 ##########
 File path: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/MVCreateTestCase.scala
 ##########
 @@ -1345,7 +1346,31 @@ class MVCreateTestCase extends QueryTest with BeforeAndAfterAll {
     FileFactory.deleteAllFilesOfDir(new File(newPath))
   }
 
-
+  test("test join query with & without filter columns in projection") {
+    sql("drop table if exists t1")
+    sql("drop table if exists t2")
+    sql("drop datamap if exists mv1")
+    sql("drop datamap if exists mv2")
+    sql("create table t1(userId string,score int) stored by 'carbondata'")
+    sql("create table t2(userId string,age int,sex string) stored by 'carbondata'")
+    sql("insert into t1 values(1,100),(2,500)")
+    sql("insert into t2 values(1,20,'f'),(2,30,'m')")
+    var result  = sql("select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    sql("create datamap mv1 using 'mv' as select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    var df = sql("select avg(t1.score),t2.age,t2.sex from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
+    TestUtil.verifyMVDataMap(df.queryExecution.analyzed, "mv1")
+    checkAnswer(df, result)
+    intercept[ProcessMetaDataException] {
+      sql("alter table t1 drop columns(userId)")
+    }.getMessage.contains("Column name cannot be dropped because it exists in mv datamap: mv1")
+    sql("drop datamap if exists mv1")
+    sql("create datamap mv2 using 'mv' as select avg(t1.score),t2.age,t2.sex,sum(t1.userId),sum(t2.userId) from t1 join t2 on t1.userId=t2.userId group by t2.age,t2.sex")
 
 Review comment:
   removed

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

Indhumathi27 commented on a change in pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#discussion_r360630594
 
 

 ##########
 File path: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala
 ##########
 @@ -159,8 +161,9 @@ object MVHelper {
     tableProperties.put(CarbonCommonConstants.DATAMAP_NAME, dataMapSchema.getDataMapName)
     tableProperties.put(CarbonCommonConstants.PARENT_TABLES, parentTables.asScala.mkString(","))
 
-    val fieldRelationMap = mvUtil.getFieldsAndDataMapFieldsFromPlan(
-      logicalPlan, queryString, sparkSession)
+    val finalModularPlan: ModularPlan = new SQLBuilder(modularPlan).SQLizer.execute(modularPlan)
 
 Review comment:
   changed

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#issuecomment-568157948
 
 
   Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1232/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#issuecomment-568161708
 
 
   Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1242/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#issuecomment-568166397
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1251/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on issue #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517#issuecomment-568182420
 
 
   LGTM

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

asfgit closed pull request #3517: [CARBONDATA-3624] Support creating MV datamap without giving filter columns in projection and bug fixes
URL: https://github.com/apache/carbondata/pull/3517
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services