drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject [1/3] drill git commit: DRILL-2199: Fix major type returned for convert to nullable type functions
Date Thu, 12 Feb 2015 02:17:27 GMT
Repository: drill
Updated Branches:
  refs/heads/master 00c08eff2 -> b79f76619


DRILL-2199: Fix major type returned for convert to nullable type functions


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/9f9fb6c9
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/9f9fb6c9
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/9f9fb6c9

Branch: refs/heads/master
Commit: 9f9fb6c95b85c8c3c63d3aa8b1fc1daf90bae6f3
Parents: 21432de
Author: Mehant Baid <mehantr@gmail.com>
Authored: Mon Feb 9 18:01:23 2015 -0800
Committer: Mehant Baid <mehantr@gmail.com>
Committed: Wed Feb 11 17:00:29 2015 -0800

----------------------------------------------------------------------
 .../drill/common/util/CoreDecimalUtility.java   | 12 ++++
 .../templates/ConvertToNullableHolder.java      |  4 ++
 .../exec/expr/ExpressionTreeMaterializer.java   | 64 +++++++-------------
 .../expr/fn/DrillDecimalMaxScaleFuncHolder.java | 19 +++---
 .../org/apache/drill/TestFunctionsQuery.java    | 14 +++++
 5 files changed, 62 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/9f9fb6c9/common/src/main/java/org/apache/drill/common/util/CoreDecimalUtility.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/drill/common/util/CoreDecimalUtility.java b/common/src/main/java/org/apache/drill/common/util/CoreDecimalUtility.java
index c5cd8c0..302652e 100644
--- a/common/src/main/java/org/apache/drill/common/util/CoreDecimalUtility.java
+++ b/common/src/main/java/org/apache/drill/common/util/CoreDecimalUtility.java
@@ -73,4 +73,16 @@ public class CoreDecimalUtility {
 
     return (input.unscaledValue().intValue());
   }
+
+  /*
+   * Helper function to detect if the given data type is Decimal
+   */
+  public static boolean isDecimalType(TypeProtos.MajorType type) {
+    TypeProtos.MinorType minorType = type.getMinorType();
+    if (minorType == TypeProtos.MinorType.DECIMAL9 || minorType == TypeProtos.MinorType.DECIMAL18
||
+        minorType == TypeProtos.MinorType.DECIMAL28SPARSE || minorType == TypeProtos.MinorType.DECIMAL38SPARSE)
{
+      return true;
+    }
+    return false;
+  }
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/9f9fb6c9/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java b/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
index 89a400d..57efd5e 100644
--- a/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
+++ b/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
@@ -32,7 +32,11 @@ import org.apache.drill.exec.expr.annotations.*;
 import org.apache.drill.exec.expr.holders.*;
 import org.apache.drill.exec.record.RecordBatch;
 
