[GitHub] [carbondata] nihal0107 opened a new pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

classic Classic list List threaded Threaded
103 messages Options
123456
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-772560841


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3660/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

Indhumathi27 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773027802


   retest this please


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r569949324



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +94,30 @@ class DropTableTest extends QueryTest with BeforeAndAfterAll {
     assert(sql("show indexes on testDrop").collect().isEmpty)
     sql("drop table if exists testDrop")
   }
+
+  test("test index drop when SI table is not deleted while main table is deleted") {
+    sql("drop database if exists test cascade")
+    sql("create database test")
+    sql("use test")
+    sql("drop table if exists testDrop")
+    sql("create table testDrop (a string, b string, c string) STORED AS carbondata")
+    sql("create index index11 on table testDrop (c) AS 'carbondata'")
+    sql("insert into testDrop values('ab', 'cd', 'ef')")
+    val indexTablePath = CarbonEnv.getCarbonTable(Some("test"),
+      "index11")(sqlContext.sparkSession).getTablePath
+    val mock: MockUp[CarbonInternalMetastore.type] = new MockUp[CarbonInternalMetastore.type]() {
+      @Mock
+      def deleteIndexSilent(carbonTableIdentifier: TableIdentifier,
+        storePath: String,
+        parentCarbonTable: CarbonTable)(sparkSession: SparkSession): Unit = {
+        throw new RuntimeException("An exception occurred while deleting SI table")
+      }
+    }
+    sql("drop table if exists testDrop")
+    mock.tearDown()
+    assert(Files.exists(Paths.get(indexTablePath)))
+    sql("drop table if exists testDrop")
+    sql("drop database if exists test cascade")

Review comment:
       can you add drop db in finally block / or in afterALL, to make sure database is dropped in case of failure




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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r569951500



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +94,30 @@ class DropTableTest extends QueryTest with BeforeAndAfterAll {
     assert(sql("show indexes on testDrop").collect().isEmpty)
     sql("drop table if exists testDrop")
   }
+
+  test("test index drop when SI table is not deleted while main table is deleted") {
+    sql("drop database if exists test cascade")
+    sql("create database test")
+    sql("use test")
+    sql("drop table if exists testDrop")
+    sql("create table testDrop (a string, b string, c string) STORED AS carbondata")
+    sql("create index index11 on table testDrop (c) AS 'carbondata'")
+    sql("insert into testDrop values('ab', 'cd', 'ef')")
+    val indexTablePath = CarbonEnv.getCarbonTable(Some("test"),
+      "index11")(sqlContext.sparkSession).getTablePath
+    val mock: MockUp[CarbonInternalMetastore.type] = new MockUp[CarbonInternalMetastore.type]() {
+      @Mock
+      def deleteIndexSilent(carbonTableIdentifier: TableIdentifier,
+        storePath: String,
+        parentCarbonTable: CarbonTable)(sparkSession: SparkSession): Unit = {
+        throw new RuntimeException("An exception occurred while deleting SI table")
+      }
+    }
+    sql("drop table if exists testDrop")
+    mock.tearDown()
+    assert(Files.exists(Paths.get(indexTablePath)))
+    sql("drop table if exists testDrop")
+    sql("drop database if exists test cascade")

Review comment:
       added in finally block




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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773073732


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5425/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773075985


   retest this please


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773076642


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3665/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773123973


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3667/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773125323


   retest this please


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773183668


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5430/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773185285


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3670/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773185819


   retest this please


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773242373


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3672/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773243978


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5432/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773245019


   retest this please


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773295921


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3674/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773306381


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5434/
   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

Indhumathi27 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773350323


   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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] asfgit closed pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

asfgit closed pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071


   


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773073732






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


123456