You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by vo...@apache.org on 2020/03/10 18:41:33 UTC

[drill] 04/10: DRILL-7622: Compilation error when using HLL / TDigest with `group by` clause

This is an automated email from the ASF dual-hosted git repository.

volodymyr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 06dc001454220ddce33dc6ab9106966f4e6d18ae
Author: ozinoviev <oz...@solit-clouds.ru>
AuthorDate: Tue Mar 3 19:37:27 2020 +0300

    DRILL-7622: Compilation error when using HLL / TDigest with `group by` clause
    
    closes #2009
---
 .../drill/exec/expr/fn/DrillAggFuncHolder.java     |  2 +-
 .../apache/drill/exec/expr/fn/DrillFuncHolder.java | 30 ++++++++++++----------
 .../drill/exec/fn/impl/TestAggregateFunctions.java | 15 +++++++++++
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
index 9c11c89..992e032 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
@@ -159,7 +159,7 @@ class DrillAggFuncHolder extends DrillFuncHolder {
     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"));
+        assignInjectableValue(g, workspaceJVars[i], getWorkspaceVars()[i]);
       } else {
         Preconditions.checkState(Types.isFixedWidthType(getWorkspaceVars()[i].getMajorType()), String.format("Workspace variable '%s' in aggregation function '%s' is not allowed to " +
             "have variable length type.", getWorkspaceVars()[i].getName(), getRegisteredNames()[0]));
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
index 76e1e16..cf3c0ed 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
@@ -173,32 +173,34 @@ public abstract class DrillFuncHolder extends AbstractFuncHolder {
     for (int i = 0; i < attributes.getWorkspaceVars().length; i++) {
       WorkspaceReference ref = attributes.getWorkspaceVars()[i];
       JType jtype = g.getModel()._ref(ref.getType());
+      workspaceJVars[i] = g.declareClassField("work", jtype);
 
       if (ScalarReplacementTypes.CLASSES.contains(ref.getType())) {
-        workspaceJVars[i] = g.declareClassField("work", jtype);
         JBlock b = g.getBlock(SignatureHolder.DRILL_INIT_METHOD);
         b.assign(workspaceJVars[i], JExpr._new(jtype));
-      } else {
-        workspaceJVars[i] = g.declareClassField("work", jtype);
       }
 
       if (ref.isInject()) {
-        if (UdfUtilities.INJECTABLE_GETTER_METHODS.get(ref.getType()) != null) {
-          g.getBlock(BlockType.SETUP).assign(
-              workspaceJVars[i],
-              g.getMappingSet().getIncoming().invoke("getContext").invoke(
-                  UdfUtilities.INJECTABLE_GETTER_METHODS.get(ref.getType())
-              ));
-        } else {
-          // Invalid injectable type provided, this should have been caught in FunctionConverter
-          throw new DrillRuntimeException("Invalid injectable type requested in UDF: " +
-                ref.getType().getSimpleName());
-        }
+        assignInjectableValue(g, workspaceJVars[i], ref);
       }
     }
     return workspaceJVars;
   }
 
+  protected void assignInjectableValue(ClassGenerator<?> g, JVar variable, WorkspaceReference ref) {
+    if (UdfUtilities.INJECTABLE_GETTER_METHODS.get(ref.getType()) != null) {
+      g.getBlock(BlockType.SETUP).assign(
+          variable,
+          g.getMappingSet().getIncoming().invoke("getContext").invoke(
+              UdfUtilities.INJECTABLE_GETTER_METHODS.get(ref.getType())
+          ));
+    } else {
+      // Invalid injectable type provided, this should have been caught in FunctionConverter
+      throw new DrillRuntimeException("Invalid injectable type requested in UDF: " +
+              ref.getType().getSimpleName());
+    }
+  }
+
   /**
    * Generate the body of a Drill function by copying the source code of the
    * corresponding function method into the generated output. For this to work,
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java
index fcfd3fe..61e87f7 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java
@@ -1224,4 +1224,19 @@ public class TestAggregateFunctions extends ClusterTest {
       client.resetSession(PlannerSettings.STREAMAGG.getOptionName());
     }
   }
+
+  @Test
+  public void testInjectVariablesHashAgg() throws Exception {
+    try {
+      client.alterSession(PlannerSettings.STREAMAGG.getOptionName(), false);
+      testBuilder()
+          .sqlQuery("select tdigest(p.col_int) from " +
+              "cp.`parquet/alltypes_required.parquet` p group by p.col_flt")
+          .unOrdered()
+          .expectsNumRecords(4)
+          .go();
+    } finally {
+      client.resetSession(PlannerSettings.STREAMAGG.getOptionName());
+    }
+  }
 }