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 2020/10/30 07:20:45 UTC
[GitHub] [carbondata] nihal0107 opened a new pull request #4000: [WIP][CARBONDATA-4020]Fixed drop index when multiple index exists
nihal0107 opened a new pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000
### Why is this PR needed?
Currently when we have multiple bloom indexes and we drop one index then 'show index' command is showing an empty index list.
### What changes were proposed in this PR?
Checked, if no CG or FG index then set indexExist as true.
### 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] CarbonDataQA2 commented on pull request #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-737029820
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5010/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-734743170
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4950/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-736988368
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531507427
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
ok, removed the check because indexes map can have max 3 elements and it won't throw any exception for empty map.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-736216044
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] CarbonDataQA2 commented on pull request #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-737027155
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3255/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r532377244
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,20 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1", "index2")
+ sql(s"drop index index1 on $bloomSampleTable")
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index2")
Review comment:
done
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,20 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
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] VenuReddy2103 commented on a change in pull request #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r532368311
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,20 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
Review comment:
This line can be 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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-733019583
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3135/
----------------------------------------------------------------
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] CarbonDataQA1 commented on pull request #4000: [WIP][CARBONDATA-4020]Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-719435945
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2978/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531409958
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
indexmetadata will also have SI Indexes. indexExists property is only for CG or FG indexes
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r529229059
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,22 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1")
Review comment:
```suggestion
checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1", "index2" )
```
Remove next line
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,22 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1")
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index2")
+ sql(s"drop index index1 on $bloomSampleTable")
+ sql(s"show indexes on table $bloomSampleTable").show()
Review comment:
remove this line
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,10 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
Please add a comment, on which case, we need to set 'indexExists' to false
----------------------------------------------------------------
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] CarbonDataQA1 commented on pull request #4000: [WIP][CARBONDATA-4020]Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-719433040
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4736/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-734801021
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4955/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531409321
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
Review comment:
For example, create 2 bloom indexes, Drop both of them. In last index drop, `indexMetadata` will be null at this point(line 186). We do not seem to set `indexExists` property to `false` in that case.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-734801467
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3200/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531399137
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
`indexMetadata.getIndexesMap.size() != 0` would always be true at this point. `indexMetadata` will be null if empty. It is redundant check.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531399137
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
`indexMetadata.getIndexesMap.size() != 0` would always be true at this point. `indexMetadata` will be null if empty.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-734746457
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3195/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r529518888
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,22 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1")
Review comment:
done
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,22 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1")
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index2")
+ sql(s"drop index index1 on $bloomSampleTable")
+ sql(s"show indexes on table $bloomSampleTable").show()
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 a change in pull request #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531434575
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
Review comment:
ok, handle the scenario when no cg or fg index exists then set `indexExists` property as false. Earlier this case was not handled when we drop all indexes then we didn't set the `indexExists` property as false.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-735640398
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3214/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-737030967
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-737079277
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3258/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-737076274
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5014/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531416935
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
I understand that `indexMetadata` will have SI indexes as well. But, What i meant was indexMetadata.getIndexesMap.size() != 0 always evaluates to true at line 189.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
nihal0107 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r529518625
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,10 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
added.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r532367120
##########
File path: integration/spark/src/test/scala/org/apache/carbondata/index/bloom/BloomCoarseGrainIndexFunctionSuite.scala
##########
@@ -660,6 +660,20 @@ class BloomCoarseGrainIndexFunctionSuite
sql(s"SELECT * FROM $normalTable WHERE salary='1040'"))
}
+ test("test drop index when more than one bloom index exists") {
+ sql(s"CREATE TABLE $bloomSampleTable " +
+ "(id int,name string,salary int)STORED as carbondata TBLPROPERTIES('SORT_COLUMNS'='id')")
+ sql(s"CREATE index index1 ON TABLE $bloomSampleTable(id) as 'bloomfilter' " +
+ "PROPERTIES ( 'BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"CREATE index index2 ON TABLE $bloomSampleTable (name) as 'bloomfilter' " +
+ "PROPERTIES ('BLOOM_SIZE'='640000', 'BLOOM_FPP'='0.00001', 'BLOOM_COMPRESS'='true')")
+ sql(s"insert into $bloomSampleTable values(1,'nihal',20)")
+ sql(s"SHOW INDEXES ON TABLE $bloomSampleTable").collect()
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index1", "index2")
+ sql(s"drop index index1 on $bloomSampleTable")
+ checkExistence(sql(s"SHOW INDEXES ON TABLE $bloomSampleTable"), true, "index2")
Review comment:
add case for drop all cg/fg indexes 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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-735637553
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4969/
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r532363660
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -183,17 +183,20 @@ private[sql] case class DropIndexCommand(
parentCarbonTable)
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
+ var hasCgFgIndexes = false
Review comment:
Can keep old code only i think, as hasCgFgIndexes is not used in multiple places. Just change hasCgFgIndexes logic.
----------------------------------------------------------------
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 #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
VenuReddy2103 commented on a change in pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#discussion_r531416935
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/DropIndexCommand.scala
##########
@@ -184,10 +184,12 @@ private[sql] case class DropIndexCommand(
parentCarbonTable = getRefreshedParentTable(sparkSession, dbName)
val indexMetadata = parentCarbonTable.getIndexMetadata
if (null != indexMetadata && null != indexMetadata.getIndexesMap) {
- val hasCgFgIndexes =
- !(indexMetadata.getIndexesMap.size() == 1 &&
- indexMetadata.getIndexesMap.containsKey(IndexType.SI.getIndexProviderName))
- if (hasCgFgIndexes) {
+ // check if any CG or FG index exists. If not exists,
+ // then set indexExists as false to return empty index list for next query.
+ val hasCgFgIndexes = indexMetadata.getIndexesMap.size() != 0 &&
Review comment:
Got 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
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4000: [CARBONDATA-4020] Fixed drop index when multiple index exists
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4000:
URL: https://github.com/apache/carbondata/pull/4000#issuecomment-733012325
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4889/
----------------------------------------------------------------
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