drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject [1/2] incubator-drill git commit: DRILL-1663: Fix casting to a variable width type implicitly in a join condition
Date Mon, 10 Nov 2014 23:15:06 GMT
Repository: incubator-drill
Updated Branches:
  refs/heads/master e2ad5b038 -> d5b9b11c9


DRILL-1663: Fix casting to a variable width type implicitly in a join condition

Factor out logic in ExpressionTreeMaterializer so the same logic can be reused in ChainedHashTable.


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

Branch: refs/heads/master
Commit: 3430f8112376932c96b4023c666eff2604f00223
Parents: e2ad5b0
Author: Mehant Baid <mehantr@gmail.com>
Authored: Fri Nov 7 13:55:48 2014 -0800
Committer: Mehant Baid <mehantr@gmail.com>
Committed: Mon Nov 10 14:01:40 2014 -0800

----------------------------------------------------------------------
 .../exec/expr/ExpressionTreeMaterializer.java   | 146 +++++++++++--------
 .../physical/impl/common/ChainedHashTable.java  |  25 ++--
 2 files changed, 102 insertions(+), 69 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/3430f811/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 2a5afce..9cec3a4 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
@@ -101,6 +101,61 @@ public class ExpressionTreeMaterializer {
     }
   }
 
+  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.getResolver(castCall);
+    DrillFuncHolder matchedCastFuncHolder = registry.findDrillFunction(resolver, castCall);
+
+    if (matchedCastFuncHolder == null) {
+      logFunctionResolutionError(errorCollector, castCall);
+      return NullExpression.INSTANCE;
+    }
+    return matchedCastFuncHolder.getExpr(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
+  }
+
+  private static void logFunctionResolutionError(ErrorCollector errorCollector, FunctionCall
call) {
+    // add error to collector
+    StringBuilder sb = new StringBuilder();
+    sb.append("Missing function implementation: ");
+    sb.append("[");
+    sb.append(call.getName());
+    sb.append("(");
+    boolean first = true;
+    for(LogicalExpression e : call.args) {
+      TypeProtos.MajorType mt = e.getMajorType();
+      if (first) {
+        first = false;
+      } else {
+        sb.append(", ");
+      }
+      sb.append(mt.getMinorType().name());
+      sb.append("-");
+      sb.append(mt.getMode().name());
+    }
+    sb.append(")");
+    sb.append("]");
+
+    errorCollector.addGeneralError(call.getPosition(), sb.toString());
+  }
+
   private static class MaterializeVisitor extends AbstractExprVisitor<LogicalExpression,
FunctionImplementationRegistry, RuntimeException> {
     private ExpressionValidator validator = new ExpressionValidator();
     private final ErrorCollector errorCollector;
@@ -143,38 +198,7 @@ public class ExpressionTreeMaterializer {
       return new BooleanOperator(op.getName(), args, op.getPosition());
     }
 
-    private LogicalExpression addCastExpression(LogicalExpression fromExpr, MajorType toType,
FunctionImplementationRegistry registry) {
-      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.getResolver(castCall);
-      DrillFuncHolder matchedCastFuncHolder = registry.findDrillFunction(resolver, castCall);
-
-      if (matchedCastFuncHolder == null) {
-        logFunctionResolutionError(errorCollector, castCall);
-        return NullExpression.INSTANCE;
-      }
-
-      return matchedCastFuncHolder.getExpr(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
-
-    }
-    @Override
+   @Override
     public LogicalExpression visitFunctionCall(FunctionCall call, FunctionImplementationRegistry
registry) {
       List<LogicalExpression> args = Lists.newArrayList();
       for (int i = 0; i < call.args.size(); ++i) {
@@ -217,7 +241,7 @@ 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));
+            argsWithCast.add(addCastExpression(call.args.get(i), parmType, registry, errorCollector));
           }
         }
 
@@ -238,7 +262,7 @@ public class ExpressionTreeMaterializer {
             extArgsWithCast.add(currentArg);
           } else {
             // Insert cast if param type is different from arg type.
-            extArgsWithCast.add(addCastExpression(call.args.get(i), parmType, registry));
+            extArgsWithCast.add(addCastExpression(call.args.get(i), parmType, registry, errorCollector));
           }
         }
 
@@ -249,31 +273,37 @@ public class ExpressionTreeMaterializer {
       return NullExpression.INSTANCE;
     }
 
-    private void logFunctionResolutionError(ErrorCollector errorCollector, FunctionCall call)
{
-      // add error to collector
-      StringBuilder sb = new StringBuilder();
-      sb.append("Missing function implementation: ");
-      sb.append("[");
-      sb.append(call.getName());
-      sb.append("(");
-      boolean first = true;
-      for(LogicalExpression e : call.args) {
-        TypeProtos.MajorType mt = e.getMajorType();
-        if (first) {
-          first = false;
-        } else {
-          sb.append(", ");
-        }
-        sb.append(mt.getMinorType().name());
-        sb.append("-");
-        sb.append(mt.getMode().name());
+    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));
       }
