DRILL-665: Handle null values in case expressions. Signed-off-by: vkorukanti <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/2cdbd600 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/2cdbd600 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/2cdbd600 Branch: refs/heads/master Commit: 2cdbd6000abc33965f1f7b960b954fd6a4f7b58f Parents: 1012784 Author: tnachen <[email protected]> Authored: Tue Feb 18 14:45:22 2014 -0800 Committer: Jacques Nadeau <[email protected]> Committed: Wed May 28 15:13:06 2014 -0700 ---------------------------------------------------------------------- .../expression/ExpressionStringBuilder.java | 11 ++++ .../drill/common/expression/NullExpression.java | 57 +++++++++++++++++ .../common/expression/TypedNullConstant.java | 2 +- .../visitors/AbstractExprVisitor.java | 12 ++++ .../expression/visitors/AggregateChecker.java | 11 ++++ .../expression/visitors/ConstantChecker.java | 12 ++++ .../common/expression/visitors/ExprVisitor.java | 5 +- .../visitors/ExpressionValidator.java | 18 +++++- .../sig/ConstantExpressionIdentifier.java | 12 ++++ .../drill/exec/expr/EvaluationVisitor.java | 6 +- .../exec/expr/ExpressionTreeMaterializer.java | 66 +++++++++++++++++++- .../drill/exec/planner/logical/DrillOptiq.java | 52 +++++++++------ .../drill/exec/record/NullExpression.java | 59 ----------------- 13 files changed, 237 insertions(+), 86 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java b/common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java index 8026cdb..4e9807f 100644 --- a/common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java +++ b/common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java @@ -303,4 +303,15 @@ public class ExpressionStringBuilder extends AbstractExprVisitor<Void, StringBui return null; } + @Override + public Void visitNullConstant(TypedNullConstant e, StringBuilder sb) throws RuntimeException { + sb.append("NULL"); + return null; + } + + @Override + public Void visitNullExpression(NullExpression e, StringBuilder sb) throws RuntimeException { + sb.append("NULL"); + return null; + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/NullExpression.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/NullExpression.java b/common/src/main/java/org/apache/drill/common/expression/NullExpression.java new file mode 100644 index 0000000..c39e06a --- /dev/null +++ b/common/src/main/java/org/apache/drill/common/expression/NullExpression.java @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.common.expression; + +import java.util.Iterator; + +import org.apache.drill.common.expression.ExpressionPosition; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.visitors.ExprVisitor; +import org.apache.drill.common.types.TypeProtos.MajorType; +import org.apache.drill.common.types.TypeProtos.MinorType; +import org.apache.drill.common.types.Types; + +import com.google.common.collect.Iterators; + +public class NullExpression implements LogicalExpression{ + static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(NullExpression.class); + + public static final NullExpression INSTANCE = new NullExpression(); + + final MajorType t = Types.optional(MinorType.NULL); + + @Override + public MajorType getMajorType() { + return t; + } + + @Override + public <T, V, E extends Exception> T accept(ExprVisitor<T, V, E> visitor, V value) throws E { + return visitor.visitNullExpression(this, value); + } + + @Override + public ExpressionPosition getPosition() { + return ExpressionPosition.UNKNOWN; + } + + @Override + public Iterator<LogicalExpression> iterator() { + return Iterators.emptyIterator(); + } +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java b/common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java index 1efb029..8b3a93f 100644 --- a/common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java +++ b/common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java @@ -44,7 +44,7 @@ public class TypedNullConstant extends LogicalExpressionBase { @Override public <T, V, E extends Exception> T accept(ExprVisitor<T, V, E> visitor, V value) throws E { - return visitor.visitUnknown(this, value); + return visitor.visitNullConstant(this, value); } @Override http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java b/common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java index 526275f..0dd6697 100644 --- a/common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java +++ b/common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java @@ -23,7 +23,9 @@ import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.FloatExpression; import org.apache.drill.common.expression.ValueExpressions.IntExpression; @@ -150,6 +152,16 @@ public abstract class AbstractExprVisitor<T, VAL, EXCEP extends Exception> imple } @Override + public T visitNullConstant(TypedNullConstant e, VAL value) throws EXCEP { + return visitUnknown(e, value); + } + + @Override + public T visitNullExpression(NullExpression e, VAL value) throws EXCEP { + return visitUnknown(e, value); + } + + @Override public T visitUnknown(LogicalExpression e, VAL value) throws EXCEP { throw new UnsupportedOperationException(String.format("Expression of type %s not handled by visitor type %s.", e.getClass().getCanonicalName(), this.getClass().getCanonicalName())); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java b/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java index bf67a6b..48ab3b4 100644 --- a/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java +++ b/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java @@ -25,7 +25,9 @@ import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.DoubleExpression; import org.apache.drill.common.expression.ValueExpressions.LongExpression; @@ -177,4 +179,13 @@ public final class AggregateChecker implements ExprVisitor<Boolean, ErrorCollect return false; } + @Override + public Boolean visitNullConstant(TypedNullConstant e, ErrorCollector value) throws RuntimeException { + return false; + } + + @Override + public Boolean visitNullExpression(NullExpression e, ErrorCollector value) throws RuntimeException { + return false; + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java b/common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java index ab94987..2b136a0 100644 --- a/common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java +++ b/common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java @@ -25,7 +25,9 @@ import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.DoubleExpression; import org.apache.drill.common.expression.ValueExpressions.FloatExpression; @@ -180,4 +182,14 @@ final class ConstantChecker implements ExprVisitor<Boolean, ErrorCollector, Runt public Boolean visitConvertExpression(ConvertExpression e, ErrorCollector value) throws RuntimeException { return e.getInput().accept(this, value); } + + @Override + public Boolean visitNullConstant(TypedNullConstant e, ErrorCollector value) throws RuntimeException { + return true; + } + + @Override + public Boolean visitNullExpression(NullExpression e, ErrorCollector value) throws RuntimeException { + return true; + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java b/common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java index 799c9dd..d56a16a 100644 --- a/common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java +++ b/common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java @@ -23,7 +23,9 @@ import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.DoubleExpression; import org.apache.drill.common.expression.ValueExpressions.FloatExpression; @@ -40,7 +42,6 @@ import org.apache.drill.common.expression.ValueExpressions.Decimal28Expression; import org.apache.drill.common.expression.ValueExpressions.Decimal38Expression; import org.apache.drill.common.expression.ValueExpressions.QuotedString; - public interface ExprVisitor<T, VAL, EXCEP extends Exception> { public T visitFunctionCall(FunctionCall call, VAL value) throws EXCEP; public T visitFunctionHolderExpression(FunctionHolderExpression holder, VAL value) throws EXCEP; @@ -61,6 +62,8 @@ public interface ExprVisitor<T, VAL, EXCEP extends Exception> { public T visitDoubleConstant(DoubleExpression dExpr, VAL value) throws EXCEP; public T visitBooleanConstant(BooleanExpression e, VAL value) throws EXCEP; public T visitQuotedStringConstant(QuotedString e, VAL value) throws EXCEP; + public T visitNullConstant(TypedNullConstant e, VAL value) throws EXCEP; + public T visitNullExpression(NullExpression e, VAL value) throws EXCEP; public T visitUnknown(LogicalExpression e, VAL value) throws EXCEP; public T visitCastExpression(CastExpression e, VAL value) throws EXCEP; public T visitConvertExpression(ConvertExpression e, VAL value) throws EXCEP; http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java b/common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java index e9bd03a..c8b7857 100644 --- a/common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java +++ b/common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java @@ -24,7 +24,9 @@ import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions; import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; @@ -94,13 +96,15 @@ public class ExpressionValidator implements ExprVisitor<Void, ErrorCollector, Ru for (IfCondition c : ifExpr.conditions) { MajorType innerT = c.expression.getMajorType(); if ((innerT.getMode() == DataMode.REPEATED && mt.getMode() != DataMode.REPEATED) || // - (innerT.getMinorType() != mt.getMinorType())) { + ((innerT.getMinorType() != mt.getMinorType()) && + (innerT.getMode() != DataMode.OPTIONAL && mt.getMode() != DataMode.OPTIONAL && + (innerT.getMinorType() != MinorType.NULL && mt.getMinorType() != MinorType.NULL)))) { errors .addGeneralError( c.condition.getPosition(), String .format( - "Failure composing If Expression. All expressions must return the same MajorType as the else expression. The %d if condition returned type type %s but the else expression was of type %s", + "Failure composing If Expression. All expressions must return the same MinorType as the else expression. The %d if condition returned type type %s but the else expression was of type %s", i, innerT, mt)); } i++; @@ -199,6 +203,16 @@ public class ExpressionValidator implements ExprVisitor<Void, ErrorCollector, Ru } @Override + public Void visitNullConstant(TypedNullConstant e, ErrorCollector value) throws RuntimeException { + return null; + } + + @Override + public Void visitNullExpression(NullExpression e, ErrorCollector value) throws RuntimeException { + return null; + } + + @Override public Void visitConvertExpression(ConvertExpression e, ErrorCollector value) throws RuntimeException { return e.getInput().accept(this, value); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java index 8a1efdf..489f623 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java @@ -28,7 +28,9 @@ import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.DoubleExpression; @@ -204,6 +206,16 @@ public class ConstantExpressionIdentifier implements ExprVisitor<Boolean, Identi } @Override + public Boolean visitNullConstant(TypedNullConstant e, IdentityHashMap<LogicalExpression, Object> value) throws RuntimeException { + return true; + } + + @Override + public Boolean visitNullExpression(NullExpression e, IdentityHashMap<LogicalExpression, Object> value) throws RuntimeException { + return true; + } + + @Override public Boolean visitConvertExpression(ConvertExpression e, IdentityHashMap<LogicalExpression, Object> value) throws RuntimeException { return e.getInput().accept(this, value); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---------------------------------------------------------------------- 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 dcc273c..1c012ab 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 @@ -29,6 +29,7 @@ import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.LogicalExpression; import org.apache.drill.common.expression.PathSegment; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions.BooleanExpression; import org.apache.drill.common.expression.ValueExpressions.DateExpression; @@ -55,7 +56,6 @@ import org.apache.drill.exec.expr.fn.DrillFuncHolder; import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry; import org.apache.drill.exec.expr.fn.HiveFuncHolder; import org.apache.drill.exec.physical.impl.filter.ReturnValueExpression; -import org.apache.drill.exec.record.NullExpression; import org.apache.drill.exec.vector.ValueHolderHelper; import org.apache.drill.exec.vector.complex.reader.FieldReader; import org.apache.drill.exec.vector.complex.writer.FieldWriter; @@ -160,7 +160,7 @@ public class EvaluationVisitor { HoldingContainer thenExpr = c.expression.accept(this, generator); if (thenExpr.isOptional()) { - JConditional newCond = jc._then()._if(thenExpr.getIsSet()); + JConditional newCond = jc._then()._if(thenExpr.getIsSet().ne(JExpr.lit(0))); JBlock b = newCond._then(); b.assign(output.getHolder(), thenExpr.getHolder()); b.assign(output.getIsSet(), thenExpr.getIsSet()); @@ -172,7 +172,7 @@ public class EvaluationVisitor { HoldingContainer elseExpr = ifExpr.elseExpression.accept(this, generator); if (elseExpr.isOptional()) { - JConditional newCond = jc._else()._if(elseExpr.getIsSet()); + JConditional newCond = jc._else()._if(elseExpr.getIsSet().ne(JExpr.lit(0))); JBlock b = newCond._then(); b.assign(output.getHolder(), elseExpr.getHolder()); b.assign(output.getIsSet(), elseExpr.getIsSet()); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/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 6297148..50ddb75 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 @@ -19,7 +19,12 @@ package org.apache.drill.exec.expr; import java.util.List; +import com.google.common.base.Function; import com.google.common.base.Joiner; +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import org.apache.drill.common.expression.CastExpression; import org.apache.drill.common.expression.ConvertExpression; import org.apache.drill.common.expression.ErrorCollector; @@ -27,7 +32,9 @@ import org.apache.drill.common.expression.ExpressionPosition; import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.expression.IfExpression; +import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.PathSegment; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.common.expression.TypedNullConstant; @@ -51,6 +58,7 @@ import org.apache.drill.common.expression.fn.CastFunctions; import org.apache.drill.common.expression.visitors.AbstractExprVisitor; import org.apache.drill.common.expression.visitors.ExpressionValidator; import org.apache.drill.common.types.TypeProtos; +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; @@ -58,7 +66,6 @@ import org.apache.drill.exec.expr.annotations.FunctionTemplate; import org.apache.drill.exec.expr.fn.DrillFuncHolder; import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry; import org.apache.drill.exec.expr.fn.HiveFuncHolder; -import org.apache.drill.exec.record.NullExpression; import org.apache.drill.exec.record.TypedFieldId; import org.apache.drill.exec.record.VectorAccessible; import org.apache.drill.exec.resolver.FunctionResolver; @@ -233,9 +240,61 @@ public class ExpressionTreeMaterializer { conditions.set(i, new IfExpression.IfCondition(newCondition, newExpr)); } + // Resolve NullExpression into TypedNullConstant by visiting all conditions + // We need to do this because we want to give the correct MajorType to the Null constant + Iterable<LogicalExpression> logicalExprs = Iterables.transform(conditions, + new Function<IfCondition, LogicalExpression>() { + @Override + public LogicalExpression apply(IfExpression.IfCondition input) { + return input.expression; + } + } + ); + + List<LogicalExpression> allExpressions = Lists.newArrayList(logicalExprs); + allExpressions.add(newElseExpr); + + boolean containsNullExpr = Iterables.any(allExpressions, new Predicate<LogicalExpression>() { + @Override + public boolean apply(LogicalExpression input) { + return input instanceof NullExpression; + } + }); + + if (containsNullExpr) { + Optional<LogicalExpression> nonNullExpr = Iterables.tryFind(allExpressions, + new Predicate<LogicalExpression>() { + @Override + public boolean apply(LogicalExpression input) { + return !input.getMajorType().getMinorType().equals(TypeProtos.MinorType.NULL); + } + } + ); + + if(nonNullExpr.isPresent()) { + MajorType type = nonNullExpr.get().getMajorType(); + for (int i = 0; i < conditions.size(); ++i) { + IfExpression.IfCondition condition = conditions.get(i); + conditions.set(i, + new IfExpression.IfCondition(condition.condition, rewriteNullExpression(condition.expression, type)) + ); + } + + newElseExpr = rewriteNullExpression(newElseExpr, type); + } + } + return validateNewExpr(IfExpression.newBuilder().setElse(newElseExpr).addConditions(conditions).build()); } + private LogicalExpression rewriteNullExpression(LogicalExpression expr, MajorType type) { + if(expr instanceof NullExpression) { + return new TypedNullConstant(type); + } else { + return expr; + } + } + @Override public LogicalExpression visitSchemaPath(SchemaPath path, FunctionImplementationRegistry value) { // logger.debug("Visiting schema path {}", path); @@ -280,6 +339,11 @@ public class ExpressionTreeMaterializer { } @Override + public LogicalExpression visitNullConstant(TypedNullConstant nullConstant, FunctionImplementationRegistry value) throws RuntimeException { + return nullConstant; + } + + @Override public LogicalExpression visitIntervalYearConstant(IntervalYearExpression intExpr, FunctionImplementationRegistry registry) { return intExpr; } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java index 1e0f7ea..a77dd6f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java @@ -27,15 +27,15 @@ import org.apache.drill.common.expression.FunctionCallFactory; import org.apache.drill.common.expression.IfExpression; import org.apache.drill.common.expression.IfExpression.IfCondition; import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.NullExpression; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.TypedNullConstant; import org.apache.drill.common.expression.ValueExpressions; import org.apache.drill.common.expression.ValueExpressions.QuotedString; import org.apache.drill.common.types.TypeProtos.MajorType; import org.apache.drill.common.types.TypeProtos; import org.apache.drill.common.types.TypeProtos.MinorType; import org.apache.drill.common.types.Types; -import org.apache.drill.exec.expr.EvaluationVisitor; -import org.apache.drill.exec.record.NullExpression; import org.eigenbase.rel.RelNode; import org.eigenbase.reltype.RelDataTypeField; import org.eigenbase.rex.RexCall; @@ -339,20 +339,20 @@ public class DrillOptiq { switch(literal.getType().getSqlTypeName()){ case BIGINT: long l = ((BigDecimal) literal.getValue()).longValue(); - return ValueExpressions.getBigInt(l); + return checkNullLiteral(literal, MinorType.BIGINT, ValueExpressions.getBigInt(l)); case BOOLEAN: - return ValueExpressions.getBit(((Boolean) literal.getValue())); + return checkNullLiteral(literal, MinorType.BIT, ValueExpressions.getBit(((Boolean) literal.getValue()))); case CHAR: - return ValueExpressions.getChar(((NlsString)literal.getValue()).getValue()); + return checkNullLiteral(literal, MinorType.VARCHAR, ValueExpressions.getChar(((NlsString) literal.getValue()).getValue())); case DOUBLE: double d = ((BigDecimal) literal.getValue()).doubleValue(); - return ValueExpressions.getFloat8(d); + return checkNullLiteral(literal, MinorType.FLOAT8, ValueExpressions.getFloat8(d)); case FLOAT: float f = ((BigDecimal) literal.getValue()).floatValue(); - return ValueExpressions.getFloat4(f); + return checkNullLiteral(literal, MinorType.FLOAT4, ValueExpressions.getFloat4(f)); case INTEGER: int a = ((BigDecimal) literal.getValue()).intValue(); - return ValueExpressions.getInt(a); + return checkNullLiteral(literal, MinorType.INT, ValueExpressions.getInt(a)); case DECIMAL: /* TODO: Enable using Decimal literals once we have more functions implemented for Decimal * For now continue using Double instead of decimals @@ -370,26 +370,40 @@ public class DrillOptiq { double dbl = ((BigDecimal) literal.getValue()).doubleValue(); logger.warn("Converting exact decimal into approximate decimal. Should be fixed once decimal is implemented."); - return ValueExpressions.getFloat8(dbl); + return checkNullLiteral(literal, MinorType.FLOAT8, ValueExpressions.getFloat8(dbl)); case VARCHAR: - return ValueExpressions.getChar(((NlsString)literal.getValue()).getValue()); + return checkNullLiteral(literal, MinorType.VARCHAR, ValueExpressions.getChar(((NlsString) literal.getValue()).getValue())); case SYMBOL: - return ValueExpressions.getChar(literal.getValue().toString()); + return checkNullLiteral(literal, MinorType.VARCHAR, ValueExpressions.getChar(literal.getValue().toString())); case DATE: - return (ValueExpressions.getDate((GregorianCalendar)literal.getValue())); + return checkNullLiteral(literal, MinorType.DATE, ValueExpressions.getDate((GregorianCalendar) literal.getValue())); case TIME: - return (ValueExpressions.getTime((GregorianCalendar)literal.getValue())); + return checkNullLiteral(literal, MinorType.TIME, ValueExpressions.getTime((GregorianCalendar) literal.getValue())); case TIMESTAMP: - return (ValueExpressions.getTimeStamp((GregorianCalendar) literal.getValue())); + return checkNullLiteral(literal, MinorType.TIMESTAMP, ValueExpressions.getTimeStamp((GregorianCalendar) literal.getValue())); case INTERVAL_YEAR_MONTH: - return (ValueExpressions.getIntervalYear(((BigDecimal) (literal.getValue())).intValue())); + return checkNullLiteral(literal, MinorType.INTERVALYEAR, ValueExpressions.getIntervalYear(((BigDecimal) (literal.getValue())).intValue())); case INTERVAL_DAY_TIME: - return (ValueExpressions.getIntervalDay(((BigDecimal) (literal.getValue())).longValue())); - case NULL: - return NullExpression.INSTANCE; + return checkNullLiteral(literal, MinorType.INTERVALDAY, ValueExpressions.getIntervalDay(((BigDecimal) (literal.getValue())).longValue())); + case ANY: + if (isLiteralNull(literal)) { + return NullExpression.INSTANCE; + } default: - throw new UnsupportedOperationException(String.format("Unable to convert the value of %s and type %s to a Drill constant expression.", literal, literal.getTypeName())); + throw new UnsupportedOperationException(String.format("Unable to convert the value of %s and type %s to a Drill constant expression.", literal, literal.getType().getSqlTypeName())); } } } + + private static LogicalExpression checkNullLiteral(RexLiteral literal, MinorType type, LogicalExpression orExpr) { + if(isLiteralNull(literal)) { + return new TypedNullConstant(Types.optional(type)); + } else { + return orExpr; + } + } + + private static boolean isLiteralNull(RexLiteral literal) { + return literal.getTypeName().getName().equals("NULL"); + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2cdbd600/exec/java-exec/src/main/java/org/apache/drill/exec/record/NullExpression.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/NullExpression.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/NullExpression.java deleted file mode 100644 index 1543550..0000000 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/NullExpression.java +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.drill.exec.record; - -import java.util.Iterator; - -import org.apache.drill.common.expression.ExpressionPosition; -import org.apache.drill.common.expression.LogicalExpression; -import org.apache.drill.common.expression.visitors.ExprVisitor; -import org.apache.drill.common.types.TypeProtos.MajorType; -import org.apache.drill.common.types.TypeProtos.MinorType; -import org.apache.drill.common.types.Types; - -import com.google.common.collect.Iterators; - -public class NullExpression implements LogicalExpression{ - static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(NullExpression.class); - - public static final NullExpression INSTANCE = new NullExpression(); - - final MajorType t = Types.optional(MinorType.NULL); - - @Override - public MajorType getMajorType() { - return t; - } - - @Override - public <T, V, E extends Exception> T accept(ExprVisitor<T, V, E> visitor, V value) throws E { - return visitor.visitUnknown(this, value); - } - - @Override - public ExpressionPosition getPosition() { - return ExpressionPosition.UNKNOWN; - } - - @Override - public Iterator<LogicalExpression> iterator() { - return Iterators.emptyIterator(); - } - - -}
