[GitHub] [carbondata] ajantha-bhat opened a new pull request #3944: [WIP] Fix SI global sort load on partition table

[GitBox]

ajantha-bhat opened a new pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944


    ### Why is this PR needed?
   
   
    ### What changes were proposed in this PR?
   
       
    ### Does this PR introduce any user interface change?
    - No
    - Yes. (please explain the change and update document)
   
    ### Is any new testcase added?
    - No
    - 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 #3944:
URL: https://github.com/apache/carbondata/pull/3944#issuecomment-696187761


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


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


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


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






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

kunal642 commented on a change in pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#discussion_r492552371



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       revert this change.. Not related to 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]

ajantha-bhat commented on a change in pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#discussion_r492562276



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       Actually when the main table was giving 0 rows. I was debugging this flow. And this naming is wrong. segmentID should be string, here it is not the segmentID but the complete Segment object. so renamed. Still you want to revert it ?




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



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       Actually when the main table was giving 0 rows. I was debugging this flow. And this naming is wrong (confusing to debug). segmentID should be string, here it is not the segmentID but the complete Segment object. so renamed. Still you want to revert it ?




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

kunal642 commented on a change in pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#discussion_r492632500



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       its fine.. ill 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]

[GitBox]

kunal642 commented on pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#issuecomment-696639688


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


   


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

kunal642 commented on a change in pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#discussion_r492552371



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       revert this change.. Not related to this PR

##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       its fine.. ill 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]

[GitBox]

ajantha-bhat commented on a change in pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#discussion_r492562276



##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       Actually when the main table was giving 0 rows. I was debugging this flow. And this naming is wrong. segmentID should be string, here it is not the segmentID but the complete Segment object. so renamed. Still you want to revert it ?

##########
File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java
##########
@@ -466,15 +466,15 @@ public IndexFilter getFilterPredicates(Configuration configuration) {
    * get data blocks of given segment
    */
   protected List<CarbonInputSplit> getDataBlocksOfSegment(JobContext job, CarbonTable carbonTable,
-      IndexFilter expression, List<Segment> segmentIds,
+      IndexFilter expression, List<Segment> validSegments,

Review comment:
       Actually when the main table was giving 0 rows. I was debugging this flow. And this naming is wrong (confusing to debug). segmentID should be string, here it is not the segmentID but the complete Segment object. so renamed. Still you want to revert it ?




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

kunal642 commented on pull request #3944:
URL: https://github.com/apache/carbondata/pull/3944#issuecomment-696639688


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


   


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