[GitHub] [carbondata] akkio-97 opened a new pull request #3987: [WIP ]local-dictionary

[GitBox]

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


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


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


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


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


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


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


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


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



##########
File path: integration/presto/src/main/prestodb/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java
##########
@@ -139,7 +139,7 @@ public void putComplexObject(List<Integer> offsetVector) {
       Block rowBlock = RowBlock
           .fromFieldBlocks(childBlocks.get(0).getPositionCount(), Optional.empty(),
               childBlocks.toArray(new Block[0]));
-      for (int position = 0; position < childBlocks.get(0).getPositionCount(); position++) {
+      for (int position = 0; position < offsetVector.size(); position++) {

Review comment:
       missed this to handle at presto sql file, I wonder how the test case passed for prestosql




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



##########
File path: integration/presto/src/test/prestosql/org/apache/carbondata/presto/server/PrestoTestUtil.scala
##########
@@ -114,4 +114,60 @@ object PrestoTestUtil {
       }
     }
   }
+
+  // this method depends on prestodb jdbc PrestoArray class

Review comment:
       ```suggestion
     // this method depends on prestosql jdbc PrestoArray class
   ```




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

akkio-97 commented on a change in pull request #3987:
URL: https://github.com/apache/carbondata/pull/3987#discussion_r510812324



##########
File path: integration/presto/src/main/prestodb/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java
##########
@@ -139,7 +139,7 @@ public void putComplexObject(List<Integer> offsetVector) {
       Block rowBlock = RowBlock
           .fromFieldBlocks(childBlocks.get(0).getPositionCount(), Optional.empty(),
               childBlocks.toArray(new Block[0]));
-      for (int position = 0; position < childBlocks.get(0).getPositionCount(); position++) {
+      for (int position = 0; position < offsetVector.size(); position++) {

Review comment:
       We don't have that file in prestoDB profile




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



##########
File path: integration/presto/src/main/prestodb/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java
##########
@@ -139,7 +139,7 @@ public void putComplexObject(List<Integer> offsetVector) {
       Block rowBlock = RowBlock
           .fromFieldBlocks(childBlocks.get(0).getPositionCount(), Optional.empty(),
               childBlocks.toArray(new Block[0]));
-      for (int position = 0; position < childBlocks.get(0).getPositionCount(); position++) {
+      for (int position = 0; position < offsetVector.size(); position++) {

Review comment:
       please check agian. Both prestodb and prestosql has this class
   
   integration/presto/src/main/prestosql/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java
   integration/presto/src/main/prestodb/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java




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

akkio-97 commented on a change in pull request #3987:
URL: https://github.com/apache/carbondata/pull/3987#discussion_r510819729



##########
File path: integration/presto/src/main/prestodb/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java
##########
@@ -139,7 +139,7 @@ public void putComplexObject(List<Integer> offsetVector) {
       Block rowBlock = RowBlock
           .fromFieldBlocks(childBlocks.get(0).getPositionCount(), Optional.empty(),
               childBlocks.toArray(new Block[0]));
-      for (int position = 0; position < childBlocks.get(0).getPositionCount(); position++) {
+      for (int position = 0; position < offsetVector.size(); position++) {

Review comment:
       It had passed earlier because this is just refactoring. I have anyway added changes in other profile.




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

akkio-97 commented on a change in pull request #3987:
URL: https://github.com/apache/carbondata/pull/3987#discussion_r510819971



##########
File path: integration/presto/src/test/prestosql/org/apache/carbondata/presto/server/PrestoTestUtil.scala
##########
@@ -114,4 +114,60 @@ object PrestoTestUtil {
       }
     }
   }
+
+  // this method depends on prestodb jdbc PrestoArray class

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


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


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


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


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


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


   


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