[GitHub] [carbondata] Indhumathi27 opened a new pull request #4087: [WIP] SI compatability issue fix

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox

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


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


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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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


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


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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] akashrn5 commented on a change in pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala
##########
@@ -239,6 +249,25 @@ private[sql] case class CarbonCreateSecondaryIndexCommand(
         throw new ErrorMessage(
           s"Table [$tableName] under database [$databaseName] is already an index table")
       }
+      val absoluteTableIdentifier = AbsoluteTableIdentifier.
+        from(tablePath, databaseName, indexTableName)
+      val indexTablePath = CarbonTablePath
+        .getMetadataPath(absoluteTableIdentifier.getTablePath)
+      val mainTblLoadMetadataDetails: Array[LoadMetadataDetails] =
+        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
+      var siTblLoadMetadataDetails: Array[LoadMetadataDetails] =
+        SegmentStatusManager.readLoadMetadata(indexTablePath)
+      if (isRegisterIndex) {
+        // check if SI table has undergone any Update or delete operation, which can happen in
+        // case of compatibility scenario. IUD after Refresh SI and before register index
+        val updatedSegmentsCount = siTblLoadMetadataDetails.filter(loadMetaDetail =>
+          !loadMetaDetail.getUpdateStatusFileName.equals(""))
+        if (!updatedSegmentsCount.isEmpty) {
+          throw new ErrorMessage("Cannot Register Secondary index table " + indexTableName +

Review comment:
       use the scala string concat as scala file, `s"message ${var}"`




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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] akashrn5 commented on a change in pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala
##########
@@ -239,6 +249,25 @@ private[sql] case class CarbonCreateSecondaryIndexCommand(
         throw new ErrorMessage(
           s"Table [$tableName] under database [$databaseName] is already an index table")
       }
+      val absoluteTableIdentifier = AbsoluteTableIdentifier.
+        from(tablePath, databaseName, indexTableName)
+      val indexTablePath = CarbonTablePath
+        .getMetadataPath(absoluteTableIdentifier.getTablePath)
+      val mainTblLoadMetadataDetails: Array[LoadMetadataDetails] =
+        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
+      var siTblLoadMetadataDetails: Array[LoadMetadataDetails] =
+        SegmentStatusManager.readLoadMetadata(indexTablePath)
+      if (isRegisterIndex) {
+        // check if SI table has undergone any Update or delete operation, which can happen in
+        // case of compatibility scenario. IUD after Refresh SI and before register index
+        val updatedSegmentsCount = siTblLoadMetadataDetails.filter(loadMetaDetail =>
+          !loadMetaDetail.getUpdateStatusFileName.equals(""))
+        if (!updatedSegmentsCount.isEmpty) {
+          throw new ErrorMessage("Cannot Register Secondary index table " + indexTableName +

Review comment:
       use the scala string concat as scala file, `s"message ${var}"`. Please check in other places 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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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


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


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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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


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


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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] akashrn5 commented on pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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


   LGTM


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

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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] asfgit closed pull request #4087: [CARBONDATA-4125] SI compatability issue fix

GitBox
In reply to this post by GitBox

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


   


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


12