[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996310 ## File path: docs/mv-guide.md ## @@ -1,3 +1,4 @@ + Review comment: remove this empty line change ## File path: docs/mv-guide.md ## @@ -241,6 +242,11 @@ The current information includes: | Refresh Mode | FULL / INCREMENTAL refresh to MV | | Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user | | Properties | Table properties of the materialized view | + +**NOTE**: For materialized views created +before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run +refresh mv command to add mv name to fact table property and to enable it. If refresh command Review comment: ```suggestion refresh mv command to add mv name to fact table's table properties and to enable it. If refresh command ``` 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: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996744 ## File path: docs/mv-guide.md ## @@ -241,6 +242,11 @@ The current information includes: | Refresh Mode | FULL / INCREMENTAL refresh to MV | | Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user | | Properties | Table properties of the materialized view | + +**NOTE**: For materialized views created +before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run +refresh mv command to add mv name to fact table property and to enable it. If refresh command Review comment: ```suggestion refresh mv command to add mv name to fact table's table properties and to enable it. If refresh command ``` 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: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996310 ## File path: docs/mv-guide.md ## @@ -1,3 +1,4 @@ + Review comment: remove this empty line change 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: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#discussion_r568501151 ## File path: docs/mv-guide.md ## @@ -241,6 +242,10 @@ The current information includes: | Refresh Mode | FULL / INCREMENTAL refresh to MV | | Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user | | Properties | Table properties of the materialized view | + +**NOTE**: For materialized views created +before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run +refresh mv command to add mv name to fact table property and to enable it. Review comment: also please add what happens if user doesn't use refresh 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: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#discussion_r568501151 ## File path: docs/mv-guide.md ## @@ -241,6 +242,10 @@ The current information includes: | Refresh Mode | FULL / INCREMENTAL refresh to MV | | Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user | | Properties | Table properties of the materialized view | + +**NOTE**: For materialized views created +before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run +refresh mv command to add mv name to fact table property and to enable it. Review comment: also please add what happens if user doesn't use refresh 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: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r565042150
##
File path:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##
@@ -1222,6 +1222,11 @@ private CarbonCommonConstants() {
public static final String CARBON_ENABLE_MV_DEFAULT = "true";
+ /**
+ * Related mv table's map for a fact table
+ */
+ public static final String RELATED_MV_TABLES_MAP = "relatedMVTablesMap";
Review comment:
since you are always making lower case in usage, define in lowercase
itself and avoid converting
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##
@@ -38,7 +38,8 @@ case class CarbonDropMVCommand(
databaseNameOption: Option[String],
name: String,
ifExistsSet: Boolean,
-forceDrop: Boolean = false)
+forceDrop: Boolean = false,
+lockAcquiredOnFactTable: String = null)
Review comment:
```suggestion
isLockAcquiredOnFactTable: String = null)
```
please change other places also
##
File path:
integration/spark/src/main/scala/org/apache/carbondata/view/MVHelper.scala
##
@@ -372,6 +380,89 @@ object MVHelper {
updatedName
}
+ /**
+ * Add or modify the MV database and table name to fact table's related mv
tables Map
+ */
+ def addOrModifyMVTablesMap(session: SparkSession,
Review comment:
please add the scenarios where this method is called and how handled
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:
us...@infra.apache.org