+<#if minor.class.startsWith("Decimal")>
+@FunctionTemplate(name = "convertToNullable${minor.class?upper_case}", scope = FunctionTemplate.FunctionScope.DECIMAL_MAX_SCALE,
nulls = FunctionTemplate.NullHandling.INTERNAL)
+<#else>
 @FunctionTemplate(name = "convertToNullable${minor.class?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE,
nulls = FunctionTemplate.NullHandling.INTERNAL)
+</#if>
 public class ${className} implements DrillSimpleFunc {
 
   @Param ${minor.class}Holder input;

http://git-wip-us.apache.org/repos/asf/drill/blob/9f9fb6c9/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
index 8557765..e1ad854 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
@@ -58,6 +58,7 @@ import org.apache.drill.common.types.TypeProtos.DataMode;
 import org.apache.drill.common.types.TypeProtos.MajorType;
 import org.apache.drill.common.types.TypeProtos.MinorType;
 import org.apache.drill.common.types.Types;
+import org.apache.drill.common.util.CoreDecimalUtility;
 import org.apache.drill.exec.expr.annotations.FunctionTemplate;
 import org.apache.drill.exec.expr.fn.AbstractFuncHolder;
 import org.apache.drill.exec.expr.fn.DrillComplexWriterFuncHolder;
@@ -108,20 +109,19 @@ public class ExpressionTreeMaterializer {
 
     if (!Types.isFixedWidthType(toType)) {
 
-        /* We are implicitly casting to VARCHAR so we don't have a max length,
-         * using an arbitrary value. We trim down the size of the stored bytes
-         * to the actual size so this size doesn't really matter.
-         */
+      /* We are implicitly casting to VARCHAR so we don't have a max length,
+       * using an arbitrary value. We trim down the size of the stored bytes
+       * to the actual size so this size doesn't really matter.
+       */
       castArgs.add(new ValueExpressions.LongExpression(65536, null));
     }
-    else if (toType.getMinorType().name().startsWith("DECIMAL")) {
+    else if (CoreDecimalUtility.isDecimalType(toType)) {
       // Add the scale and precision to the arguments of the implicit cast
-      castArgs.add(new ValueExpressions.LongExpression(fromExpr.getMajorType().getPrecision(),
null));
-      castArgs.add(new ValueExpressions.LongExpression(fromExpr.getMajorType().getScale(),
null));
+      castArgs.add(new ValueExpressions.LongExpression(toType.getPrecision(), null));
+      castArgs.add(new ValueExpressions.LongExpression(toType.getScale(), null));
     }
-
     FunctionCall castCall = new FunctionCall(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
-    FunctionResolver resolver = FunctionResolverFactory.getResolver(castCall);
+    FunctionResolver resolver = FunctionResolverFactory.getExactResolver(castCall);
     DrillFuncHolder matchedCastFuncHolder = registry.findDrillFunction(resolver, castCall);
 
     if (matchedCastFuncHolder == null) {
@@ -241,7 +241,12 @@ public class ExpressionTreeMaterializer {
             argsWithCast.add(currentArg);
           } else {
             //Case 3: insert cast if param type is different from arg type.
-            argsWithCast.add(addCastExpression(call.args.get(i), parmType, registry, errorCollector));
+            if (CoreDecimalUtility.isDecimalType(parmType)) {
+              // We are implicitly promoting a decimal type, set the required scale and precision
+              parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
+                  setScale(currentArg.getMajorType().getScale()).setPrecision(currentArg.getMajorType().getPrecision()).build();
+            }
+            argsWithCast.add(addCastExpression(currentArg, parmType, registry, errorCollector));
           }
         }
 
@@ -262,6 +267,11 @@ public class ExpressionTreeMaterializer {
             extArgsWithCast.add(currentArg);
           } else {
             // Insert cast if param type is different from arg type.
+            if (CoreDecimalUtility.isDecimalType(parmType)) {
+              // We are implicitly promoting a decimal type, set the required scale and precision
+              parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
+                  setScale(currentArg.getMajorType().getScale()).setPrecision(currentArg.getMajorType().getPrecision()).build();
+            }
             extArgsWithCast.add(addCastExpression(call.args.get(i), parmType, registry, errorCollector));
           }
         }
@@ -273,38 +283,6 @@ public class ExpressionTreeMaterializer {
       return NullExpression.INSTANCE;
     }
 
-    public static LogicalExpression addCastExpression(LogicalExpression fromExpr, MajorType
toType, FunctionImplementationRegistry registry, ErrorCollector errorCollector) {
-      String castFuncName = CastFunctions.getCastFunc(toType.getMinorType());
-      List<LogicalExpression> castArgs = Lists.newArrayList();
-      castArgs.add(fromExpr);  //input_expr
-
-      if (!Types.isFixedWidthType(toType)) {
-
-        /* We are implicitly casting to VARCHAR so we don't have a max length,
-         * using an arbitrary value. We trim down the size of the stored bytes
-         * to the actual size so this size doesn't really matter.
-         */
-        castArgs.add(new ValueExpressions.LongExpression(65536, null));
-      }
-      else if (toType.getMinorType().name().startsWith("DECIMAL")) {
-        // Add the scale and precision to the arguments of the implicit cast
-        castArgs.add(new ValueExpressions.LongExpression(fromExpr.getMajorType().getPrecision(),
null));
-        castArgs.add(new ValueExpressions.LongExpression(fromExpr.getMajorType().getScale(),
null));
-      }
-
-      FunctionCall castCall = new FunctionCall(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
-      FunctionResolver resolver = FunctionResolverFactory.getExactResolver(castCall);
-      DrillFuncHolder matchedCastFuncHolder = registry.findDrillFunction(resolver, castCall);
-
-      if (matchedCastFuncHolder == null) {
-        logFunctionResolutionError(errorCollector, castCall);
-        return NullExpression.INSTANCE;
-      }
-
-      return matchedCastFuncHolder.getExpr(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
-
-    }
-
     @Override
     public LogicalExpression visitIfExpression(IfExpression ifExpr, FunctionImplementationRegistry
registry) {
       IfExpression.IfCondition conditions = ifExpr.ifCondition;
@@ -546,7 +524,7 @@ public class ExpressionTreeMaterializer {
         //VarLen type
         if (!Types.isFixedWidthType(type)) {
           newArgs.add(new ValueExpressions.LongExpression(type.getWidth(), null));
-        } else if (type.getMinorType().name().startsWith("DECIMAL")) {
+        }  if (CoreDecimalUtility.isDecimalType(type)) {
             newArgs.add(new ValueExpressions.LongExpression(type.getPrecision(), null));
             newArgs.add(new ValueExpressions.LongExpression(type.getScale(), null));
         }

http://git-wip-us.apache.org/repos/asf/drill/blob/9f9fb6c9/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java
index 7e54586..aa8e2b5 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java
@@ -39,19 +39,22 @@ public class DrillDecimalMaxScaleFuncHolder extends DrillSimpleFuncHolder{
     public MajorType getReturnType(List<LogicalExpression> args) {
 
         TypeProtos.DataMode mode = returnValue.type.getMode();
+        boolean nullInput = false;
         int scale = 0;
         int precision = 0;
 
-        if (nullHandling == NullHandling.NULL_IF_NULL) {
-            // if any one of the input types is nullable, then return nullable return type
-            for (LogicalExpression e : args) {
-                if (e.getMajorType().getMode() == TypeProtos.DataMode.OPTIONAL) {
-                    mode = TypeProtos.DataMode.OPTIONAL;
-                }
-                scale = Math.max(scale, e.getMajorType().getScale());
-                precision = Math.max(precision, e.getMajorType().getPrecision());
+        for (LogicalExpression e : args) {
+            if (e.getMajorType().getMode() == TypeProtos.DataMode.OPTIONAL) {
+                nullInput = true;
             }
+            scale = Math.max(scale, e.getMajorType().getScale());
+            precision = Math.max(precision, e.getMajorType().getPrecision());
         }
+
+        if (nullHandling == NullHandling.NULL_IF_NULL && nullInput) {
+            mode = TypeProtos.DataMode.OPTIONAL;
+        }
+
         return (TypeProtos.MajorType.newBuilder().setMinorType(returnValue.type.getMinorType()).setScale(scale).setPrecision(precision).setMode(mode).build());
     }
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/9f9fb6c9/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
----------------------------------------------------------------------
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 7356640..f1005ab 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
@@ -695,4 +695,18 @@ public class TestFunctionsQuery extends BaseTestQuery {
         .baselineValues(time1, time2, ts1, ts2, date1, date2)
         .go();
   }
+
+  @Test
+  public void testCaseWithDecimalExpressions() throws Exception {
+    String query = "select " +
+        "case when true then cast(employee_id as decimal(15, 5)) else cast('0.0' as decimal(2,
1)) end as col1 " +
+        "from cp.`employee.json` where employee_id = 1";
+
+    testBuilder()
+        .sqlQuery(query)
+        .unOrdered()
+        .baselineColumns("col1")
+        .baselineValues(new BigDecimal("1.00000"))
+        .go();
+  }
 }


Mime
View raw message