[GitHub] [carbondata] VenuReddy2103 opened a new pull request #3725: [HOTFIX]Alter table drop column on main table is not dropping the eligible secondary index tables

[GitBox]

VenuReddy2103 opened a new pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725


   
   ### Why is this PR needed?
   Alter table drop column on main table when droping columns match to a secondary index table columns is actually not dropping secondary index table.
   
    ### What changes were proposed in this PR?
   1. Have corrected the `dropApplicableSITables` to check if the drop columns are matching the index table columns. If all columns of a index table match to drop columns, then drop the index table itself.
   2. Index table refresh was missed in `CarbonCreateSecondaryIndexCommand`. It was missed in secondary index feature PR.
   
    ### 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:
[hidden email]

[GitBox]

CarbonDataQA1 commented on issue #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618535535


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618610776


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618720390


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618720616


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618837476


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-618838353


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on a change in pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#discussion_r415548051



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
##########
@@ -61,6 +62,16 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     }
   }
 
+  test ("test alter drop columns on the SI columns") {
+    sql("create table test1 (name string, id string, country string) stored as carbondata")

Review comment:
       please change table name, and give some meaningful name with issue




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on a change in pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#discussion_r415548360



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
##########
@@ -61,6 +62,16 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     }
   }
 
+  test ("test alter drop columns on the SI columns") {
+    sql("create table test1 (name string, id string, country string) stored as carbondata")
+    sql("insert into test1 select 'xx', '1', 'china'")
+    sql("create index t_index on table test1(id, country) as 'carbondata'")
+    // alter table to drop the columns used in index
+    sql("alter table test1 drop columns(id, country)")
+    sql("insert into test1 select 'xy'")
+    assert(sql("show indexes on test1").collect().isEmpty)
+  }
+
   test("Test secondry index data count") {

Review comment:
       add one more test case where the all columns of are not dropped and we get the error.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#discussion_r415748871



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
##########
@@ -61,6 +62,16 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     }
   }
 
+  test ("test alter drop columns on the SI columns") {
+    sql("create table test1 (name string, id string, country string) stored as carbondata")

Review comment:
       modified




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#discussion_r415749044



##########
File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala
##########
@@ -61,6 +62,16 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll {
     }
   }
 
+  test ("test alter drop columns on the SI columns") {
+    sql("create table test1 (name string, id string, country string) stored as carbondata")
+    sql("insert into test1 select 'xx', '1', 'china'")
+    sql("create index t_index on table test1(id, country) as 'carbondata'")
+    // alter table to drop the columns used in index
+    sql("alter table test1 drop columns(id, country)")
+    sql("insert into test1 select 'xy'")
+    assert(sql("show indexes on test1").collect().isEmpty)
+  }
+
   test("Test secondry index data count") {

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:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-619939530


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-619945072


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-619947105


   @VenuReddy2103 please fix the 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:
[hidden email]

[GitBox]

ydvpankaj99 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620084794


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620090865


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620157085


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620178875


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620238804


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3725:
URL: https://github.com/apache/carbondata/pull/3725#issuecomment-620246809


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]