[GitHub] [carbondata] nihal0107 opened a new pull request #4015: Handled insert overwrite scenario for SI

[GitBox]

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/rdd/SecondaryIndexCreator.scala
##########
@@ -398,6 +404,28 @@ object SecondaryIndexCreator {
           secondaryIndexModel.sqlContext.sparkSession,
           carbonLoadModelForMergeDataFiles.getFactTimeStamp,
           rebuiltSegments)
+
+        if (isInsertOverwrite) {
+          var staleSegmentsList = new ListBuffer[String]()
+          SegmentStatusManager
+            .readLoadMetadata(indexCarbonTable.getMetadataPath).foreach { loadMetadata =>
+            if (!successSISegments.contains(loadMetadata.getLoadName)) {

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

[GitBox]

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/rdd/SecondaryIndexCreator.scala
##########
@@ -398,6 +404,22 @@ object SecondaryIndexCreator {
           secondaryIndexModel.sqlContext.sparkSession,
           carbonLoadModelForMergeDataFiles.getFactTimeStamp,
           rebuiltSegments)
+
+        if (isInsertOverwrite) {
+          FileInternalUtil
+            .updateTableStatus(
+              SegmentStatusManager
+                .readLoadMetadata(indexCarbonTable.getMetadataPath)
+                .filter(loadMetadata => !successSISegments.contains(loadMetadata.getLoadName))
+                .map(_.getLoadName).toList,
+              secondaryIndexModel.carbonLoadModel.getDatabaseName,

Review comment:
       please do not add complete filter logic inside a method parameter, please assign that to a variable for clean code practice.




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

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


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


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

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


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


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

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/rdd/SecondaryIndexCreator.scala
##########
@@ -398,6 +404,22 @@ object SecondaryIndexCreator {
           secondaryIndexModel.sqlContext.sparkSession,
           carbonLoadModelForMergeDataFiles.getFactTimeStamp,
           rebuiltSegments)
+
+        if (isInsertOverwrite) {
+          FileInternalUtil
+            .updateTableStatus(
+              SegmentStatusManager
+                .readLoadMetadata(indexCarbonTable.getMetadataPath)
+                .filter(loadMetadata => !successSISegments.contains(loadMetadata.getLoadName))
+                .map(_.getLoadName).toList,
+              secondaryIndexModel.carbonLoadModel.getDatabaseName,

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

[GitBox]

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


   LGTM


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

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

[GitBox]

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


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


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

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


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


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

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


   


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