akashrn5 commented on a change in pull request #3841:
URL: https://github.com/apache/carbondata/pull/3841#discussion_r457843327
##########
File path:
integration/spark/src/test/scala/org/apache/carbondata/view/MVTest.scala
##########
@@ -178,6 +180,13 @@ class MVTest extends QueryTest with BeforeAndAfterAll {
}
}
+ test("test drop mv must fail if not exists") {
+ val ex = intercept[MalformedMVCommandException] {
+ sql("drop materialized view MV11")
Review comment:
please give meaningful MV name based on test scenario
##########
File path:
integration/spark/src/test/scala/org/apache/carbondata/view/rewrite/MVCreateTestCase.scala
##########
@@ -955,9 +955,9 @@ class MVCreateTestCase extends QueryTest with
BeforeAndAfterAll {
sql(" insert into mvtable1 select 'n4',12,12")
sql("update mvtable1 set(name) = ('updatedName')").show()
checkAnswer(sql("select count(*) from mvtable1 where name =
'updatedName'"),Seq(Row(4)))
+ sql(s"drop materialized view MV11")
Review comment:
why this file change required, please revert if not required, i cant see
any valid changes.
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVManager.java
##########
@@ -143,9 +143,9 @@ public void createSchema(String databaseName, MVSchema
viewSchema)
/**
* Drops the mv schema from storage
*
- * @param viewName index name
+ * @param viewName mv name
*/
- public void deleteSchema(String databaseName, String viewName) throws
IOException {
+ public synchronized void deleteSchema(String databaseName, String viewName)
throws IOException {
Review comment:
why we need `synchronized` here? As i can see, if two people are
dropping same, then one will be successful and other will get exception as MV
does not exists. Any special case or scenario you observed?
##########
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -91,6 +92,14 @@ case class CarbonDropMVCommand(
this.dropTableCommand = dropTableCommand
}
+ else {
Review comment:
move else block to above line and correct the formatting
----------------------------------------------------------------
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:
[email protected]