You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@carbondata.apache.org by GitBox <gi...@apache.org> on 2022/04/07 12:12:46 UTC

[GitHub] [carbondata] Indhumathi27 commented on a diff in pull request #4257: [CARBONDATA-4330] Incremental‌ ‌Dataload‌ ‌of Average aggregate in ‌MV‌‌

Indhumathi27 commented on code in PR #4257:
URL: https://github.com/apache/carbondata/pull/4257#discussion_r845058254


##########
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala:
##########
@@ -143,9 +143,24 @@ case class CarbonCreateMVCommand(
       tableIdentifier: TableIdentifier,
       viewManager: MVManagerInSpark,
       viewCatalog: MVCatalogInSpark): MVSchema = {
-    val logicalPlan = MVHelper.dropDummyFunction(
+    var logicalPlan = MVHelper.dropDummyFunction(
       MVQueryParser.getQueryPlan(queryString, session))
-      // check if mv with same query already exists
+    val relatedTables = getRelatedTables(logicalPlan)
+    val viewRefreshMode = if (checkIsQueryNeedFullRefresh(logicalPlan) ||
+                              checkIsHasNonCarbonTable(relatedTables)) {
+      MVProperty.REFRESH_MODE_FULL
+    } else {
+      MVProperty.REFRESH_MODE_INCREMENTAL
+    }
+    var modifiedQueryString = queryString
+    if (viewRefreshMode.equalsIgnoreCase(MVProperty.REFRESH_MODE_INCREMENTAL) &&
+        queryString.toLowerCase.contains(CarbonCommonConstants.AVERAGE)) {

Review Comment:
   please check from the logical plan, instead of string, as column names also can contain avg keyword



##########
integration/spark/src/main/scala/org/apache/spark/sql/parser/MVQueryParser.scala:
##########
@@ -103,6 +104,43 @@ object MVQueryParser {
       .drop(MVFunctions.DUMMY_FUNCTION)
   }
 
+  // In the mv query string, check if avg of column is present and
+  // replace it with sum and count of same column.
+  // Check for duplicates and replace column if its not present already.
+  def checkForAvgAndModifySql(sql: String): String = {
+    val (avg, sum, count) = (CarbonCommonConstants.AVERAGE,
+      CarbonCommonConstants.SUM, CarbonCommonConstants.COUNT)
+    var modifiedSql: String = sql.toLowerCase
+    if (modifiedSql.contains(avg)) {

Review Comment:
   this check can be removed, as the condition is already checked in the caller



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org