[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4107: [CARBONDATA-4149] Query with SI after add partition based on location on partition table gives incorrect results

[GitBox]

Indhumathi27 commented on a change in pull request #4107:
URL: https://github.com/apache/carbondata/pull/4107#discussion_r596686311



##########
File path: 
core/src/main/java/org/apache/carbondata/core/indexstore/ExtendedBlockletWrapper.java
##########
@@ -121,8 +121,9 @@ public ExtendedBlockletWrapper(List<ExtendedBlocklet> 
extendedBlockletList, Stri
     DataOutputStream stream = new DataOutputStream(bos);
     try {
       for (ExtendedBlocklet extendedBlocklet : extendedBlockletList) {
+        boolean isExternalPath = 
!extendedBlocklet.getFilePath().contains(tablePath);

Review comment:
       please check and remove, as discussed

##########
File path: 
core/src/main/java/org/apache/carbondata/core/readcommitter/TableStatusReadCommittedScope.java
##########
@@ -86,6 +87,13 @@ public TableStatusReadCommittedScope(AbsoluteTableIdentifier 
identifier,
     } else {
       SegmentFileStore fileStore =
           new SegmentFileStore(identifier.getTablePath(), 
segment.getSegmentFileName());
+      Optional<SegmentFileStore.FolderDetails>

Review comment:
       please add a comment, about this scenario

##########
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/partition/CarbonAlterTableAddHivePartitionCommand.scala
##########
@@ -137,6 +153,19 @@ case class CarbonAlterTableAddHivePartitionCommand(
         CarbonUtil.checkAndCreateFolderWithPermission(segmentsLoc)
         val segmentPath = segmentsLoc + CarbonCommonConstants.FILE_SEPARATOR + 
segmentFileName
         SegmentFileStore.writeSegmentFile(segmentFile, segmentPath)
+        CarbonLoaderUtil
+          .recordNewLoadMetadata(newMetaEntry, loadModel, false, false)
+        operationContext.setProperty(table.getTableUniqueName + "_Segment", 
loadModel.getSegmentId)

Review comment:
       We no need to fire events for SI, since we are not going to load data to 
SI




----------------------------------------------------------------
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:
us...@infra.apache.org