[GitHub] [carbondata] marchpure opened a new pull request #3981: [CARBONDATA-4027] Incorrect query result after Update/Delete and Inse…

[GitBox]

marchpure commented on pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716406302


   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]

marchpure commented on pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716406534


   > please do rebase
   
   I have do rebase according to your suggestion


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716460174


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


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716466945


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


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716977104


   @marchpure please correct the PR title and please give detailed explanation of the issue and the fix in the PR description.


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

marchpure commented on pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-716981130


   > @marchpure please correct the PR title and please give detailed explanation of the issue and the fix in the PR description.
   
   Yes. I have modified the PR desc as your suggestion


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

marchpure commented on pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-717259421


   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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-717328681


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


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-717333719


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


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#discussion_r512875192



##########
File path: core/src/main/java/org/apache/carbondata/core/mutate/CarbonUpdateUtil.java
##########
@@ -276,14 +278,13 @@ public static boolean updateTableMetadataStatus(Set<Segment> updatedSegmentsList
                 SegmentStatusManager.readLoadMetadata(metaDataFilepath);
 
         for (LoadMetadataDetails loadMetadata : listOfLoadFolderDetailsArray) {
+          if (isUpdateStatusFileUpdateRequired &&
+              loadMetadata.getLoadName().equalsIgnoreCase("0")) {
+            loadMetadata.setUpdateStatusFileName(
+                CarbonUpdateUtil.getUpdateStatusFileName(updatedTimeStamp));
+          }
 
           if (isTimestampUpdateRequired) {
-            // we are storing the link between the 2 status files in the segment 0 only.

Review comment:
       why this condition is moved up? and why not `isTimestampUpdateRequired`




----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-717398581


   @marchpure as from the description, i understood that the table status file after insert overwrite do not contain the `updatetablestatusfile ` name. So why do we need to add boolean variable, and send this boolean from many places. This may lead to confusion later also. So why dont we take care it just while updating the table status in case of insert overwrite?
   
   I mean to say, while updating the status after insert overwrite, we can first copy the existing meta and then prepare new entry and update case can be handled there, this way, it will lead to less changes and less confusion later.


----------------------------------------------------------------
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 #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-717398850


   > > @marchpure please correct the PR title and please give detailed explanation of the issue and the fix in the PR description.
   >
   > Yes. I have modified the PR desc as your suggestion
   
   PR title is still not yet formatted, please 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:
[hidden email]

[GitBox]

ajantha-bhat commented on pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981#issuecomment-725199629


   @marchpure : This is already handled in #3999 , need to close this PR ?


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

marchpure closed pull request #3981:
URL: https://github.com/apache/carbondata/pull/3981


   


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