[GitHub] [carbondata] marchpure opened a new pull request #3881: [HOTFIX] NPE While Data Loading

[GitBox]

marchpure opened a new pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881


    ### Why is this PR needed?
   1. getLastModifiedTime of LoadMetadataDetails fails due to "updateDeltaEndTimestamp is empty string".
   2. In the getCommittedIndexFile founction, NPE happens because of "segmentfile is null" under the Unusual cases.
   3. Cleaning temp files fails because of "partitionInfo is null"  under the unusual cases.
   
    ### What changes were proposed in this PR?
   1. when updateDeltaEndTimestamp is empty string, skip the code "convertTimeStampToLong(updateDeltaEndTimestamp)";
   2. when segmentfile is null, avoid  trigger "segmentfile.getSegmentMetaDataInfo()"
   3. init partitionInfo with a empty list instead of "null"
       
    ### 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 pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#issuecomment-669696213


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


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


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


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






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

QiangCai commented on a change in pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#discussion_r466295972



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonRelation.scala
##########
@@ -207,7 +207,7 @@ case class CarbonRelation(
                   null != validSeg.getLoadMetadataDetails.getIndexSize) {
                 size = size + validSeg.getLoadMetadataDetails.getDataSize.toLong +
                        validSeg.getLoadMetadataDetails.getIndexSize.toLong
-              } else {
+              } else if (!carbonTable.isHivePartitionTable) {

Review comment:
       why add this check?

##########
File path: core/src/main/java/org/apache/carbondata/core/readcommitter/TableStatusReadCommittedScope.java
##########
@@ -87,7 +87,9 @@ public TableStatusReadCommittedScope(AbsoluteTableIdentifier identifier,
       SegmentFileStore fileStore =
           new SegmentFileStore(identifier.getTablePath(), segment.getSegmentFileName());
       indexFiles = fileStore.getIndexOrMergeFiles();
-      segment.setSegmentMetaDataInfo(fileStore.getSegmentFile().getSegmentMetaDataInfo());
+      if (fileStore != null && fileStore.getSegmentFile() != null) {

Review comment:
       no need to check "fileStore != null"




----------------------------------------------------------------
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 a change in pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#discussion_r466303536



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonRelation.scala
##########
@@ -207,7 +207,7 @@ case class CarbonRelation(
                   null != validSeg.getLoadMetadataDetails.getIndexSize) {
                 size = size + validSeg.getLoadMetadataDetails.getDataSize.toLong +
                        validSeg.getLoadMetadataDetails.getIndexSize.toLong
-              } else {
+              } else if (!carbonTable.isHivePartitionTable) {

Review comment:
       Here, it aims collect the datasize of segment path. but the format of segment path generated is "Fart/Part0/Segment_0".
   For partition table. will throw out FileNotFound exception.




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


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


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


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


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

QiangCai commented on a change in pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#discussion_r466769303



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonRelation.scala
##########
@@ -207,7 +207,7 @@ case class CarbonRelation(
                   null != validSeg.getLoadMetadataDetails.getIndexSize) {
                 size = size + validSeg.getLoadMetadataDetails.getDataSize.toLong +
                        validSeg.getLoadMetadataDetails.getIndexSize.toLong
-              } else {
+              } else if (!carbonTable.isHivePartitionTable) {

Review comment:
       better to find the root cause and fix it. LoadMetadataDetail should have data/index size beside of old store.




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

QiangCai commented on pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#issuecomment-670270759


   please create an issue in JIRA to describe the issues.


----------------------------------------------------------------
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 a change in pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#discussion_r466962667



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonRelation.scala
##########
@@ -207,7 +207,7 @@ case class CarbonRelation(
                   null != validSeg.getLoadMetadataDetails.getIndexSize) {
                 size = size + validSeg.getLoadMetadataDetails.getDataSize.toLong +
                        validSeg.getLoadMetadataDetails.getIndexSize.toLong
-              } else {
+              } else if (!carbonTable.isHivePartitionTable) {

Review comment:
       modified

##########
File path: core/src/main/java/org/apache/carbondata/core/readcommitter/TableStatusReadCommittedScope.java
##########
@@ -87,7 +87,9 @@ public TableStatusReadCommittedScope(AbsoluteTableIdentifier identifier,
       SegmentFileStore fileStore =
           new SegmentFileStore(identifier.getTablePath(), segment.getSegmentFileName());
       indexFiles = fileStore.getIndexOrMergeFiles();
-      segment.setSegmentMetaDataInfo(fileStore.getSegmentFile().getSegmentMetaDataInfo());
+      if (fileStore != null && fileStore.getSegmentFile() != null) {

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]

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


   issue created


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


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


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


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


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

QiangCai commented on pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#issuecomment-670853149


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


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


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


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


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

QiangCai commented on pull request #3881:
URL: https://github.com/apache/carbondata/pull/3881#issuecomment-671033903


   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]

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


   


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