[GitHub] [carbondata] marchpure opened a new pull request #3696: [HOTFIX] Fix Repeated access to getSegmentProperties

[GitBox]

marchpure opened a new pull request #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696
 
 
    ### Why is this PR needed?
    getSegmentProperties is accessed repeated in query processing, which  leads to  heavy time overhead.
   
    ### What changes were proposed in this PR?
   Reuse the segmentProperty.
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - No
   
       
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609058390
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2638/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609060050
 
 
   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/929/
   

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


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on a change in pull request #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#discussion_r403857254
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletIndexFactory.java
 ##########
 @@ -653,9 +653,13 @@ public SegmentProperties getSegmentPropertiesFromDataMap(Index coarseGrainIndex)
       throws IOException {
     List<Blocklet> blocklets = new ArrayList<>();
     List<CoarseGrainIndex> dataMaps = getIndexes(segment, partitions);
+    if (dataMaps.size() == 0 || dataMaps == null) {
 
 Review comment:
   dataMaps cannot be null from getIndexes, it can be empty.
   **So, remove it.**
   
   Also If we checking null, it has to be the first condition of || else we can get NPE for null scenarios

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


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on a change in pull request #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#discussion_r403857254
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletIndexFactory.java
 ##########
 @@ -653,9 +653,13 @@ public SegmentProperties getSegmentPropertiesFromDataMap(Index coarseGrainIndex)
       throws IOException {
     List<Blocklet> blocklets = new ArrayList<>();
     List<CoarseGrainIndex> dataMaps = getIndexes(segment, partitions);
+    if (dataMaps.size() == 0 || dataMaps == null) {
 
 Review comment:
   dataMaps cannot be null from getIndexes, it can be empty.
   **So, remove null check.**
   
   Also If we checking null, it has to be the first condition of || else we can get NPE for null scenarios

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


With regards,
Apache Git Services

[GitBox]

marchpure commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609882006
 
 
   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]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609884426
 
 
   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/945/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609958244
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2656/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-609964286
 
 
   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/946/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610272808
 
 
   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/951/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610275540
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2661/
   

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


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610305169
 
 
   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]


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610305928
 
 
   LGTM.
   
   waiting for build to merge

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610363572
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2666/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610370243
 
 
   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/956/
   

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


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610784413
 
 
   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]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610847525
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2678/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610866626
 
 
   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/966/
   

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


With regards,
Apache Git Services

[GitBox]

asfgit closed pull request #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
URL: https://github.com/apache/carbondata/pull/3696
 
 
   

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


With regards,
Apache Git Services