Updated Branches: refs/heads/execwork c941874d7 -> 8186b5a1a Updated Tags: refs/tags/pre_exec_merge [created] a97a22b0a
Initial argument validation implementation Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/e68bba25 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/e68bba25 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/e68bba25 Branch: refs/heads/execwork Commit: e68bba25a683cb9290322f7974b0743e8167b6bc Parents: c941874 Author: Timothy Chen <[email protected]> Authored: Wed Mar 13 00:15:04 2013 -0700 Committer: Jacques Nadeau <[email protected]> Committed: Mon Jul 15 13:32:08 2013 -0700 ---------------------------------------------------------------------- .../org/apache/drill/common/expression/Arg.java | 6 +- .../common/expression/ArgumentValidator.java | 2 +- .../common/expression/ArgumentValidators.java | 22 +-- .../expression/BasicArgumentValidator.java | 6 +- .../drill/common/expression/ErrorCollector.java | 30 ++-- .../common/expression/ErrorCollectorImpl.java | 83 +++++++++++ .../expression/ExpressionValidationError.java | 9 ++ .../drill/common/expression/FieldReference.java | 22 ++- .../drill/common/expression/FunctionCall.java | 5 + .../common/expression/FunctionDefinition.java | 4 + .../drill/common/expression/IfExpression.java | 36 +++-- .../common/expression/LogicalExpression.java | 103 +++++++------- .../expression/LogicalExpressionBase.java | 45 +++--- .../drill/common/expression/SchemaPath.java | 15 +- .../common/expression/ValueExpressions.java | 69 +++++++--- .../drill/common/expression/types/AtomType.java | 67 --------- .../exec/record/ExpressionTreeMaterializer.java | 138 +++++++++++++++++++ .../drill/exec/record/MaterializeVisitor.java | 4 + .../drill/exec/record/MaterializedField.java | 11 ++ .../record/ExpressionTreeMaterializerTest.java | 110 +++++++++++++++ 20 files changed, 584 insertions(+), 203 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/Arg.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/Arg.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/Arg.java index 7848cf7..de9057b 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/Arg.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/Arg.java @@ -44,9 +44,9 @@ public class Arg { return name; } - public void confirmDataType(int argIndex, LogicalExpression e, ErrorCollector errors){ + public void confirmDataType(String expr, int argIndex, LogicalExpression e, ErrorCollector errors){ if(constantsOnly){ - if(ConstantChecker.onlyIncludesConstants(e)) errors.addExpectedConstantValue(argIndex, name); + if(ConstantChecker.onlyIncludesConstants(e)) errors.addExpectedConstantValue(expr, argIndex, name); } DataType dt = e.getDataType(); if(dt.isLateBind()){ @@ -57,7 +57,7 @@ public class Arg { } // didn't find an allowed type. - errors.addUnexpectedArgumentType(name, dt, allowedTypes, argIndex); + errors.addUnexpectedArgumentType(expr, name, dt, allowedTypes, argIndex); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java index 8c374b2..dc22045 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java @@ -23,6 +23,6 @@ import java.util.List; * Validates whether the set of arguments are acceptable */ public interface ArgumentValidator { - public void validateArguments(List<LogicalExpression> expressions, ErrorCollector errors); + public void validateArguments(String expr, List<LogicalExpression> expressions, ErrorCollector errors); public String[] getArgumentNamesByPosition(); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java index 0ad1a64..25cb887 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java @@ -46,10 +46,10 @@ public class ArgumentValidators { } @Override - public void validateArguments(List<LogicalExpression> expressions, ErrorCollector errors) { + public void validateArguments(String expr, List<LogicalExpression> expressions, ErrorCollector errors) { // only need to check argument count since any type is allowed. if (!argumentCount.contains(expressions.size())) - errors.addUnexpectedArgumentCount(expressions.size(), argumentCount); + errors.addUnexpectedArgumentCount(expr, expressions.size(), argumentCount); } @Override @@ -79,25 +79,26 @@ public class ArgumentValidators { } @Override - public void validateArguments(List<LogicalExpression> expressions, ErrorCollector errors) { + public void validateArguments(String expr, List<LogicalExpression> expressions, ErrorCollector errors) { int i = -1; DataType t = null; for (LogicalExpression le : expressions) { i++; - if (t == null) t = le.getDataType(); + DataType dataType = le.getDataType(); + if (t == null) t = dataType; - if (!predicate.apply(le.getDataType())) { - errors.addUnexpectedType(i, le.getDataType()); + if (!predicate.apply(dataType)) { + errors.addUnexpectedType(expr, i, dataType); continue; } - if (allSame && t != le.getDataType()) { - errors.addUnexpectedType(i, le.getDataType()); + if (allSame && t != DataType.LATEBIND && dataType != DataType.LATEBIND && t != dataType) { + errors.addUnexpectedType(expr, i, dataType); } } if (!argumentCount.contains(expressions.size())) - errors.addUnexpectedArgumentCount(expressions.size(), argumentCount); + errors.addUnexpectedArgumentCount(expr, expressions.size(), argumentCount); } @Override @@ -119,7 +120,8 @@ public class ArgumentValidators { public static class ComparableChecker implements Predicate<DataType> { public boolean apply(DataType dt) { - return dt.getComparability().equals(Comparability.ORDERED); + Comparability comparability = dt.getComparability(); + return comparability.equals(Comparability.ORDERED) || comparability.equals(Comparability.UNKNOWN); } } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java index 89b015a..eed49d3 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java @@ -34,12 +34,12 @@ public class BasicArgumentValidator implements ArgumentValidator { } @Override - public void validateArguments(List<LogicalExpression> expressions, ErrorCollector errors) { - if (expressions.size() != args.length) errors.addUnexpectedArgumentCount(expressions.size(), args.length); + public void validateArguments(String expr, List<LogicalExpression> expressions, ErrorCollector errors) { + if (expressions.size() != args.length) errors.addUnexpectedArgumentCount(expr, expressions.size(), args.length); int i = 0; for (LogicalExpression e : expressions) { - args[i].confirmDataType(i, e, errors); + args[i].confirmDataType(expr, i, e, errors); i++; } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollector.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollector.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollector.java index a618ac6..21ecec4 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollector.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollector.java @@ -6,9 +6,9 @@ * 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. @@ -21,13 +21,23 @@ import org.apache.drill.common.expression.types.DataType; import com.google.common.collect.Range; -public class ErrorCollector { +public interface ErrorCollector { - public void addGeneralError(String s){}; - public void addUnexpectedArgumentType(String name, DataType actual, DataType[] expected, int argumentIndex){} - public void addUnexpectedArgumentCount(int actual, Range<Integer> expected){} - public void addUnexpectedArgumentCount(int actual, int expected){} - public void addNonNumericType(DataType actual){}; - public void addUnexpectedType(int index, DataType actual){}; - public void addExpectedConstantValue(int actual, String s){}; + public void addGeneralError(String expr, String s); + + public void addUnexpectedArgumentType(String expr, String name, DataType actual, DataType[] expected, int argumentIndex); + + public void addUnexpectedArgumentCount(String expr, int actual, Range<Integer> expected); + + public void addUnexpectedArgumentCount(String expr, int actual, int expected); + + public void addNonNumericType(String expr, DataType actual); + + public void addUnexpectedType(String expr, int index, DataType actual); + + public void addExpectedConstantValue(String expr, int actual, String s); + + boolean hasErrors(); + + String toErrorString(); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollectorImpl.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollectorImpl.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollectorImpl.java new file mode 100644 index 0000000..cc90b82 --- /dev/null +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ErrorCollectorImpl.java @@ -0,0 +1,83 @@ +package org.apache.drill.common.expression; + +import com.google.common.base.Joiner; +import com.google.common.collect.Lists; +import com.google.common.collect.Range; +import org.apache.drill.common.expression.types.DataType; + +import java.util.Arrays; +import java.util.List; + +public class ErrorCollectorImpl implements ErrorCollector { + List<ExpressionValidationError> errors; + + public ErrorCollectorImpl() { + errors = Lists.newArrayList(); + } + + private String addExpr(String expr, String message) { + return "Expression: [" + expr + "]. Error: " + message; + } + + @Override + public void addGeneralError(String expr, String s) { + errors.add(new ExpressionValidationError(addExpr(expr, s))); + } + + @Override + public void addUnexpectedArgumentType(String expr, String name, DataType actual, DataType[] expected, int argumentIndex) { + errors.add( + new ExpressionValidationError( + addExpr(expr, String.format( + "Unexpected argument type. Index :%d Name: %s, Type: %s, Expected type(s): %s", + argumentIndex, name, actual, Arrays.toString(expected) + )) + ) + ); + } + + @Override + public void addUnexpectedArgumentCount(String expr, int actual, Range<Integer> expected) { + errors.add(new ExpressionValidationError( + addExpr(expr, String.format("Unexpected argument count. Actual argument count: %d, Expected range: %s", actual, expected)) + )); + } + + @Override + public void addUnexpectedArgumentCount(String expr, int actual, int expected) { + errors.add(new ExpressionValidationError( + addExpr(expr, String.format("Unexpected argument count. Actual argument count: %d, Expected count: %d", actual, expected)) + )); + } + + @Override + public void addNonNumericType(String expr, DataType actual) { + errors.add(new ExpressionValidationError( + addExpr(expr, String.format("Unexpected numeric type. Actual type: %s", actual)) + )); + } + + @Override + public void addUnexpectedType(String expr, int index, DataType actual) { + errors.add(new ExpressionValidationError( + addExpr(expr, String.format("Unexpected argument type. Actual type: %s, Index: %d", actual, index)) + )); + } + + @Override + public void addExpectedConstantValue(String expr, int actual, String s) { + errors.add(new ExpressionValidationError( + addExpr(expr, String.format("Unexpected constant value. Name: %s, Actual: %s", s, actual)) + )); + } + + @Override + public boolean hasErrors() { + return !errors.isEmpty(); + } + + @Override + public String toErrorString() { + return "\n" + Joiner.on("\n").join(errors); + } +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ExpressionValidationError.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ExpressionValidationError.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ExpressionValidationError.java index 3e47001..50024a3 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ExpressionValidationError.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ExpressionValidationError.java @@ -18,5 +18,14 @@ package org.apache.drill.common.expression; public class ExpressionValidationError { + String message; + public ExpressionValidationError(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FieldReference.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FieldReference.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FieldReference.java index 073e449..94800ba 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FieldReference.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FieldReference.java @@ -6,9 +6,9 @@ * 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. @@ -32,16 +32,32 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.deser.std.StdDeserializer; import com.fasterxml.jackson.databind.ser.std.StdSerializer; +import org.apache.drill.common.expression.types.DataType; @JsonSerialize(using = Se.class) @JsonDeserialize(using = De.class) public class FieldReference extends SchemaPath { + DataType overrideType; public FieldReference(String value) { super(value); } - public static class De extends StdDeserializer<FieldReference> { + public FieldReference(String value, DataType dataType) { + super(value); + this.overrideType = dataType; + } + + @Override + public DataType getDataType() { + if(overrideType == null) { + return super.getDataType(); + } else { + return overrideType; + } + } + + public static class De extends StdDeserializer<FieldReference> { public De() { super(FieldReference.class); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionCall.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionCall.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionCall.java index 8c0b580..ee76a5c 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionCall.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionCall.java @@ -86,4 +86,9 @@ public class FunctionCall extends LogicalExpressionBase implements Iterable<Logi sb.append(") "); } } + + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + func.getArgumentValidator().validateArguments(expr, args, errors); + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java index 8797a4c..9d21763 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java @@ -49,6 +49,10 @@ public class FunctionDefinition { public String[] getArgumentNames(){ return argumentValidator.getArgumentNamesByPosition(); } + + public ArgumentValidator getArgumentValidator() { + return argumentValidator; + } public static FunctionDefinition simple(String name, ArgumentValidator argumentValidator, OutputTypeDeterminer outputType, String... registeredNames){ return new FunctionDefinition(name, argumentValidator, outputType, false, false, registeredNames); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/IfExpression.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/IfExpression.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/IfExpression.java index cdd4440..a553f4c 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/IfExpression.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/IfExpression.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.drill.common.expression.IfExpression.IfCondition; +import org.apache.drill.common.expression.types.DataType; import org.apache.drill.common.expression.visitors.ExprVisitor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,8 +38,7 @@ public class IfExpression extends LogicalExpressionBase implements Iterable<IfCo private IfExpression(List<IfCondition> conditions, LogicalExpression elseExpression){ this.conditions = ImmutableList.copyOf(conditions); this.elseExpression = elseExpression; - }; - + } public static class IfCondition{ public final LogicalExpression condition; @@ -64,12 +64,21 @@ public class IfExpression extends LogicalExpressionBase implements Iterable<IfCo List<IfCondition> conditions = new ArrayList<IfCondition>(); private LogicalExpression elseExpression; - public void addCondition(IfCondition condition){ + public Builder addCondition(IfCondition condition){ conditions.add(condition); + return this; } + + public Builder addConditions(Iterable<IfCondition> conditions) { + for(IfCondition condition : conditions) { + addCondition(condition); + } + return this; + } - public void setElse(LogicalExpression elseExpression) { + public Builder setElse(LogicalExpression elseExpression) { this.elseExpression = elseExpression; + return this; } public IfExpression build(){ @@ -77,11 +86,14 @@ public class IfExpression extends LogicalExpressionBase implements Iterable<IfCo } } - - - - - @Override + + + @Override + public DataType getDataType() { + return DataType.BOOLEAN; + } + + @Override public void addToString(StringBuilder sb) { sb.append(" ( "); for(int i =0; i < conditions.size(); i++){ @@ -97,8 +109,12 @@ public class IfExpression extends LogicalExpressionBase implements Iterable<IfCo sb.append(" ) "); } + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } + - public static Builder newBuilder(){ + public static Builder newBuilder(){ return new Builder(); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java index f7632a6..3df33a0 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java @@ -6,9 +6,9 @@ * 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. @@ -44,58 +44,65 @@ import com.fasterxml.jackson.databind.ser.std.StdSerializer; //@JsonDeserialize(using = LogicalExpression.De.class) // Excluded as we need to register this with the DrillConfig. @JsonSerialize(using = LogicalExpression.Se.class) public interface LogicalExpression { - static final Logger logger = LoggerFactory.getLogger(LogicalExpression.class); - - public abstract DataType getDataType(); - public void addToString(StringBuilder sb); - public void resolveAndValidate(ErrorCollector errors); - public <T> T accept(ExprVisitor<T> visitor); - - public static class De extends StdDeserializer<LogicalExpression> { - DrillConfig config; - public De(DrillConfig config) { - super(LogicalExpression.class); - this.config = config; - } + static final Logger logger = LoggerFactory.getLogger(LogicalExpression.class); - @Override - public LogicalExpression deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException, - JsonProcessingException { - String expr = jp.getText(); - - if (expr == null || expr.isEmpty()) - return null; - try { - // logger.debug("Parsing expression string '{}'", expr); - ExprLexer lexer = new ExprLexer(new ANTLRStringStream(expr)); - - CommonTokenStream tokens = new CommonTokenStream(lexer); - ExprParser parser = new ExprParser(tokens); - parser.setRegistry(new FunctionRegistry(config)); - parse_return ret = parser.parse(); - // logger.debug("Found expression '{}'", ret.e); - return ret.e; - } catch (RecognitionException e) { - throw new RuntimeException(e); - } - } + public abstract DataType getDataType(); - } + public void addToString(StringBuilder sb); - public static class Se extends StdSerializer<LogicalExpression> { + public void resolveAndValidate(String expr, ErrorCollector errors); - protected Se() { - super(LogicalExpression.class); - } + public <T> T accept(ExprVisitor<T> visitor); + + public static class De extends StdDeserializer<LogicalExpression> { + DrillConfig config; + ErrorCollector errorCollector; + + public De(DrillConfig config) { + super(LogicalExpression.class); + this.config = config; + this.errorCollector = config.getErrorCollector(); + } + + @Override + public LogicalExpression deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException, + JsonProcessingException { + String expr = jp.getText(); + + if (expr == null || expr.isEmpty()) + return null; + try { + // logger.debug("Parsing expression string '{}'", expr); + ExprLexer lexer = new ExprLexer(new ANTLRStringStream(expr)); + + CommonTokenStream tokens = new CommonTokenStream(lexer); + ExprParser parser = new ExprParser(tokens); + parser.setRegistry(new FunctionRegistry(config)); + parse_return ret = parser.parse(); + // logger.debug("Found expression '{}'", ret.e); + ret.e.resolveAndValidate(expr, errorCollector); + return ret.e; + } catch (RecognitionException e) { + throw new RuntimeException(e); + } + } - @Override - public void serialize(LogicalExpression value, JsonGenerator jgen, SerializerProvider provider) throws IOException, - JsonGenerationException { - StringBuilder sb = new StringBuilder(); - value.addToString(sb); - jgen.writeString(sb.toString()); } - } + public static class Se extends StdSerializer<LogicalExpression> { + + protected Se() { + super(LogicalExpression.class); + } + + @Override + public void serialize(LogicalExpression value, JsonGenerator jgen, SerializerProvider provider) throws IOException, + JsonGenerationException { + StringBuilder sb = new StringBuilder(); + value.addToString(sb); + jgen.writeString(sb.toString()); + } + + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpressionBase.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpressionBase.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpressionBase.java index a907146..e973df7 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpressionBase.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/LogicalExpressionBase.java @@ -6,9 +6,9 @@ * 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. @@ -24,46 +24,33 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; -@JsonPropertyOrder({ "type" }) -public abstract class LogicalExpressionBase implements LogicalExpression{ +@JsonPropertyOrder({"type"}) +public abstract class LogicalExpressionBase implements LogicalExpression { + - // public static DataType getJointType(String parentName, LogicalExpression expr1, LogicalExpression expr2) throws ExpressionValidationException{ // DataType dt = DataType.getCombinedCast(expr1.getDataType(), expr2.getDataType()); // if(dt == null) throw new ExpressionValidationException(); // // return dt; // } - - protected void i(StringBuilder sb, int indent){ - for(int i = 0; i < indent; i++){ - sb.append(" "); - } - } - + + protected void i(StringBuilder sb, int indent) { + for (int i = 0; i < indent; i++) { + sb.append(" "); + } + } + // @Override // public <T> T accept(ExprVisitor<T> visitor) { // return visitor.visit(this); // } - @Override - public DataType getDataType() { - throw new UnsupportedOperationException(); - } - - - @Override - public void resolveAndValidate(ErrorCollector errors) { - } - - - @JsonProperty("type") - public String getDescription(){ - return this.getClass().getSimpleName(); - } - + @JsonProperty("type") + public String getDescription() { + return this.getClass().getSimpleName(); + } - } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/SchemaPath.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/SchemaPath.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/SchemaPath.java index 004d812..dfae6fd 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/SchemaPath.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/SchemaPath.java @@ -21,6 +21,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.drill.common.expression.ValueExpressions.CollisionBehavior; +import org.apache.drill.common.expression.types.DataType; import org.apache.drill.common.expression.visitors.ExprVisitor; public class SchemaPath extends LogicalExpressionBase{ @@ -119,17 +120,25 @@ public class SchemaPath extends LogicalExpressionBase{ public CharSequence getPath(){ return originalPath; } - - @Override + + @Override + public DataType getDataType() { + return DataType.LATEBIND; + } + + @Override public void addToString(StringBuilder sb) { sb.append("'"); sb.append(originalPath); sb.append("'"); } + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } - @Override + @Override public String toString() { return "SchemaPath [rootSegment=" + rootSegment + "]"; } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java index d572715..f07f2a7 100644 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java +++ b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java @@ -17,6 +17,7 @@ ******************************************************************************/ package org.apache.drill.common.expression; +import org.apache.drill.common.expression.types.DataType; import org.apache.drill.common.expression.visitors.ExprVisitor; @@ -44,8 +45,7 @@ public class ValueExpressions { } - protected static abstract class ValueExpression<V> extends - LogicalExpressionBase { + protected static abstract class ValueExpression<V> extends LogicalExpressionBase { public final V value; protected ValueExpression(String value) { @@ -67,12 +67,22 @@ public class ValueExpressions { return Boolean.parseBoolean(s); } - @Override + @Override + public DataType getDataType() { + return DataType.BOOLEAN; + } + + @Override public void addToString(StringBuilder sb) { sb.append(value.toString()); } - @Override + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } + + + @Override public <T> T accept(ExprVisitor<T> visitor) { return visitor.visitBoolean(this); } @@ -92,13 +102,22 @@ public class ValueExpressions { public double getDouble(){ return d; } - - @Override + + @Override + public DataType getDataType() { + return DataType.FLOAT32; + } + + @Override public void addToString(StringBuilder sb) { sb.append(d); } - - @Override + + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } + + @Override public <T> T accept(ExprVisitor<T> visitor) { return visitor.visitDoubleExpression(this); } @@ -113,13 +132,22 @@ public class ValueExpressions { public long getLong(){ return l; } - - @Override + + @Override + public DataType getDataType() { + return DataType.INT64; + } + + @Override public void addToString(StringBuilder sb) { sb.append(l); } - - @Override + + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } + + @Override public <T> T accept(ExprVisitor<T> visitor) { return visitor.visitLongExpression(this); } @@ -134,15 +162,24 @@ public class ValueExpressions { protected String parseValue(String s) { return s; } - - @Override + + @Override + public DataType getDataType() { + return DataType.NVARCHAR; + } + + @Override public void addToString(StringBuilder sb) { sb.append("\""); sb.append(value.toString()); sb.append("\""); } - - @Override + + @Override + public void resolveAndValidate(String expr, ErrorCollector errors) { + } + + @Override public <T> T accept(ExprVisitor<T> visitor) { return visitor.visitQuotedString(this); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/types/AtomType.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/types/AtomType.java b/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/types/AtomType.java deleted file mode 100644 index f21ddb9..0000000 --- a/sandbox/prototype/common/src/main/java/org/apache/drill/common/expression/types/AtomType.java +++ /dev/null @@ -1,67 +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.common.expression.types; - - -public class AtomType extends DataType { - private String name; - private Comparability comparability; - private boolean isNumericType; - - public AtomType(String name, Comparability comparability, boolean isNumericType) { - super(); - this.name = name; - this.comparability = comparability; - this.isNumericType = isNumericType; - } - - - public boolean isNumericType() { - return isNumericType; - } - - - @Override - public String getName() { - return name; - } - - @Override - public boolean isLateBind() { - return false; - } - - @Override - public boolean hasChildType() { - return false; - } - - @Override - public DataType getChildType() { - return null; - } - - @Override - public Comparability getComparability() { - return comparability; - } - - - - -} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpressionTreeMaterializer.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpressionTreeMaterializer.java b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpressionTreeMaterializer.java new file mode 100644 index 0000000..391aec5 --- /dev/null +++ b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpressionTreeMaterializer.java @@ -0,0 +1,138 @@ +/******************************************************************************* + * 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 com.google.common.collect.Lists; +import org.apache.drill.common.expression.*; +import org.apache.drill.common.expression.types.DataType; +import org.apache.drill.common.expression.visitors.ExprVisitor; + +import java.util.List; + +public class ExpressionTreeMaterializer { + public LogicalExpression Materialize(LogicalExpression expr, BatchSchema schema, ErrorCollector errorCollector) { + return expr.accept(new MaterializeVisitor(schema, errorCollector)); + } + + private class MaterializeVisitor implements ExprVisitor<LogicalExpression> { + private final ErrorCollector errorCollector; + private final BatchSchema schema; + private boolean isModified; // Flag to track if children is changed + + public MaterializeVisitor(BatchSchema schema, ErrorCollector errorCollector) { + this.schema = schema; + this.errorCollector = errorCollector; + isModified = false; + } + + private LogicalExpression validateNewExpr(LogicalExpression newExpr) { + StringBuilder stringBuilder = new StringBuilder(); + newExpr.addToString(stringBuilder); + newExpr.resolveAndValidate(stringBuilder.toString(), errorCollector); + return newExpr; + } + + @Override + public LogicalExpression visitFunctionCall(FunctionCall call) { + List<LogicalExpression> args = Lists.newArrayList(call.iterator()); + boolean hasChanged = false; + for (int i = 0; i < args.size(); ++i) { + LogicalExpression newExpr = args.get(i).accept(this); + if (isModified) { + hasChanged = true; + args.set(i, newExpr); + isModified = false; + } + } + + if(hasChanged) { + isModified = true; + return validateNewExpr(new FunctionCall(call.getDefinition(), args)); + } + + return call; + } + + @Override + public LogicalExpression visitIfExpression(IfExpression ifExpr) { + List<IfExpression.IfCondition> conditions = Lists.newArrayList(ifExpr.iterator()); + boolean hasChanged = false; + LogicalExpression newElseExpr = null; + if(ifExpr.elseExpression != null) { + newElseExpr = ifExpr.elseExpression.accept(this); + hasChanged = isModified; + } + + isModified = false; + + for(int i = 0; i < conditions.size(); ++i) { + IfExpression.IfCondition condition = conditions.get(i); + + LogicalExpression newCondition = condition.condition.accept(this); + boolean modified = isModified; + isModified = false; + LogicalExpression newExpr = condition.expression.accept(this); + if(modified || isModified) { + conditions.set(i, new IfExpression.IfCondition(newCondition, newExpr)); + hasChanged = true; + isModified = false; + } + } + + if(hasChanged) { + isModified = true; + return validateNewExpr(IfExpression.newBuilder().setElse(newElseExpr).addConditions(conditions).build()); + } + + return ifExpr; + } + + @Override + public LogicalExpression visitSchemaPath(SchemaPath path) { + for (MaterializedField field : schema) { + if (field.getType() != DataType.LATEBIND && field.matches(path)) { + isModified = true; + return validateNewExpr(new FieldReference(path.getPath().toString(), field.getType())); + } + } + + return path; + } + + @Override + public LogicalExpression visitLongExpression(ValueExpressions.LongExpression intExpr) { + return intExpr; + } + + @Override + public LogicalExpression visitDoubleExpression(ValueExpressions.DoubleExpression dExpr) { + return dExpr; + } + + @Override + public LogicalExpression visitBoolean(ValueExpressions.BooleanExpression e) { + return e; + } + + @Override + public LogicalExpression visitQuotedString(ValueExpressions.QuotedString e) { + return e; + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializeVisitor.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializeVisitor.java b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializeVisitor.java new file mode 100644 index 0000000..e28778f --- /dev/null +++ b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializeVisitor.java @@ -0,0 +1,4 @@ +package org.apache.drill.exec.record; + +public interface MaterializeVisitor { +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java index 05fb576..b692a93 100644 --- a/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java +++ b/sandbox/prototype/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java @@ -142,6 +142,17 @@ public class MaterializedField implements Comparable<MaterializedField> { // we've reviewed all path segments. confirm that we don't have any extra name parts. return !iter.hasNext(); + private void check(String name, Object val1, Object expected) throws SchemaChangeException{ + if(expected.equals(val1)) return; + throw new SchemaChangeException("Expected and actual field definitions don't match. Actual %s: %s, expected %s: %s", name, val1, name, expected); + } + + public void checkMaterialization(MaterializedField expected) throws SchemaChangeException{ + if(this.type == expected.type || expected.type == DataType.LATEBIND) throw new SchemaChangeException("Expected and actual field definitions don't match. Actual DataType: %s, expected DataTypes: %s", this.type, expected.type); + if(expected.valueClass != null) check("valueClass", this.valueClass, expected.valueClass); + check("fieldId", this.fieldId, expected.fieldId); + check("nullability", this.nullable, expected.nullable); + check("valueMode", this.mode, expected.mode); } // private void check(String name, Object val1, Object expected) throws SchemaChangeException{ http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/e68bba25/sandbox/prototype/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b/sandbox/prototype/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java new file mode 100644 index 0000000..ab68ea2 --- /dev/null +++ b/sandbox/prototype/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java @@ -0,0 +1,110 @@ +package org.apache.drill.exec.record; + +import com.beust.jcommander.internal.Lists; +import com.google.common.collect.Range; +import org.apache.drill.common.expression.*; +import org.apache.drill.common.expression.types.DataType; +import org.apache.drill.common.physical.RecordField; +import org.apache.drill.exec.exception.SchemaChangeException; +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class ExpressionTreeMaterializerTest { + @Test + public void testMaterializingConstantTree() throws SchemaChangeException { + ExpressionTreeMaterializer tm = new ExpressionTreeMaterializer(); + ErrorCollector ec = new ErrorCollectorImpl(); + BatchSchema schema = new BatchSchema.BatchSchemaBuilder().buildAndClear(); + LogicalExpression expr = tm.Materialize(new ValueExpressions.LongExpression(1L), schema, ec); + assertTrue(expr instanceof ValueExpressions.LongExpression); + assertEquals(1L, ValueExpressions.LongExpression.class.cast(expr).getLong()); + assertFalse(ec.hasErrors()); + } + + @Test + public void testMaterializingLateboundField() throws SchemaChangeException { + ExpressionTreeMaterializer tm = new ExpressionTreeMaterializer(); + ErrorCollector ec = new ErrorCollectorImpl(); + BatchSchema.BatchSchemaBuilder builder = new BatchSchema.BatchSchemaBuilder(); + builder.addTypedField((short) 2, DataType.INT64, false, RecordField.ValueMode.RLE, Long.class); + LogicalExpression expr = tm.Materialize(new FieldReference("test"), builder.buildAndClear(), ec); + assertEquals(DataType.INT64, expr.getDataType()); + assertFalse(ec.hasErrors()); + } + + @Test + public void testMaterializingLateboundTree() throws SchemaChangeException { + ExpressionTreeMaterializer tm = new ExpressionTreeMaterializer(); + ErrorCollector ec = new ErrorCollectorImpl(); + BatchSchema.BatchSchemaBuilder builder = new BatchSchema.BatchSchemaBuilder(); + builder.addTypedField((short) 2, DataType.INT64, false, RecordField.ValueMode.RLE, Long.class); + LogicalExpression expr = new IfExpression.Builder().addCondition( + new IfExpression.IfCondition(new FieldReference("test"), + new IfExpression.Builder().addCondition(new IfExpression.IfCondition(new ValueExpressions.LongExpression(1L), new FieldReference("test1"))).build() + ) + ).build(); + LogicalExpression newExpr = tm.Materialize(expr, builder.buildAndClear(), ec); + assertTrue(newExpr instanceof IfExpression); + IfExpression newIfExpr = (IfExpression) newExpr; + assertEquals(1, newIfExpr.conditions.size()); + IfExpression.IfCondition ifCondition = newIfExpr.conditions.get(0); + assertEquals(DataType.INT64, ifCondition.condition.getDataType()); + assertTrue(ifCondition.expression instanceof IfExpression); + newIfExpr = (IfExpression) ifCondition.expression; + assertEquals(1, newIfExpr.conditions.size()); + ifCondition = newIfExpr.conditions.get(0); + assertEquals(DataType.INT64, ifCondition.expression.getDataType()); + assertEquals(1L, ((ValueExpressions.LongExpression) ifCondition.condition).getLong()); + assertFalse(ec.hasErrors()); + } + + @Test + public void testMaterializingLateboundTreeValidated() throws SchemaChangeException { + ExpressionTreeMaterializer tm = new ExpressionTreeMaterializer(); + ErrorCollector ec = new ErrorCollector() { + boolean errorFound = false; + @Override + public void addGeneralError(String expr, String s) {errorFound = true;} + @Override + public void addUnexpectedArgumentType(String expr, String name, DataType actual, DataType[] expected, int argumentIndex) {} + @Override + public void addUnexpectedArgumentCount(String expr, int actual, Range<Integer> expected) {} + @Override + public void addUnexpectedArgumentCount(String expr, int actual, int expected) {} + @Override + public void addNonNumericType(String expr, DataType actual) {} + @Override + public void addUnexpectedType(String expr, int index, DataType actual) {} + @Override + public void addExpectedConstantValue(String expr, int actual, String s) {} + @Override + public boolean hasErrors() { return errorFound; } + @Override + public String toErrorString() { return ""; } + }; + BatchSchema.BatchSchemaBuilder builder = new BatchSchema.BatchSchemaBuilder(); + builder.addTypedField((short) 2, DataType.INT64, false, RecordField.ValueMode.RLE, Long.class); + LogicalExpression expr = new FunctionCall(FunctionDefinition.simple("testFunc", new ArgumentValidator() { + @Override + public void validateArguments(String expr, List<LogicalExpression> expressions, ErrorCollector errors) { + errors.addGeneralError(expr, "Error!"); + } + + @Override + public String[] getArgumentNamesByPosition() { + return new String[0]; + } + }, OutputTypeDeterminer.FIXED_BOOLEAN), Lists.newArrayList((LogicalExpression) new FieldReference("test"))); + LogicalExpression newExpr = tm.Materialize(expr, builder.buildAndClear(), ec); + assertTrue(newExpr instanceof FunctionCall); + FunctionCall funcExpr = (FunctionCall) newExpr; + assertEquals(1, funcExpr.args.size()); + assertEquals(DataType.INT64, funcExpr.args.get(0).getDataType()); + assertTrue(ec.hasErrors()); + } +}
