You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2021/01/07 09:48:28 UTC
[GitHub] [carbondata] nihal0107 opened a new pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
nihal0107 opened a new pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071
### Why is this PR needed?
Added UT and FT to improve coverage of SI module and also removed the dead or unused code.
### What changes were proposed in this PR?
Added UT and FT to improve coverage of SI module and also removed the dead or unused code.
### Does this PR introduce any user interface change?
- No
### Is any new testcase added?
- Yes
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r563482533
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ }
+
+ test("test show index when SI were created before the change CARBONDATA-3765") {
+ val mock = TestSecondaryIndexUtils.mockIndexInfo()
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ mock.tearDown()
+ }
+
+ test("test refresh index with different value of isIndexTableExists") {
+ setVariables("index1")
+ sql("create index index2 on table1(b) as 'bloomfilter'")
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("true"))
+ CarbonIndexUtil.addOrModifyTableProperty(parentCarbonTable, Map("indexExists" -> "false"))(
Review comment:
yeah, made the changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766553712
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766617968
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5344/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-756127667
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3528/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770768986
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3629/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567674967
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCreateIndexWithLoadAndCompaction.scala
##########
@@ -283,19 +285,32 @@ class TestCreateIndexWithLoadAndCompaction extends QueryTest with BeforeAndAfter
}
sql("ALTER TABLE table1 COMPACT 'CUSTOM' WHERE SEGMENT.ID IN (1,2,3)")
- val segments = sql("SHOW SEGMENTS FOR TABLE idx1")
- val segInfos = segments.collect().map { each =>
- ((each.toSeq) (0).toString, (each.toSeq) (1).toString)
- }
- assert(segInfos.length == 6)
+ val segInfos = checkSegmentList(6)
assert(segInfos.contains(("0", "Success")))
assert(segInfos.contains(("1", "Compacted")))
assert(segInfos.contains(("2", "Compacted")))
assert(segInfos.contains(("3", "Compacted")))
assert(segInfos.contains(("1.1", "Success")))
assert(segInfos.contains(("4", "Success")))
checkAnswer(sql("select * from table1 where c3='b2'"), Seq(Row(3, "a2", "b2")))
- sql("drop table if exists table1")
+
+ // after clean files
+ val mock = TestSecondaryIndexUtils.mockreadSegmentList()
Review comment:
We have mocked here to have some invalid segment in case of clean files for SI. Added assert for those scenario also. This testcase will cover the lines for `CleanFilesPostEventListener.cleanUpUnwantedSegmentsOfSIAndUpdateMetadata`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-762305509
@Indhumathi27 please find the comparision between master and current branch
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766567489
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5053/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770853160
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770914272
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3636/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567675597
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondaryIndex.scala
##########
@@ -577,6 +589,59 @@ class TestSIWithSecondaryIndex extends QueryTest with BeforeAndAfterAll {
assert(ex.getMessage.contains("Problem loading data while creating secondary index:"))
}
+ test("test SI with carbon.use.local.dir as true") {
+ CarbonProperties.getInstance()
Review comment:
wanted to add for `carbon.use.local.dir` as `false`. Made the changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566602925
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -327,6 +259,35 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+
+ // exception is thrown by compaction executor
+ val mock3 = TestSecondaryIndexUtils.mockCompactionExecutor()
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
+ CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+ }
+
+ test("test refresh index command when block need to be sorted") {
+ CarbonProperties.getInstance()
+ .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ createTableAndLoadData("100", 2)
+ sql("CREATE INDEX nonindexmerge_index1 on table nonindexmerge (name) AS 'carbondata'")
+ val mock = TestSecondaryIndexUtils.mockIsSortRequired();
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ mock.tearDown()
+ val df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(isFilterPushedDownToSI(df1))
+ assert(getDataFileCount("nonindexmerge_index1", "0") < 15)
+ assert(getDataFileCount("nonindexmerge_index1", "1") < 15)
Review comment:
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE property is set to "false" at beginning and end of testcase. Note that CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT value is also "false". Why set to false second time ? You might want to set to true at the end of testcase.
Same issue observed in following testcases. Please recheck for all of these cases.
`test("Verify data file merge in SI segments with sort scope as gloabl sort and" +
"CARBON_SI_SEGMENT_MERGE property is enabled")`
`test("Verify REFRESH INDEX command with sort scope as global sort")`
`test("test verify data file merge when exception occurred in rebuild segment")`
`test("test refresh index command when block need to be sorted")`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-768231136
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3602/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-772554062
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5421/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771619559
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3650/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771387700
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3640/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566602925
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -327,6 +259,35 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+
+ // exception is thrown by compaction executor
+ val mock3 = TestSecondaryIndexUtils.mockCompactionExecutor()
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
+ CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+ }
+
+ test("test refresh index command when block need to be sorted") {
+ CarbonProperties.getInstance()
+ .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ createTableAndLoadData("100", 2)
+ sql("CREATE INDEX nonindexmerge_index1 on table nonindexmerge (name) AS 'carbondata'")
+ val mock = TestSecondaryIndexUtils.mockIsSortRequired();
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ mock.tearDown()
+ val df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(isFilterPushedDownToSI(df1))
+ assert(getDataFileCount("nonindexmerge_index1", "0") < 15)
+ assert(getDataFileCount("nonindexmerge_index1", "1") < 15)
Review comment:
`CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE` property is set to `false` at beginning and end of testcase. Note that `CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT` value is also `false`. Why set to false second time ? You might want to set to true at the end of testcase.
Same issue observed in following testcases. Please recheck for all of these cases.
`test("Verify data file merge in SI segments with sort scope as gloabl sort and" +
"CARBON_SI_SEGMENT_MERGE property is enabled")`
`test("Verify REFRESH INDEX command with sort scope as global sort")`
`test("test verify data file merge when exception occurred in rebuild segment")`
`test("test refresh index command when block need to be sorted")`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770844763
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3632/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771437007
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3641/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566680396
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondaryIndex.scala
##########
@@ -577,6 +589,59 @@ class TestSIWithSecondaryIndex extends QueryTest with BeforeAndAfterAll {
assert(ex.getMessage.contains("Problem loading data while creating secondary index:"))
}
+ test("test SI with carbon.use.local.dir as true") {
+ CarbonProperties.getInstance()
Review comment:
Note default for `CARBON_LOADING_USE_YARN_LOCAL_DIR` is `true`. Why do we set to `true` again both at the begin and end of testcase?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567670208
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithColumnMetCacheAndCacheLevelProperty.scala
##########
@@ -363,5 +365,20 @@ class TestQueryWithColumnMetCacheAndCacheLevelProperty
sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
sql("drop index if exists parallel_index on parallel_index_load")
+ val mock = mockIsSchemaModified()
+ sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
+ checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
+ sql("drop index if exists parallel_index on parallel_index_load")
+ mock.tearDown()
+ }
+
+ def mockIsSchemaModified(): MockUp[TableInfo] = {
Review comment:
moved inside testcase.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] ydvpankaj99 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
ydvpankaj99 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771448246
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r569392411
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -338,8 +276,48 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
- CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
- CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+
+ // exception is thrown by compaction executor
+ val mock3: MockUp[CarbonCompactionExecutor] = new MockUp[CarbonCompactionExecutor]() {
+ @Mock
+ def processTableBlocks(configuration: Configuration, filterExpr: Expression):
+ util.Map[String, util.List[RawResultIterator]] = {
+ throw new IOException("An exception occurred while compaction executor.")
+ }
+ }
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants
Review comment:
done
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCreateIndexWithLoadAndCompaction.scala
##########
@@ -283,19 +291,35 @@ class TestCreateIndexWithLoadAndCompaction extends QueryTest with BeforeAndAfter
}
sql("ALTER TABLE table1 COMPACT 'CUSTOM' WHERE SEGMENT.ID IN (1,2,3)")
- val segments = sql("SHOW SEGMENTS FOR TABLE idx1")
- val segInfos = segments.collect().map { each =>
- ((each.toSeq) (0).toString, (each.toSeq) (1).toString)
- }
- assert(segInfos.length == 6)
+ val segInfos = checkSegmentList(6)
assert(segInfos.contains(("0", "Success")))
assert(segInfos.contains(("1", "Compacted")))
assert(segInfos.contains(("2", "Compacted")))
assert(segInfos.contains(("3", "Compacted")))
assert(segInfos.contains(("1.1", "Success")))
assert(segInfos.contains(("4", "Success")))
checkAnswer(sql("select * from table1 where c3='b2'"), Seq(Row(3, "a2", "b2")))
- sql("drop table if exists table1")
+
+ // after clean files
+ val mock = mockreadSegmentList()
+ sql("CLEAN FILES FOR TABLE table1 options('force'='true')")
+ mock.tearDown()
+ val details = SegmentStatusManager.readLoadMetadata(CarbonEnv
+ .getCarbonTable(Some("default"), "idx1")(sqlContext.sparkSession).getMetadataPath)
+ assert(SegmentStatusManager.countInvisibleSegments(details, 4) == 1)
+ checkSegmentList(4)
+ CarbonProperties.getInstance()
Review comment:
done
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-762304031
@Indhumathi27 please find the comparison with master branch and current branch
x-special/nautilus-clipboard
copy
file:///home/nihal/Pictures/after1.png
file:///home/nihal/Pictures/before2.png
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567759675
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+
+ sql("drop index if exists index1 on table1")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ setVariables("index1")
+ // delete will fail as we are giving indexTable as parentTable.
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ }
+
+ test("test show index when SI were created before the change CARBONDATA-3765") {
+ val mock = TestSecondaryIndexUtils.mockIndexInfo()
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ mock.tearDown()
+ }
+
+ test("test refresh index with different value of isIndexTableExists") {
+ setVariables("index1")
+ sql("create index index2 on table1(b) as 'bloomfilter'")
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("true"))
+ assert(CarbonIndexUtil.isIndexTableExists(parentCarbonTable).equalsIgnoreCase("true"))
+ CarbonIndexUtil.addOrModifyTableProperty(parentCarbonTable, Map("indexExists" -> "false",
+ "indextableexists" -> "false"))(sqlContext.sparkSession)
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("false"))
+ assert(CarbonIndexUtil.isIndexTableExists(parentCarbonTable).equalsIgnoreCase("false"))
+ val mock = TestSecondaryIndexUtils.mockIsIndexTableExists()
Review comment:
removed and changed as suggested.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-768810377
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566685541
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondaryIndex.scala
##########
@@ -577,6 +589,59 @@ class TestSIWithSecondaryIndex extends QueryTest with BeforeAndAfterAll {
assert(ex.getMessage.contains("Problem loading data while creating secondary index:"))
}
+ test("test SI with carbon.use.local.dir as true") {
+ CarbonProperties.getInstance()
Review comment:
When some testcase has set this value and not reverted back to default, we need to add this property again.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] ydvpankaj99 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
ydvpankaj99 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771448246
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r562601511
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,17 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
+ sql("drop table if exists testDrop")
+ mock.tearDown()
+ assert(Files.exists(Paths.get(warehouse + "/" + "index11")))
+ sql("drop table if exists testDrop")
+ sql("drop database if exists test cascade")
Review comment:
looks like this database is not created in this testclass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r563482438
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
Review comment:
this is just to check when we are passing index table as parent table then delete will fail. I have added comments and also included true condition.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566641195
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCreateIndexWithLoadAndCompaction.scala
##########
@@ -283,19 +285,32 @@ class TestCreateIndexWithLoadAndCompaction extends QueryTest with BeforeAndAfter
}
sql("ALTER TABLE table1 COMPACT 'CUSTOM' WHERE SEGMENT.ID IN (1,2,3)")
- val segments = sql("SHOW SEGMENTS FOR TABLE idx1")
- val segInfos = segments.collect().map { each =>
- ((each.toSeq) (0).toString, (each.toSeq) (1).toString)
- }
- assert(segInfos.length == 6)
+ val segInfos = checkSegmentList(6)
assert(segInfos.contains(("0", "Success")))
assert(segInfos.contains(("1", "Compacted")))
assert(segInfos.contains(("2", "Compacted")))
assert(segInfos.contains(("3", "Compacted")))
assert(segInfos.contains(("1.1", "Success")))
assert(segInfos.contains(("4", "Success")))
checkAnswer(sql("select * from table1 where c3='b2'"), Seq(Row(3, "a2", "b2")))
- sql("drop table if exists table1")
+
+ // after clean files
+ val mock = TestSecondaryIndexUtils.mockreadSegmentList()
Review comment:
why do we need to mock here? It should have cleaned compacted segments without mocking. Please check and remove mockreadSegmentList mocking.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567759908
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,16 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
Review comment:
made the changes for the nondefault database
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r562601511
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,17 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
+ sql("drop table if exists testDrop")
+ mock.tearDown()
+ assert(Files.exists(Paths.get(warehouse + "/" + "index11")))
+ sql("drop table if exists testDrop")
+ sql("drop database if exists test cascade")
Review comment:
looks like this database `test` is not created in this testclass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771610225
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5410/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566795460
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,16 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
Review comment:
I think, we should either run this testcase in a non-default db or drop index like any other normal table i.e., add `sql("drop table if exists index11")` before create index(line 97). Because, we have mocked to fail SI deletion. SI table remains in default db thereafter. Hence, if we try to run this testcase multiple times, it fails to create index with `index11` name.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766622335
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3583/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r563482082
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -288,9 +268,7 @@ class CarbonDataFileMergeTestCaseOnSI
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
}
- test("test verify data file merge when exception occurred in rebuild segment") {
- CarbonProperties.getInstance()
- .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ def createTableAndLoadData(): Unit = {
Review comment:
done
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,17 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
+ sql("drop table if exists testDrop")
+ mock.tearDown()
+ assert(Files.exists(Paths.get(warehouse + "/" + "index11")))
+ sql("drop table if exists testDrop")
+ sql("drop database if exists test cascade")
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-756057859
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3527/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766720456
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5351/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771354445
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-772416726
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3654/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766553712
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3295/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-756055862
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5287/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-766721723
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3591/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566094958
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithColumnMetCacheAndCacheLevelProperty.scala
##########
@@ -363,5 +365,20 @@ class TestQueryWithColumnMetCacheAndCacheLevelProperty
sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
sql("drop index if exists parallel_index on parallel_index_load")
+ val mock = mockIsSchemaModified()
+ sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
+ checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
+ sql("drop index if exists parallel_index on parallel_index_load")
+ mock.tearDown()
+ }
+
+ def mockIsSchemaModified(): MockUp[TableInfo] = {
Review comment:
This tableInfo mockup instance for mocking isSchemaModified method to return true is specific to a particular testcase. No need to define a separate method. How about directly setup in testcase(line 368 can have like below) ? I think, we can do same at other places also.
new MockUp[TableInfo] {
@Mock
def isSchemaModified(): Boolean = {
true
}
}
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771518388
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3646/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-772415977
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5415/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566752898
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+
+ sql("drop index if exists index1 on table1")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ setVariables("index1")
+ // delete will fail as we are giving indexTable as parentTable.
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ }
+
+ test("test show index when SI were created before the change CARBONDATA-3765") {
+ val mock = TestSecondaryIndexUtils.mockIndexInfo()
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ mock.tearDown()
+ }
+
+ test("test refresh index with different value of isIndexTableExists") {
+ setVariables("index1")
+ sql("create index index2 on table1(b) as 'bloomfilter'")
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("true"))
+ assert(CarbonIndexUtil.isIndexTableExists(parentCarbonTable).equalsIgnoreCase("true"))
+ CarbonIndexUtil.addOrModifyTableProperty(parentCarbonTable, Map("indexExists" -> "false",
+ "indextableexists" -> "false"))(sqlContext.sparkSession)
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("false"))
+ assert(CarbonIndexUtil.isIndexTableExists(parentCarbonTable).equalsIgnoreCase("false"))
+ val mock = TestSecondaryIndexUtils.mockIsIndexTableExists()
Review comment:
IMO, we don't need this mock. Could have just removed `indextableexists` and/or `indexexists` property appropriately from parent table. I mean, can do like below
`parentCarbonTable.getTableInfo.getFactTable.getTableProperties.remove("indextableexists")`
and/or
`parentCarbonTable.getTableInfo.getFactTable.getTableProperties.remove("indexexists")`
appropriately. Probably can check all testcases using `mockIsIndexTableExists()` and `mockIsIndexExists()`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770762628
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5389/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567657351
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -327,6 +259,35 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+
+ // exception is thrown by compaction executor
+ val mock3 = TestSecondaryIndexUtils.mockCompactionExecutor()
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
+ CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+ }
+
+ test("test refresh index command when block need to be sorted") {
+ CarbonProperties.getInstance()
+ .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ createTableAndLoadData("100", 2)
+ sql("CREATE INDEX nonindexmerge_index1 on table nonindexmerge (name) AS 'carbondata'")
+ val mock = TestSecondaryIndexUtils.mockIsSortRequired();
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ mock.tearDown()
+ val df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(isFilterPushedDownToSI(df1))
+ assert(getDataFileCount("nonindexmerge_index1", "0") < 15)
+ assert(getDataFileCount("nonindexmerge_index1", "1") < 15)
Review comment:
In beforeall we are setting this property as true. That's why in the beginning it is set to false, make it true at end of testcase and also made the changes for other testcases.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r562567398
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -288,9 +268,7 @@ class CarbonDataFileMergeTestCaseOnSI
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
}
- test("test verify data file merge when exception occurred in rebuild segment") {
- CarbonProperties.getInstance()
- .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ def createTableAndLoadData(): Unit = {
Review comment:
Still five occurences of 'CREATE TABLE nonindexmerge' can be replaced. GLOBAL_SORT_PARTITIONS can be passed from test cases
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567570962
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithColumnMetCacheAndCacheLevelProperty.scala
##########
@@ -363,5 +365,20 @@ class TestQueryWithColumnMetCacheAndCacheLevelProperty
sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
sql("drop index if exists parallel_index on parallel_index_load")
+ val mock = mockIsSchemaModified()
+ sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
+ checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
+ sql("drop index if exists parallel_index on parallel_index_load")
+ mock.tearDown()
Review comment:
We don't know the order of execution of testcases in a file. If we will remove it from here then it may impact the other testcases. I think it's better to keep teardown for mocked function because we may not required to mock the function for other testcases.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566590213
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -288,20 +216,24 @@ class CarbonDataFileMergeTestCaseOnSI
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
}
- test("test verify data file merge when exception occurred in rebuild segment") {
- CarbonProperties.getInstance()
- .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ def createTableAndLoadData(globalSortPartition: String, loadTimes: Int): Unit = {
sql("DROP TABLE IF EXISTS nonindexmerge")
sql(
"""
| CREATE TABLE nonindexmerge(id INT, name STRING, city STRING, age INT)
| STORED AS carbondata
| TBLPROPERTIES('SORT_COLUMNS'='city,name', 'SORT_SCOPE'='GLOBAL_SORT')
""".stripMargin)
- sql(s"LOAD DATA LOCAL INPATH '$file2' INTO TABLE nonindexmerge OPTIONS('header'='false', " +
- s"'GLOBAL_SORT_PARTITIONS'='100')")
- sql(s"LOAD DATA LOCAL INPATH '$file2' INTO TABLE nonindexmerge OPTIONS('header'='false', " +
- s"'GLOBAL_SORT_PARTITIONS'='100')")
+ for (_ <- 0 to loadTimes) {
Review comment:
loop runs for 0 to loadTimes(included) i.e., loadTimes + 1 times. You would want to use `until loadTimes` instead of `to loadTimes`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566616826
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
Review comment:
IMO, drop, create and use database should go in `beforeAll()`. And drop, create table, index and insert into can go in `beforeEach()`. No need to drop and create database for each testcase.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771393400
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770910752
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5396/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-773075985
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r569392649
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestRenameTableWithIndex.scala
##########
@@ -107,6 +107,17 @@ class TestRenameTableWithIndex extends QueryTest with BeforeAndAfterAll {
s"""
| explain select * from carbon_tb where name='eason'
""".stripMargin).collect()
+
+ sql(
+ s"""
+ | CREATE INDEX dm_carbon_si
+ | ON TABLE carbon_tb (name, city)
+ | AS 'carbondata'
+ """.stripMargin)
+
+ sql(s"""alter TABLE carbon_tb rename to carbon_table""".stripMargin)
Review comment:
done
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 removed a comment on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 removed a comment on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-762304031
@Indhumathi27 please find the comparison with master branch and current branch
x-special/nautilus-clipboard
copy
file:///home/nihal/Pictures/after1.png
file:///home/nihal/Pictures/before2.png
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r562603767
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
Review comment:
is it needed? proper use case?
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ }
+
+ test("test show index when SI were created before the change CARBONDATA-3765") {
+ val mock = TestSecondaryIndexUtils.mockIndexInfo()
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ mock.tearDown()
+ }
+
+ test("test refresh index with different value of isIndexTableExists") {
+ setVariables("index1")
+ sql("create index index2 on table1(b) as 'bloomfilter'")
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("true"))
+ CarbonIndexUtil.addOrModifyTableProperty(parentCarbonTable, Map("indexExists" -> "false"))(
Review comment:
i think here, you need to verify the property indextableexists which table has SI and indexExists for bloom/luecene
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567670649
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -288,20 +216,24 @@ class CarbonDataFileMergeTestCaseOnSI
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
}
- test("test verify data file merge when exception occurred in rebuild segment") {
- CarbonProperties.getInstance()
- .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ def createTableAndLoadData(globalSortPartition: String, loadTimes: Int): Unit = {
sql("DROP TABLE IF EXISTS nonindexmerge")
sql(
"""
| CREATE TABLE nonindexmerge(id INT, name STRING, city STRING, age INT)
| STORED AS carbondata
| TBLPROPERTIES('SORT_COLUMNS'='city,name', 'SORT_SCOPE'='GLOBAL_SORT')
""".stripMargin)
- sql(s"LOAD DATA LOCAL INPATH '$file2' INTO TABLE nonindexmerge OPTIONS('header'='false', " +
- s"'GLOBAL_SORT_PARTITIONS'='100')")
- sql(s"LOAD DATA LOCAL INPATH '$file2' INTO TABLE nonindexmerge OPTIONS('header'='false', " +
- s"'GLOBAL_SORT_PARTITIONS'='100')")
+ for (_ <- 0 to loadTimes) {
Review comment:
yeah wanted to use until load times. Made the 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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771436357
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5401/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-756127331
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5288/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r569357842
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -338,8 +276,48 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
- CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
- CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+
+ // exception is thrown by compaction executor
+ val mock3: MockUp[CarbonCompactionExecutor] = new MockUp[CarbonCompactionExecutor]() {
+ @Mock
+ def processTableBlocks(configuration: Configuration, filterExpr: Expression):
+ util.Map[String, util.List[RawResultIterator]] = {
+ throw new IOException("An exception occurred while compaction executor.")
+ }
+ }
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants
Review comment:
please verify data correctness also
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestRenameTableWithIndex.scala
##########
@@ -107,6 +107,17 @@ class TestRenameTableWithIndex extends QueryTest with BeforeAndAfterAll {
s"""
| explain select * from carbon_tb where name='eason'
""".stripMargin).collect()
+
+ sql(
+ s"""
+ | CREATE INDEX dm_carbon_si
+ | ON TABLE carbon_tb (name, city)
+ | AS 'carbondata'
+ """.stripMargin)
+
+ sql(s"""alter TABLE carbon_tb rename to carbon_table""".stripMargin)
Review comment:
as this operation is already done on line:93, you can move create si index before and validate
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCreateIndexWithLoadAndCompaction.scala
##########
@@ -283,19 +291,35 @@ class TestCreateIndexWithLoadAndCompaction extends QueryTest with BeforeAndAfter
}
sql("ALTER TABLE table1 COMPACT 'CUSTOM' WHERE SEGMENT.ID IN (1,2,3)")
- val segments = sql("SHOW SEGMENTS FOR TABLE idx1")
- val segInfos = segments.collect().map { each =>
- ((each.toSeq) (0).toString, (each.toSeq) (1).toString)
- }
- assert(segInfos.length == 6)
+ val segInfos = checkSegmentList(6)
assert(segInfos.contains(("0", "Success")))
assert(segInfos.contains(("1", "Compacted")))
assert(segInfos.contains(("2", "Compacted")))
assert(segInfos.contains(("3", "Compacted")))
assert(segInfos.contains(("1.1", "Success")))
assert(segInfos.contains(("4", "Success")))
checkAnswer(sql("select * from table1 where c3='b2'"), Seq(Row(3, "a2", "b2")))
- sql("drop table if exists table1")
+
+ // after clean files
+ val mock = mockreadSegmentList()
+ sql("CLEAN FILES FOR TABLE table1 options('force'='true')")
+ mock.tearDown()
+ val details = SegmentStatusManager.readLoadMetadata(CarbonEnv
+ .getCarbonTable(Some("default"), "idx1")(sqlContext.sparkSession).getMetadataPath)
+ assert(SegmentStatusManager.countInvisibleSegments(details, 4) == 1)
+ checkSegmentList(4)
+ CarbonProperties.getInstance()
Review comment:
please verify data correctness also
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771387700
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771513934
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5406/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566602925
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -327,6 +259,35 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+
+ // exception is thrown by compaction executor
+ val mock3 = TestSecondaryIndexUtils.mockCompactionExecutor()
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
+ CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+ }
+
+ test("test refresh index command when block need to be sorted") {
+ CarbonProperties.getInstance()
+ .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ createTableAndLoadData("100", 2)
+ sql("CREATE INDEX nonindexmerge_index1 on table nonindexmerge (name) AS 'carbondata'")
+ val mock = TestSecondaryIndexUtils.mockIsSortRequired();
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ mock.tearDown()
+ val df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(isFilterPushedDownToSI(df1))
+ assert(getDataFileCount("nonindexmerge_index1", "0") < 15)
+ assert(getDataFileCount("nonindexmerge_index1", "1") < 15)
Review comment:
`CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE` property is set to `false` at beginning and end of testcase. Note that `CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT` value is also `false`. Why set to `false` second time ? You might want to set to `true` at the end of testcase.
Same issue observed in following testcases. Please recheck for all of these cases.
`test("Verify data file merge in SI segments with sort scope as gloabl sort and" +
"CARBON_SI_SEGMENT_MERGE property is enabled")`
`test("Verify REFRESH INDEX command with sort scope as global sort")`
`test("test verify data file merge when exception occurred in rebuild segment")`
`test("test refresh index command when block need to be sorted")`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-772422851
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566602925
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -327,6 +259,35 @@ class CarbonDataFileMergeTestCaseOnSI
.queryExecution.sparkPlan
assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+
+ // exception is thrown by compaction executor
+ val mock3 = TestSecondaryIndexUtils.mockCompactionExecutor()
+ val exception2 = intercept[Exception] {
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ }
+ mock3.tearDown()
+ assert(exception2.getMessage.contains("Merge data files Failure in Merger Rdd."))
+ df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(getDataFileCount("nonindexmerge_index1", "0") == 100)
+ assert(getDataFileCount("nonindexmerge_index1", "1") == 100)
+ CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE,
+ CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
+ }
+
+ test("test refresh index command when block need to be sorted") {
+ CarbonProperties.getInstance()
+ .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ createTableAndLoadData("100", 2)
+ sql("CREATE INDEX nonindexmerge_index1 on table nonindexmerge (name) AS 'carbondata'")
+ val mock = TestSecondaryIndexUtils.mockIsSortRequired();
+ sql("REFRESH INDEX nonindexmerge_index1 ON TABLE nonindexmerge").collect()
+ mock.tearDown()
+ val df1 = sql("""Select * from nonindexmerge where name='n16000'""")
+ .queryExecution.sparkPlan
+ assert(isFilterPushedDownToSI(df1))
+ assert(getDataFileCount("nonindexmerge_index1", "0") < 15)
+ assert(getDataFileCount("nonindexmerge_index1", "1") < 15)
Review comment:
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE property is set to "false" at beginning and end of testcase. Note that CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT value is also "false". Why set to false second time ? You might want to set to true at the end of testcase.
Same issue observed in following testcases. Please recheck for all of these cases.
test("Verify data file merge in SI segments with sort scope as gloabl sort and" +
"CARBON_SI_SEGMENT_MERGE property is enabled")
test("Verify REFRESH INDEX command with sort scope as global sort")
test("test verify data file merge when exception occurred in rebuild segment")
test("test refresh index command when block need to be sorted")
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566061138
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithColumnMetCacheAndCacheLevelProperty.scala
##########
@@ -363,5 +365,20 @@ class TestQueryWithColumnMetCacheAndCacheLevelProperty
sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
sql("drop index if exists parallel_index on parallel_index_load")
+ val mock = mockIsSchemaModified()
+ sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
+ checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
+ sql("drop index if exists parallel_index on parallel_index_load")
+ mock.tearDown()
Review comment:
Don't have call tearDown() explicitly on mockup object in this case. Probably can check all calls to mockup object's tearDown() added in this PR and remove the ones which are not required.
jmockit teardown() description -
> Discards the mock methods originally set up by instantiating this mock-up object, restoring mocked methods to their original behaviors.
JMockit will automatically restore classes mocked by a test at the end of its execution, as well as classes mocked for the whole test class before the first test in the next test class is executed. Therefore, this method should rarely be used, if ever.
http://javadox.com/org.jmockit/jmockit/1.9/mockit/MockUp.html#tearDown()
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-761995983
@nihal0107 Can you attach the report of how which SI coverage is increased from this PR
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r563482082
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/mergedata/CarbonDataFileMergeTestCaseOnSI.scala
##########
@@ -288,9 +268,7 @@ class CarbonDataFileMergeTestCaseOnSI
CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE_DEFAULT)
}
- test("test verify data file merge when exception occurred in rebuild segment") {
- CarbonProperties.getInstance()
- .addProperty(CarbonCommonConstants.CARBON_SI_SEGMENT_MERGE, "false")
+ def createTableAndLoadData(): Unit = {
Review comment:
done
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/DropTableTest.scala
##########
@@ -88,4 +90,17 @@ 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 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 mock = TestSecondaryIndexUtils.mockSIDrop()
+ sql("drop table if exists testDrop")
+ mock.tearDown()
+ assert(Files.exists(Paths.get(warehouse + "/" + "index11")))
+ sql("drop table if exists testDrop")
+ sql("drop database if exists test cascade")
Review comment:
removed
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
Review comment:
this is just to check when we are passing index table as parent table then delete will fail. I have added comments and also included true condition.
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
+ sql("create database test")
+ sql("use test")
+ sql("drop table if exists table1")
+ sql("create table table1(a string, b string, c string) stored as carbondata")
+ sql("create index index1 on table1(b) as 'carbondata'")
+ sql("insert into table1 values('ab','bc','cd')")
+ }
+
+ def setVariables(indexName: String): Unit = {
+ tableIdentifier = new TableIdentifier(indexName, Some("test"))
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ indexTable = CarbonEnv.getCarbonTable(Some("test"), "index1")(sqlContext.sparkSession)
+ }
+
+ test("test delete index silent") {
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), false, "index1")
+ }
+
+ test("test delete index table silently when exception occur") {
+ setVariables("unknown")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ parentCarbonTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ setVariables("index1")
+ CarbonInternalMetastore.deleteIndexSilent(tableIdentifier, "",
+ indexTable)(sqlContext.sparkSession)
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ }
+
+ test("test show index when SI were created before the change CARBONDATA-3765") {
+ val mock = TestSecondaryIndexUtils.mockIndexInfo()
+ checkExistence(sql("show indexes on table1"), true, "index1")
+ mock.tearDown()
+ }
+
+ test("test refresh index with different value of isIndexTableExists") {
+ setVariables("index1")
+ sql("create index index2 on table1(b) as 'bloomfilter'")
+ parentCarbonTable = CarbonEnv.getCarbonTable(Some("test"), "table1")(sqlContext.sparkSession)
+ assert(CarbonIndexUtil.isIndexExists(parentCarbonTable).equalsIgnoreCase("true"))
+ CarbonIndexUtil.addOrModifyTableProperty(parentCarbonTable, Map("indexExists" -> "false"))(
Review comment:
yeah, made the changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771389326
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5400/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-768231235
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5362/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r567671338
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestCarbonInternalMetastore.scala
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.spark.testsuite.secondaryindex
+
+import org.apache.spark.sql.CarbonEnv
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.index.CarbonIndexUtil
+import org.apache.spark.sql.secondaryindex.hive.CarbonInternalMetastore
+import org.apache.spark.sql.test.util.QueryTest
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import org.apache.carbondata.core.metadata.schema.table.CarbonTable
+
+class TestCarbonInternalMetastore extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach {
+
+ val dbName: String = "test"
+ val tableName: String = "table1"
+ val indexName: String = "index1"
+ var tableIdentifier: TableIdentifier = _
+ var parentCarbonTable: CarbonTable = _
+ var indexTable: CarbonTable = _
+
+ override def beforeAll(): Unit = {
+ sql("drop database if exists test cascade");
+ }
+
+ override def beforeEach(): Unit = {
+ sql("drop database if exists test cascade");
Review comment:
done the changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-771354445
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-770849879
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5393/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566094958
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithColumnMetCacheAndCacheLevelProperty.scala
##########
@@ -363,5 +365,20 @@ class TestQueryWithColumnMetCacheAndCacheLevelProperty
sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
sql("drop index if exists parallel_index on parallel_index_load")
+ val mock = mockIsSchemaModified()
+ sql("CREATE INDEX parallel_index on parallel_index_load(b) AS 'carbondata'")
+ checkAnswer(sql("select b from parallel_index"), Seq(Row("bb"), Row("dd"), Row("ff")))
+ sql("drop index if exists parallel_index on parallel_index_load")
+ mock.tearDown()
+ }
+
+ def mockIsSchemaModified(): MockUp[TableInfo] = {
Review comment:
This tableInfo mockup instance for mocking isSchemaModified method to return true is specific to a particular testcase. No need to define a separate method. How about directly setup in testcase(line 368 can have like below) ?
new MockUp[TableInfo] {
@Mock
def isSchemaModified(): Boolean = {
true
}
}
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] nihal0107 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#issuecomment-768178940
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4071: [CARBONDATA-4102] Added UT and FT to improve coverage of SI module.
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4071:
URL: https://github.com/apache/carbondata/pull/4071#discussion_r566686354
##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondaryIndex.scala
##########
@@ -577,6 +589,59 @@ class TestSIWithSecondaryIndex extends QueryTest with BeforeAndAfterAll {
assert(ex.getMessage.contains("Problem loading data while creating secondary index:"))
}
+ test("test SI with carbon.use.local.dir as true") {
+ CarbonProperties.getInstance()
Review comment:
@nihal0107 if this property is not used anywhere, then you remove it
----------------------------------------------------------------
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:
users@infra.apache.org