-      sb.append(")");
-      sb.append("]");
 
-      errorCollector.addGeneralError(call.getPosition(), sb.toString());
-    }
+      FunctionCall castCall = new FunctionCall(castFuncName, castArgs, ExpressionPosition.UNKNOWN);
+      FunctionResolver resolver = FunctionResolverFactory.getResolver(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) {
@@ -294,10 +324,10 @@ public class ExpressionTreeMaterializer {
         if (leastRestrictive != thenType) {
           // Implicitly cast the then expression
           conditions = new IfExpression.IfCondition(newCondition,
-              addCastExpression(conditions.expression, newElseExpr.getMajorType(), registry));
+          addCastExpression(conditions.expression, newElseExpr.getMajorType(), registry,
errorCollector));
         } else if (leastRestrictive != elseType) {
           // Implicitly cast the else expression
-          newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(),
registry);
+          newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(),
registry, errorCollector);
         } else {
           /* Cannot cast one of the two expressions to make the output type of if and else
expression
            * to be the same. Raise error.

http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/3430f811/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
index 792b7ab..4b80781 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
@@ -27,6 +27,7 @@ import org.apache.drill.common.expression.LogicalExpression;
 import org.apache.drill.common.expression.FunctionCall;
 import org.apache.drill.common.expression.ExpressionPosition;
 import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.expression.ValueExpressions;
 import org.apache.drill.common.logical.data.NamedExpression;
 import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.common.types.TypeProtos.MinorType;
@@ -287,8 +288,10 @@ public class ChainedHashTable {
     assert keyExprsBuild.length == keyExprsProbe.length;
 
     for (int i = 0; i < keyExprsBuild.length; i++) {
-      MinorType buildType = keyExprsBuild[i].getMajorType().getMinorType();
-      MinorType probeType = keyExprsProbe[i].getMajorType().getMinorType();
+      LogicalExpression buildExpr = keyExprsBuild[i];
+      LogicalExpression probeExpr = keyExprsProbe[i];
+      MinorType buildType = buildExpr.getMajorType().getMinorType();
+      MinorType probeType = probeExpr.getMajorType().getMinorType();
 
       if (buildType != probeType) {
         // We need to add a cast to one of the expressions
@@ -296,22 +299,22 @@ public class ChainedHashTable {
         types.add(buildType);
         types.add(probeType);
         MinorType result = TypeCastRules.getLeastRestrictiveType(types);
+        ErrorCollector errorCollector = new ErrorCollectorImpl();
 
-        // Add the cast
-        List<LogicalExpression> args = new LinkedList<>();
         if (result == null) {
           throw new DrillRuntimeException(String.format("Join conditions cannot be compared
failing build expression: %s failing probe expression: %s",
-              keyExprsBuild[i].getMajorType().toString(), keyExprsProbe[i].getMajorType().toString()));
+              buildExpr.getMajorType().toString(), probeExpr.getMajorType().toString()));
         }
         else if (result != buildType) {
           // Add a cast expression on top of the build expression
-          args.add(keyExprsBuild[i]);
-          FunctionCall castCall = new FunctionCall("cast" + result.toString().toUpperCase(),
args, ExpressionPosition.UNKNOWN);
-          keyExprsBuild[i] = ExpressionTreeMaterializer.materialize(castCall, incomingBuild,
new ErrorCollectorImpl(), context.getFunctionRegistry());
+          LogicalExpression castExpr = ExpressionTreeMaterializer.addCastExpression(buildExpr,
probeExpr.getMajorType(), context.getFunctionRegistry(), errorCollector);
+          // Store the newly casted expression
+          keyExprsBuild[i] = ExpressionTreeMaterializer.materialize(castExpr, incomingBuild,
errorCollector, context.getFunctionRegistry());
         } else if (result != probeType) {
-          args.add(keyExprsProbe[i]);
-          FunctionCall castCall = new FunctionCall("cast" + result.toString().toUpperCase(),
args, ExpressionPosition.UNKNOWN);
-          keyExprsProbe[i] = ExpressionTreeMaterializer.materialize(castCall, incomingProbe,
new ErrorCollectorImpl(), context.getFunctionRegistry());
+          // Add a cast expression on top of the probe expression
+          LogicalExpression castExpr = ExpressionTreeMaterializer.addCastExpression(probeExpr,
buildExpr.getMajorType(), context.getFunctionRegistry(), errorCollector);
+          // store the newly casted expression
+          keyExprsProbe[i] = ExpressionTreeMaterializer.materialize(castExpr, incomingProbe,
errorCollector, context.getFunctionRegistry());
         }
       }
     }


Mime
View raw message