From e87680c733e07d1e913b2194bb61fa9d85d932a5 Mon Sep 17 00:00:00 2001
From: chunhui-shi <cshi@maprtech.com>
Date: Wed, 25 May 2016 23:22:01 -0700
Subject: [PATCH] DRILL-4618: Fix hive function loader not correctly take
 random flag; and function visitor should not use previous function holder if
 this function is nondeterministic

---
 .../exec/expr/fn/HiveFunctionRegistry.java    | 22 ++++++++++++++++++-
 .../exec/fn/hive/TestInbuiltHiveUDFs.java     | 10 +++++++++
 .../drill/exec/expr/EqualityVisitor.java      |  3 +++
 .../drill/exec/expr/EvaluationVisitor.java    |  2 +-
 .../org/apache/drill/TestFunctionsQuery.java  | 11 ++++++++++
 5 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
index c716e9efdd3..8d8707e7207 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.expr.fn;
 
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.calcite.rel.type.RelDataType;
@@ -73,6 +74,25 @@ public HiveFunctionRegistry(DrillConfig config) {
     for (Class<? extends UDF> clazz : udfClasses) {
       register(clazz, methodsUDF);
     }
+
+    if (logger.isTraceEnabled()) {
+      StringBuilder allHiveFunctions = new StringBuilder();
+      for (Map.Entry<String, Class<? extends GenericUDF>> method : methodsGenericUDF.entries()) {
+        allHiveFunctions.append(method.toString()).append("\n");
+      }
+      logger.trace("Registered Hive GenericUDFs: [\n{}]", allHiveFunctions);
+
+      StringBuilder allUDFs = new StringBuilder();
+      for (Map.Entry<String, Class<? extends UDF>> method : methodsUDF.entries()) {
+        allUDFs.append(method.toString()).append("\n");
+      }
+      logger.trace("Registered Hive UDFs: [\n{}]", allUDFs);
+      StringBuilder allNonDeterministic = new StringBuilder();
+      for (Class<?> clz : nonDeterministicUDFs) {
+        allNonDeterministic.append(clz.toString()).append("\n");
+      }
+      logger.trace("Registered Hive nonDeterministicUDFs: [\n{}]", allNonDeterministic);
+    }
   }
 
   @Override
@@ -96,7 +116,7 @@ private <C,I> void register(Class<? extends I> clazz, ArrayListMultimap<String,C
     }
 
     UDFType type = clazz.getAnnotation(UDFType.class);
-    if (type != null && type.deterministic()) {
+    if (type != null && !type.deterministic()) {
       nonDeterministicUDFs.add(clazz);
     }
 
diff --git a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
index 9e13844ae41..0eb41169d2f 100644
--- a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
+++ b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
@@ -94,4 +94,14 @@ public void testIf() throws Exception {
         .go();
   }
 
+  @Test // DRILL-4618
+  public void testRand() throws Exception {
+    String query = "select 2*rand()=2*rand() col1 from (values (1))";
+    testBuilder()
+            .sqlQuery(query)
+            .unOrdered()
+            .baselineColumns("col1")
+            .baselineValues(false)
+            .go();
+  }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
index 7945bb461a3..433e95fd68a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
@@ -75,6 +75,9 @@ public Boolean visitFunctionHolderExpression(FunctionHolderExpression holder, Lo
     if (!holder.getName().equals(((FunctionHolderExpression) value).getName())) {
       return false;
     }
+    if (holder.isRandom()) {
+      return false;
+    }
     return checkChildren(holder, value);
   }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
index 055ab844c1f..53bd8b80b23 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
@@ -811,7 +811,7 @@ public HoldingContainer visitFunctionCall(FunctionCall call, ClassGenerator<?> g
     @Override
     public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression holder, ClassGenerator<?> generator) throws RuntimeException {
       HoldingContainer hc = getPrevious(holder, generator.getMappingSet());
-      if (hc == null) {
+      if (hc == null || holder.isRandom()) {
         hc = super.visitFunctionHolderExpression(holder, generator);
         put(holder, hc, generator.getMappingSet());
       }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
index 475d08a0e16..8be87819ee5 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
@@ -912,4 +912,15 @@ public void testConcatSingleInput() throws Exception {
         .baselineValues("foo")
         .go();
   }
+
+  @Test
+  public void testRandom() throws Exception {
+    String query = "select 2*random()=2*random() as col1 from (values (1))";
+    testBuilder()
+            .sqlQuery(query)
+            .unOrdered()
+            .baselineColumns("col1")
+            .baselineValues(false)
+            .go();
+  }
 }