You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2020/03/03 16:42:59 UTC

[GitHub] [drill] oleg-zinovev opened a new pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

oleg-zinovev opened a new pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009
 
 
   # [DRILL-7622](https://issues.apache.org/jira/browse/DRILL-7622): DRILL-7622: Compilation error when using HLL / TDigest with group by
   
   ## Description
   Fixes code generation for workspace variables injected from OperationContext
   
   ## Testing
   Appropriate unit test added (TestBugFixes#testDRILL7622)
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009#discussion_r387160620
 
 

 ##########
 File path: exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
 ##########
 @@ -316,4 +316,9 @@ public void testDRILL6318() throws Exception {
     rows = testSql("SELECT FLATTEN(data) AS d FROM cp.`jsoninput/bug6318.json` LIMIT 3 OFFSET 5");
     Assert.assertEquals(3, rows);
   }
+
+  @Test
+  public void testDRILL7622() throws Exception {
+    test("SELECT tdigest(p.col_int) FROM cp.`parquet/alltypes_required.parquet` p GROUP BY p.col_flt");
 
 Review comment:
   Please use `testBuilder()` to verify the correctness of the result.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009#discussion_r387160301
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
 ##########
 @@ -159,7 +160,11 @@ public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingConta
     for (int i = 0; i < getWorkspaceVars().length; i++) {
       if (getWorkspaceVars()[i].isInject()) {
         workspaceJVars[i] = g.declareClassField("work", g.getModel()._ref(getWorkspaceVars()[i].getType()));
-        g.getBlock(BlockType.SETUP).assign(workspaceJVars[i], g.getMappingSet().getIncoming().invoke("getContext").invoke("getManagedBuffer"));
+        g.getBlock(BlockType.SETUP).assign(
 
 Review comment:
   Could you please add checks similar to [`DrillFuncHolder`](https://github.com/apache/drill/blob/755529f3ac7ca77797f68b60e1d0713ad126e227/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java#L186) to ensure that the required variable should be injected, or even better - reformat code if possible to use common code for these two classes to inject variables in the same way?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009#discussion_r387160301
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
 ##########
 @@ -159,7 +160,11 @@ public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingConta
     for (int i = 0; i < getWorkspaceVars().length; i++) {
       if (getWorkspaceVars()[i].isInject()) {
         workspaceJVars[i] = g.declareClassField("work", g.getModel()._ref(getWorkspaceVars()[i].getType()));
-        g.getBlock(BlockType.SETUP).assign(workspaceJVars[i], g.getMappingSet().getIncoming().invoke("getContext").invoke("getManagedBuffer"));
+        g.getBlock(BlockType.SETUP).assign(
 
 Review comment:
   Could you please add checks similar to [`DrillFuncHolder`](https://github.com/apache/drill/blob/755529f3ac7ca77797f68b60e1d0713ad126e227/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java#L185) to ensure that the required variable should be injected, or even better - reformat code if possible to use common code for these two classes to inject variables in the same way?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] asfgit closed pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009#discussion_r387163842
 
 

 ##########
 File path: exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
 ##########
 @@ -316,4 +316,9 @@ public void testDRILL6318() throws Exception {
     rows = testSql("SELECT FLATTEN(data) AS d FROM cp.`jsoninput/bug6318.json` LIMIT 3 OFFSET 5");
     Assert.assertEquals(3, rows);
   }
+
+  @Test
+  public void testDRILL7622() throws Exception {
 
 Review comment:
   Please move this test into `org.apache.drill.exec.fn.impl.TestAggregateFunctions` class and update its name to describe the issue it checks.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [drill] vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2009: DRILL-7622: Compilation error when using HLL / TDigest with group by
URL: https://github.com/apache/drill/pull/2009#discussion_r387160301
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
 ##########
 @@ -159,7 +160,11 @@ public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingConta
     for (int i = 0; i < getWorkspaceVars().length; i++) {
       if (getWorkspaceVars()[i].isInject()) {
         workspaceJVars[i] = g.declareClassField("work", g.getModel()._ref(getWorkspaceVars()[i].getType()));
-        g.getBlock(BlockType.SETUP).assign(workspaceJVars[i], g.getMappingSet().getIncoming().invoke("getContext").invoke("getManagedBuffer"));
+        g.getBlock(BlockType.SETUP).assign(
 
 Review comment:
   Could you please add checks similar to [`DrillFuncHolder`](https://github.com/apache/drill/blob/755529f3ac7ca77797f68b60e1d0713ad126e227/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java#L186) to ensure that the required variable should be injected, or even better - refactor code to use common code for these two classes to inject variables in the same way.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services