You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by me...@apache.org on 2014/11/11 00:15:06 UTC
[1/2] incubator-drill git commit: DRILL-1663: Fix casting to a
variable width type implicitly in a join condition
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 <me...@gmail.com>
Authored: Fri Nov 7 13:55:48 2014 -0800
Committer: Mehant Baid <me...@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());
}
}
}
[2/2] incubator-drill git commit: DRILL-1631: Bump optiq version to
r8 Add test case. Fix is in optiq.
Posted by me...@apache.org.
DRILL-1631: Bump optiq version to r8
Add test case. Fix is in optiq.
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/d5b9b11c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/d5b9b11c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/d5b9b11c
Branch: refs/heads/master
Commit: d5b9b11c9a2337ac1f9c16784c2ae87ead7d1fd9
Parents: 3430f81
Author: Mehant Baid <me...@gmail.com>
Authored: Thu Nov 6 11:28:27 2014 -0800
Committer: Mehant Baid <me...@gmail.com>
Committed: Mon Nov 10 14:02:59 2014 -0800
----------------------------------------------------------------------
.../src/test/java/org/apache/drill/TestExampleQueries.java | 7 +++++++
pom.xml | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/d5b9b11c/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
index 9eaa20e..a328ade 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
@@ -530,4 +530,11 @@ public class TestExampleQueries extends BaseTestQuery{
expectedRecordCount, actualRecordCount), expectedRecordCount, actualRecordCount);
}
+ @Test
+ public void testImplicitDownwardCast() throws Exception {
+ int actualRecordCount = testSql("select o_totalprice from cp.`tpch/orders.parquet` where o_orderkey=60000 and o_totalprice=299402");
+ int expectedRecordCount = 0;
+ assertEquals(String.format("Received unexepcted number of rows in output: expected=%d, received=%s",
+ expectedRecordCount, actualRecordCount), expectedRecordCount, actualRecordCount);
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/d5b9b11c/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 25ca6a2..26189fd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -902,7 +902,7 @@
<dependency>
<groupId>net.hydromatic</groupId>
<artifactId>optiq-core</artifactId>
- <version>0.9-drill-r6</version>
+ <version>0.9-drill-r8</version>
<exclusions>
<exclusion>
<groupId>org.jgrapht</groupId>