[GitHub] [carbondata] QiangCai opened a new pull request #3860: [CARBONDATA-3889] Cleanup duplicated code in carbondata-core module

[GitBox]

QiangCai opened a new pull request #3860:
URL: https://github.com/apache/carbondata/pull/3860


    ### Why is this PR needed?
    There are duplicated code in carbondata-core module
   
    ### What changes were proposed in this PR?
   1. Cleanup duplicated code in carbondata-core module
   2. Fix the typo
       
    ### 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]

[GitBox]

CarbonDataQA1 commented on pull request #3860:
URL: https://github.com/apache/carbondata/pull/3860#issuecomment-662951456


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


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


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


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


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


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


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


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


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


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


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


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

kevinjmh commented on a change in pull request #3860:
URL: https://github.com/apache/carbondata/pull/3860#discussion_r460610223



##########
File path: core/src/main/java/org/apache/carbondata/core/index/dev/expr/AndIndexExprWrapper.java
##########
@@ -47,25 +47,20 @@ public AndIndexExprWrapper(IndexExprWrapper left, IndexExprWrapper right,
   }
 
   @Override
-  public List<ExtendedBlocklet> prune(List<Segment> segments, List<PartitionSpec> partitionsToPrune)
-      throws IOException {
-    List<ExtendedBlocklet> leftPrune = left.prune(segments, partitionsToPrune);
-    List<ExtendedBlocklet> rightPrune = right.prune(segments, partitionsToPrune);
-    List<ExtendedBlocklet> andBlocklets = new ArrayList<>();
-    for (ExtendedBlocklet blocklet : leftPrune) {
-      if (rightPrune.contains(blocklet)) {
-        andBlocklets.add(blocklet);
-      }
-    }
-    return andBlocklets;
+  public List<ExtendedBlocklet> prune(List<Segment> segments,
+      List<PartitionSpec> partitionsToPrune) throws IOException {
+    return and(left.prune(segments, partitionsToPrune), right.prune(segments, partitionsToPrune));
   }
 
   @Override
   public List<ExtendedBlocklet> prune(IndexInputSplit distributable,
-      List<PartitionSpec> partitionsToPrune)
-          throws IOException {
-    List<ExtendedBlocklet> leftPrune = left.prune(distributable, partitionsToPrune);
-    List<ExtendedBlocklet> rightPrune = right.prune(distributable, partitionsToPrune);
+      List<PartitionSpec> partitionsToPrune) throws IOException {
+    return and(left.prune(distributable, partitionsToPrune),
+        right.prune(distributable, partitionsToPrune));
+  }
+
+  private List<ExtendedBlocklet> and(List<ExtendedBlocklet> leftPrune,
+      List<ExtendedBlocklet> rightPrune) {

Review comment:
       `pruneBlocklets` also can apply to this method too




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

kevinjmh commented on a change in pull request #3860:
URL: https://github.com/apache/carbondata/pull/3860#discussion_r460611697



##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/chunk/impl/FixedLengthDimensionColumnPage.java
##########
@@ -136,15 +131,29 @@ public int fillVector(ColumnVectorInfo[] vectorInfo, int chunkIndex) {
           } else if (dataType == DataTypes.LONG) {
             vector.putLong(vectorOffset++, (long) valueFromSurrogate);
           } else {
-            throw new IllegalArgumentException("unsupported data type: " +
-                columnVectorInfo.directDictionaryGenerator.getReturnType());
+            throw new IllegalArgumentException(
+                "unsupported data type: " + columnVectorInfo.directDictionaryGenerator
+                    .getReturnType());

Review comment:
       no need to change this




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



##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/chunk/impl/FixedLengthDimensionColumnPage.java
##########
@@ -136,15 +131,29 @@ public int fillVector(ColumnVectorInfo[] vectorInfo, int chunkIndex) {
           } else if (dataType == DataTypes.LONG) {
             vector.putLong(vectorOffset++, (long) valueFromSurrogate);
           } else {
-            throw new IllegalArgumentException("unsupported data type: " +
-                columnVectorInfo.directDictionaryGenerator.getReturnType());
+            throw new IllegalArgumentException(
+                "unsupported data type: " + columnVectorInfo.directDictionaryGenerator
+                    .getReturnType());

Review comment:
       reverted




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



##########
File path: core/src/main/java/org/apache/carbondata/core/index/dev/expr/AndIndexExprWrapper.java
##########
@@ -47,25 +47,20 @@ public AndIndexExprWrapper(IndexExprWrapper left, IndexExprWrapper right,
   }
 
   @Override
-  public List<ExtendedBlocklet> prune(List<Segment> segments, List<PartitionSpec> partitionsToPrune)
-      throws IOException {
-    List<ExtendedBlocklet> leftPrune = left.prune(segments, partitionsToPrune);
-    List<ExtendedBlocklet> rightPrune = right.prune(segments, partitionsToPrune);
-    List<ExtendedBlocklet> andBlocklets = new ArrayList<>();
-    for (ExtendedBlocklet blocklet : leftPrune) {
-      if (rightPrune.contains(blocklet)) {
-        andBlocklets.add(blocklet);
-      }
-    }
-    return andBlocklets;
+  public List<ExtendedBlocklet> prune(List<Segment> segments,
+      List<PartitionSpec> partitionsToPrune) throws IOException {
+    return and(left.prune(segments, partitionsToPrune), right.prune(segments, partitionsToPrune));
   }
 
   @Override
   public List<ExtendedBlocklet> prune(IndexInputSplit distributable,
-      List<PartitionSpec> partitionsToPrune)
-          throws IOException {
-    List<ExtendedBlocklet> leftPrune = left.prune(distributable, partitionsToPrune);
-    List<ExtendedBlocklet> rightPrune = right.prune(distributable, partitionsToPrune);
+      List<PartitionSpec> partitionsToPrune) throws IOException {
+    return and(left.prune(distributable, partitionsToPrune),
+        right.prune(distributable, partitionsToPrune));
+  }
+
+  private List<ExtendedBlocklet> and(List<ExtendedBlocklet> leftPrune,
+      List<ExtendedBlocklet> rightPrune) {

Review comment:
       done




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


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


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


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


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


   


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