diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaPostAggMacros.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaPostAggMacros.java index 3f21ebca105e..4b7ce852fd10 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaPostAggMacros.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaPostAggMacros.java @@ -85,7 +85,7 @@ public ExprEval eval(ObjectBinding bindings) ExprEval eval = estimateExpr.eval(bindings); final Object valObj = eval.value(); if (valObj == null) { - return ExprEval.of(null); + return ExprEval.ofDouble(null); } if (valObj instanceof SketchHolder) { SketchHolder thetaSketchHolder = (SketchHolder) valObj; diff --git a/extensions-core/testing-tools/src/main/java/org/apache/druid/query/expressions/SleepExprMacro.java b/extensions-core/testing-tools/src/main/java/org/apache/druid/query/expressions/SleepExprMacro.java index a947f9ef4b78..413625b3988a 100644 --- a/extensions-core/testing-tools/src/main/java/org/apache/druid/query/expressions/SleepExprMacro.java +++ b/extensions-core/testing-tools/src/main/java/org/apache/druid/query/expressions/SleepExprMacro.java @@ -70,7 +70,7 @@ public ExprEval eval(ObjectBinding bindings) Thread.sleep((long) (seconds * 1000)); } } - return ExprEval.of(null); + return ExprEval.ofMissing(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); diff --git a/integration-tests-ex/tools/src/main/java/org/apache/druid/testing/tools/SleepExprMacro.java b/integration-tests-ex/tools/src/main/java/org/apache/druid/testing/tools/SleepExprMacro.java index ea8d29cc1dc6..85d00f2d65ed 100644 --- a/integration-tests-ex/tools/src/main/java/org/apache/druid/testing/tools/SleepExprMacro.java +++ b/integration-tests-ex/tools/src/main/java/org/apache/druid/testing/tools/SleepExprMacro.java @@ -70,7 +70,7 @@ public ExprEval eval(ObjectBinding bindings) Thread.sleep((long) (seconds * 1000)); } } - return ExprEval.of(null); + return ExprEval.ofMissing(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); diff --git a/processing/src/main/java/org/apache/druid/math/expr/ApplyFunction.java b/processing/src/main/java/org/apache/druid/math/expr/ApplyFunction.java index 8302e1c901b5..e8abdedc56ad 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ApplyFunction.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ApplyFunction.java @@ -183,7 +183,7 @@ public ExprEval apply(LambdaExpr lambdaExpr, List argsExpr, Expr.ObjectBin Object[] array = arrayEval.asArray(); if (array == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } if (array.length == 0) { return arrayEval; @@ -246,7 +246,7 @@ public ExprEval apply(LambdaExpr lambdaExpr, List argsExpr, Expr.ObjectBin arrayInputs.add(Arrays.asList(array)); } if (hadNull) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } if (hadEmpty) { return ExprEval.ofStringArray(new String[0]); @@ -334,7 +334,7 @@ public ExprEval apply(LambdaExpr lambdaExpr, List argsExpr, Expr.ObjectBin Object[] array = arrayEval.asArray(); if (array == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } Object accumulator = accEval.value(); @@ -401,7 +401,7 @@ public ExprEval apply(LambdaExpr lambdaExpr, List argsExpr, Expr.ObjectBin arrayInputs.add(Arrays.asList(array)); } if (hadNull) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } if (hadEmpty) { return ExprEval.ofStringArray(new Object[0]); diff --git a/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java b/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java index 1ec567f97b0b..588a425aae57 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java +++ b/processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java @@ -132,7 +132,7 @@ public ExprEval eval(ObjectBinding bindings) // Result of any Binary expressions is null if any of the argument is null. // e.g "select null * 2 as c;" or "select null + 1 as c;" will return null as per Standard SQL spec. if (leftVal.value() == null || rightVal.value() == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } ExpressionType type = ExpressionTypeConversion.autoDetect(leftVal, rightVal); @@ -144,7 +144,7 @@ public ExprEval eval(ObjectBinding bindings) case DOUBLE: default: if (leftVal.isNumericNull() || rightVal.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } return ExprEval.of(evalDouble(leftVal.asDouble(), rightVal.asDouble())); } @@ -183,7 +183,7 @@ public ExprEval eval(ObjectBinding bindings) // Result of any Binary expressions is null if any of the argument is null. // e.g "select null * 2 as c;" or "select null + 1 as c;" will return null as per Standard SQL spec. if (leftVal.value() == null || rightVal.value() == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } ExpressionType type = ExpressionTypeConversion.autoDetect(leftVal, rightVal); @@ -201,7 +201,7 @@ public ExprEval eval(ObjectBinding bindings) case DOUBLE: default: if (leftVal.isNumericNull() || rightVal.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } result = evalDouble(leftVal.asDouble(), rightVal.asDouble()); break; diff --git a/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java b/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java index fa31c10c61ef..bffc0435335b 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java @@ -53,7 +53,7 @@ protected BinaryOpExprBase copy(Expr left, Expr right) @Override protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left + right); + return ExprEval.ofString(left + right); } @Override diff --git a/processing/src/main/java/org/apache/druid/math/expr/BuiltInExprMacros.java b/processing/src/main/java/org/apache/druid/math/expr/BuiltInExprMacros.java index 0f5a08d2eb70..189ae918c3d3 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/BuiltInExprMacros.java +++ b/processing/src/main/java/org/apache/druid/math/expr/BuiltInExprMacros.java @@ -181,7 +181,7 @@ public ExprEval eval(ObjectBinding bindings) { ExprEval toDecode = arg.eval(bindings); if (toDecode.value() == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } return new StringExpr(StringUtils.fromUtf8(StringUtils.decodeBase64String(toDecode.asString()))).eval(bindings); } diff --git a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java index 6d44542171a0..c3f5fc0c0121 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java @@ -412,7 +412,7 @@ public String toString() @Override protected ExprEval realEval() { - return ExprEval.of(value); + return ExprEval.ofString(value); } @Override diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java b/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java index 0afa96c0f94c..d69235bd11ca 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java @@ -64,7 +64,7 @@ public abstract class ExprEval * {@link ByteBuffer}. Certain types are deserialized more efficiently if allowed * to retain references to the provided buffer. */ - public static ExprEval deserialize( + public static ExprEval deserialize( final ByteBuffer buffer, final int offset, final int maxSize, @@ -281,17 +281,40 @@ private static Class convertType(@Nullable Class existing, Class next) return Object.class; } - public static ExprEval of(long longValue) + /** + * Eval that represents a null value of undefined type. Commonly used to represent columns that do not exist. + * + * Behaviorally equivalent to {@code ofLong(null)}. Generally, this function is preferred if the type is unknown, and + * {@code ofLong(null)} is preferred if the type is known to be long. + */ + public static ExprEval ofMissing() + { + return LongExprEval.OF_NULL; + } + + public static ExprEval of(long longValue) { return new LongExprEval(longValue); } - public static ExprEval of(double doubleValue) + public static ExprEval of(double doubleValue) { return new DoubleExprEval(doubleValue); } - public static ExprEval of(@Nullable String stringValue) + /** + * Equivalent to {@link #ofString(String)}. Deprecated because the pattern {@code ExprEval.of(null)} for + * an "unknown type" null is not recommended-- instead it should be {@link ExprEval#ofMissing()} + * + * @deprecated use {@link #ofString(String)} instead, which is clearer as to type + */ + @Deprecated + public static ExprEval of(@Nullable String stringValue) + { + return ofString(stringValue); + } + + public static ExprEval ofString(@Nullable String stringValue) { if (stringValue == null) { return StringExprEval.OF_NULL; @@ -299,7 +322,7 @@ public static ExprEval of(@Nullable String stringValue) return new StringExprEval(stringValue); } - public static ExprEval ofLong(@Nullable Number longValue) + public static ExprEval ofLong(@Nullable Number longValue) { if (longValue == null) { return LongExprEval.OF_NULL; @@ -307,7 +330,7 @@ public static ExprEval ofLong(@Nullable Number longValue) return new LongExprEval(longValue); } - public static ExprEval ofDouble(@Nullable Number doubleValue) + public static ExprEval ofDouble(@Nullable Number doubleValue) { if (doubleValue == null) { return DoubleExprEval.OF_NULL; @@ -315,7 +338,7 @@ public static ExprEval ofDouble(@Nullable Number doubleValue) return new DoubleExprEval(doubleValue); } - public static ExprEval ofLongArray(@Nullable Object[] longValue) + public static ExprEval ofLongArray(@Nullable Object[] longValue) { if (longValue == null) { return ArrayExprEval.OF_NULL_LONG; @@ -323,7 +346,7 @@ public static ExprEval ofLongArray(@Nullable Object[] longValue) return new ArrayExprEval(ExpressionType.LONG_ARRAY, longValue); } - public static ExprEval ofDoubleArray(@Nullable Object[] doubleValue) + public static ExprEval ofDoubleArray(@Nullable Object[] doubleValue) { if (doubleValue == null) { return ArrayExprEval.OF_NULL_DOUBLE; @@ -331,7 +354,7 @@ public static ExprEval ofDoubleArray(@Nullable Object[] doubleValue) return new ArrayExprEval(ExpressionType.DOUBLE_ARRAY, doubleValue); } - public static ExprEval ofStringArray(@Nullable Object[] stringValue) + public static ExprEval ofStringArray(@Nullable Object[] stringValue) { if (stringValue == null) { return ArrayExprEval.OF_NULL_STRING; @@ -340,7 +363,7 @@ public static ExprEval ofStringArray(@Nullable Object[] stringValue) } - public static ExprEval ofArray(ExpressionType outputType, @Nullable Object[] value) + public static ExprEval ofArray(ExpressionType outputType, @Nullable Object[] value) { Preconditions.checkArgument(outputType.isArray(), "Output type %s is not an array", outputType); return new ArrayExprEval(outputType, value); @@ -349,12 +372,12 @@ public static ExprEval ofArray(ExpressionType outputType, @Nullable Object[] val /** * Convert a boolean into a long expression type */ - public static ExprEval ofLongBoolean(boolean value) + public static ExprEval ofLongBoolean(boolean value) { return value ? LongExprEval.TRUE : LongExprEval.FALSE; } - public static ExprEval ofComplex(ExpressionType outputType, @Nullable Object value) + public static ExprEval ofComplex(ExpressionType outputType, @Nullable Object value) { if (ExpressionType.NESTED_DATA.equals(outputType)) { return new NestedDataExprEval(value); @@ -362,7 +385,7 @@ public static ExprEval ofComplex(ExpressionType outputType, @Nullable Object val return new ComplexExprEval(outputType, value); } - public static ExprEval bestEffortArray(@Nullable List theList) + public static ExprEval bestEffortArray(@Nullable List theList) { // do not convert empty lists to arrays with a single null element here, because that should have been done // by the selectors preparing their ObjectBindings if necessary. If we get to this point it was legitimately @@ -377,13 +400,13 @@ public static ExprEval bestEffortArray(@Nullable List theList) /** * Examine java type to find most appropriate expression type */ - public static ExprEval bestEffortOf(@Nullable Object val) + public static ExprEval bestEffortOf(@Nullable Object val) { if (val == null) { - return StringExprEval.OF_NULL; + return LongExprEval.OF_NULL; } if (val instanceof ExprEval) { - return (ExprEval) val; + return (ExprEval) val; } if (val instanceof String) { return new StringExprEval((String) val); @@ -496,7 +519,7 @@ public static ExprEval bestEffortOf(@Nullable Object val) * @param type type, or null to be equivalent to {@link #bestEffortOf(Object)} * @param value object to be coerced to the type */ - public static ExprEval ofType(@Nullable ExpressionType type, @Nullable Object value) + public static ExprEval ofType(@Nullable ExpressionType type, @Nullable Object value) { if (type == null) { return bestEffortOf(value); @@ -521,7 +544,7 @@ public static ExprEval ofType(@Nullable ExpressionType type, @Nullable Object va if (value instanceof byte[]) { return new StringExprEval(StringUtils.encodeBase64String((byte[]) value)); } - return of(Evals.asString(value)); + return ofString(Evals.asString(value)); case LONG: if (value instanceof Number) { return ofLong((Number) value); @@ -844,7 +867,7 @@ public final ExprEval castTo(ExpressionType castTo) return ExprEval.of(asLong()); } case STRING: - return ExprEval.of(asString()); + return ExprEval.ofString(asString()); case ARRAY: switch (castTo.getElementType().getType()) { case DOUBLE: @@ -918,7 +941,7 @@ public final ExprEval castTo(ExpressionType castTo) case LONG: return this; case STRING: - return ExprEval.of(asString()); + return ExprEval.ofString(asString()); case ARRAY: if (value == null) { return new ArrayExprEval(castTo, null); @@ -1283,7 +1306,7 @@ public ExprEval castTo(ExpressionType castTo) switch (castTo.getType()) { case STRING: if (value.length == 1) { - return ExprEval.of(asString()); + return ExprEval.ofString(asString()); } return ExprEval.ofType(castTo, null); case LONG: diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExprListenerImpl.java b/processing/src/main/java/org/apache/druid/math/expr/ExprListenerImpl.java index b2aaeadf2146..8d2e1a690f8b 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExprListenerImpl.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExprListenerImpl.java @@ -385,7 +385,7 @@ public void exitFunctionArgs(ExprParser.FunctionArgsContext ctx) @Override public void exitNull(ExprParser.NullContext ctx) { - nodes.put(ctx, new StringExpr(null)); + nodes.put(ctx, new NullLongExpr()); } @Override diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessing.java b/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessing.java index 2e84b33d7bf5..e0582c0774ec 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessing.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessing.java @@ -45,19 +45,19 @@ public class ExpressionProcessing @VisibleForTesting public static void initializeForTests() { - INSTANCE = new ExpressionProcessingConfig(null, null, null, null); + INSTANCE = new ExpressionProcessingConfig(null, null, null); } @VisibleForTesting public static void initializeForHomogenizeNullMultiValueStrings() { - INSTANCE = new ExpressionProcessingConfig(null, null, true, null); + INSTANCE = new ExpressionProcessingConfig(null, true, null); } @VisibleForTesting public static void initializeForFallback() { - INSTANCE = new ExpressionProcessingConfig(null, null, null, true); + INSTANCE = new ExpressionProcessingConfig(null, null, true); } /** diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java b/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java index 712fac379c8a..b437bb7e3d29 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java @@ -21,14 +21,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.druid.java.util.common.logger.Logger; import javax.annotation.Nullable; public class ExpressionProcessingConfig { - private static final Logger LOG = new Logger(ExpressionProcessingConfig.class); - public static final String NULL_HANDLING_LEGACY_LOGICAL_OPS_STRING = "druid.expressions.useStrictBooleans"; // Coerce arrays to multi value strings public static final String PROCESS_ARRAYS_AS_MULTIVALUE_STRINGS_CONFIG_STRING = @@ -47,23 +44,13 @@ public class ExpressionProcessingConfig @JsonProperty("allowVectorizeFallback") private final boolean allowVectorizeFallback; - @Deprecated - @JsonProperty("useStrictBooleans") - private final boolean useStrictBooleans; - @JsonCreator public ExpressionProcessingConfig( - @Deprecated @JsonProperty("useStrictBooleans") @Nullable Boolean useStrictBooleans, @JsonProperty("processArraysAsMultiValueStrings") @Nullable Boolean processArraysAsMultiValueStrings, @JsonProperty("homogenizeNullMultiValueStringArrays") @Nullable Boolean homogenizeNullMultiValueStringArrays, @JsonProperty("allowVectorizeFallback") @Nullable Boolean allowVectorizeFallback ) { - this.useStrictBooleans = getWithPropertyFallback( - useStrictBooleans, - NULL_HANDLING_LEGACY_LOGICAL_OPS_STRING, - "true" - ); this.processArraysAsMultiValueStrings = getWithPropertyFallbackFalse( processArraysAsMultiValueStrings, PROCESS_ARRAYS_AS_MULTIVALUE_STRINGS_CONFIG_STRING diff --git a/processing/src/main/java/org/apache/druid/math/expr/Function.java b/processing/src/main/java/org/apache/druid/math/expr/Function.java index 3f2a357b6747..c95e53fce0e5 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/Function.java +++ b/processing/src/main/java/org/apache/druid/math/expr/Function.java @@ -252,14 +252,14 @@ abstract class UnivariateMathFunction extends UnivariateFunction protected final ExprEval eval(ExprEval param) { if (param.isNumericNull()) { - return ExprEval.of(null); - } - if (param.type().is(ExprType.LONG)) { + return ExprEval.ofMissing(); + } else if (param.type().is(ExprType.LONG)) { return eval(param.asLong()); } else if (param.type().is(ExprType.DOUBLE)) { return eval(param.asDouble()); + } else { + return ExprEval.ofMissing(); } - return ExprEval.of(null); } protected ExprEval eval(long param) @@ -317,13 +317,13 @@ protected final ExprEval eval(ExprEval x, ExprEval y) { // match the logic of BinaryEvalOpExprBase.eval, except there is no string handling so both strings is also null if (x.value() == null || y.value() == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } ExpressionType type = ExpressionTypeConversion.autoDetect(x, y); switch (type.getType()) { case STRING: - return ExprEval.of(null); + return ExprEval.ofString(null); case LONG: return eval(x.asLong(), y.asLong()); case DOUBLE: @@ -380,12 +380,12 @@ protected final ExprEval eval(ExprEval x, ExprEval y) // this is a copy of the logic of BivariateMathFunction for string handling, which itself is a // remix of BinaryEvalOpExprBase.eval modified so that string inputs are always null outputs if (x.value() == null || y.value() == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } ExpressionType type = ExpressionTypeConversion.autoDetect(x, y); if (type.is(ExprType.STRING)) { - return ExprEval.of(null); + return ExprEval.ofString(null); } return eval(x.asLong(), y.asLong()); } @@ -417,10 +417,10 @@ protected final ExprEval eval(ExprEval x, ExprEval y) { final String xString = x.asString(); if (xString == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } if (y.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofString(null); } return eval(xString, y.asLong()); } @@ -463,7 +463,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final ExprEval arrayExpr = getArrayArgument(args).eval(bindings); final ExprEval scalarExpr = getScalarArgument(args).eval(bindings); if (arrayExpr.asArray() == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } return doApply(arrayExpr, scalarExpr); } @@ -592,7 +592,7 @@ ExprEval doApply(ExprEval lhsExpr, ExprEval rhsExpr) final Object[] array2 = rhsExpr.asArray(); if (array1 == null) { - return ExprEval.of(null); + return ExprEval.ofMissing(); } if (array2 == null) { return lhsExpr; @@ -650,7 +650,7 @@ public ExpressionType getOutputType(Expr.InputBindingInspector inspector, List args, Expr.ObjectBinding bindings) { if (args.isEmpty()) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } // evaluate arguments and collect output type @@ -661,10 +661,12 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) ExprEval exprEval = expr.eval(bindings); ExpressionType exprType = exprEval.type(); + if (!isValidType(exprType)) { + throw validationFailed("does not accept %s types", exprType); + } + outputType = ExpressionTypeConversion.function(outputType, exprType); + if (exprEval.value() != null) { - if (isValidType(exprType)) { - outputType = ExpressionTypeConversion.function(outputType, exprType); - } evals.add(exprEval); } } @@ -676,19 +678,16 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) // databases (e.g., MySQL) return null if any expression is null. // https://www.postgresql.org/docs/9.5/functions-conditional.html // https://dev.mysql.com/doc/refman/8.0/en/comparison-operators.html#function_least - return ExprEval.of(null); + return ExprEval.ofType(outputType, null); } switch (outputType.getType()) { case DOUBLE: - //noinspection OptionalGetWithoutIsPresent (empty list handled earlier) return ExprEval.of(evals.stream().mapToDouble(ExprEval::asDouble).reduce(doubleReducer).getAsDouble()); case LONG: - //noinspection OptionalGetWithoutIsPresent (empty list handled earlier) return ExprEval.of(evals.stream().mapToLong(ExprEval::asLong).reduce(longReducer).getAsLong()); default: - //noinspection OptionalGetWithoutIsPresent (empty list handled earlier) - return ExprEval.of(evals.stream().map(ExprEval::asString).reduce(stringReducer).get()); + return ExprEval.ofString(evals.stream().map(ExprEval::asString).reduce(stringReducer).get()); } } @@ -700,7 +699,7 @@ private boolean isValidType(ExpressionType exprType) case STRING: return true; default: - throw validationFailed("does not accept %s types", exprType); + return false; } } } @@ -1501,7 +1500,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) ExprEval value1 = args.get(0).eval(bindings); if (value1.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } if (!value1.type().anyOf(ExprType.LONG, ExprType.DOUBLE)) { @@ -1551,7 +1550,7 @@ private ExprEval eval(ExprEval param, int scale) BigDecimal decimal = safeGetFromDouble(param.asDouble()); return ExprEval.of(decimal.setScale(scale, RoundingMode.HALF_UP).doubleValue()); } else { - return ExprEval.of(null); + return ExprEval.ofLong(null); } } @@ -1959,16 +1958,10 @@ public ExpressionType getOutputType(Expr.InputBindingInspector inspector, List args, final Expr.ObjectBinding bindings) } } - return ExprEval.of(null); + return ExprEval.ofMissing(); } @Override @@ -2213,7 +2207,7 @@ public ExprEval apply(final List args, final Expr.ObjectBinding bindings) } } - return ExprEval.of(null); + return ExprEval.ofMissing(); } @Override @@ -2614,15 +2608,15 @@ public String name() @Override public ExprEval apply(List args, Expr.ObjectBinding bindings) { - if (args.size() == 0) { - return ExprEval.of(null); + if (args.isEmpty()) { + return ExprEval.ofString(null); } else { // Pass first argument in to the constructor to provide StringBuilder a little extra sizing hint. String first = args.get(0).eval(bindings).asString(); if (first == null) { // Result of concatenation is null if any of the Values is null. // e.g. 'select CONCAT(null, "abc") as c;' will return null as per Standard SQL spec. - return ExprEval.of(null); + return ExprEval.ofString(null); } final StringBuilder builder = new StringBuilder(first); for (int i = 1; i < args.size(); i++) { @@ -2630,12 +2624,12 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) if (s == null) { // Result of concatenation is null if any of the Values is null. // e.g. 'select CONCAT(null, "abc") as c;' will return null as per Standard SQL spec. - return ExprEval.of(null); + return ExprEval.ofString(null); } else { builder.append(s); } } - return ExprEval.of(builder.toString()); + return ExprEval.ofString(builder.toString()); } } @@ -2711,7 +2705,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final String formatString = args.get(0).eval(bindings).asString(); if (formatString == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } final Object[] formatArgs = new Object[args.size() - 1]; @@ -2719,7 +2713,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) formatArgs[i - 1] = args.get(i).eval(bindings).value(); } - return ExprEval.of(StringUtils.nonStrictFormat(formatString, formatArgs)); + return ExprEval.ofString(StringUtils.nonStrictFormat(formatString, formatArgs)); } @Override @@ -2751,7 +2745,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final String needle = args.get(1).eval(bindings).asString(); if (haystack == null || needle == null) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } final int fromIndex; @@ -2793,7 +2787,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final String arg = args.get(0).eval(bindings).asString(); if (arg == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } // Behaves like SubstringDimExtractionFn, not SQL SUBSTRING @@ -2802,14 +2796,14 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) if (index < arg.length()) { if (length >= 0) { - return ExprEval.of(arg.substring(index, Math.min(index + length, arg.length()))); + return ExprEval.ofString(arg.substring(index, Math.min(index + length, arg.length()))); } else { - return ExprEval.of(arg.substring(index)); + return ExprEval.ofString(arg.substring(index)); } } else { // this is a behavior mismatch with SQL SUBSTRING to be consistent with SubstringDimExtractionFn // In SQL, something like 'select substring("abc", 4,5) as c;' will return an empty string - return ExprEval.of(null); + return ExprEval.ofString(null); } } @@ -2850,7 +2844,7 @@ protected ExprEval eval(String x, long y) throw validationFailed("needs a positive integer as the second argument"); } int len = x.length(); - return ExprEval.of(y < len ? x.substring(len - yInt) : x); + return ExprEval.ofString(y < len ? x.substring(len - yInt) : x); } } @@ -2876,7 +2870,7 @@ protected ExprEval eval(String x, long y) if (yInt < 0 || yInt != y) { throw validationFailed("needs a positive integer as the second argument"); } - return ExprEval.of(y < x.length() ? x.substring(0, yInt) : x); + return ExprEval.ofString(y < x.length() ? x.substring(0, yInt) : x); } } @@ -2895,9 +2889,9 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final String pattern = args.get(1).eval(bindings).asString(); final String replacement = args.get(2).eval(bindings).asString(); if (arg == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of(StringUtils.replace(arg, pattern, replacement)); + return ExprEval.ofString(StringUtils.replace(arg, pattern, replacement)); } @Override @@ -2927,9 +2921,9 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) { final String arg = args.get(0).eval(bindings).asString(); if (arg == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of(StringUtils.toLowerCase(arg)); + return ExprEval.ofString(StringUtils.toLowerCase(arg)); } @Override @@ -2959,9 +2953,9 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) { final String arg = args.get(0).eval(bindings).asString(); if (arg == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of(StringUtils.toUpperCase(arg)); + return ExprEval.ofString(StringUtils.toUpperCase(arg)); } @Override @@ -2996,11 +2990,8 @@ public ExpressionType getOutputType(Expr.InputBindingInspector inspector, List args, Expr.ObjectBinding bindings) String pad = args.get(2).eval(bindings).asString(); if (base == null || pad == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } else { - return ExprEval.of(len == 0 ? null : StringUtils.lpad(base, len, pad)); + return ExprEval.ofString(len == 0 ? null : StringUtils.lpad(base, len, pad)); } } @@ -3083,9 +3074,9 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) String pad = args.get(2).eval(bindings).asString(); if (base == null || pad == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } else { - return ExprEval.of(len == 0 ? null : StringUtils.rpad(base, len, pad)); + return ExprEval.ofString(len == 0 ? null : StringUtils.rpad(base, len, pad)); } } @@ -3116,11 +3107,8 @@ public String name() public ExprEval apply(List args, Expr.ObjectBinding bindings) { ExprEval value = args.get(0).eval(bindings); - if (!value.type().is(ExprType.STRING)) { - throw validationFailed( - "first argument should be a STRING but got %s instead", - value.type() - ); + if (value.value() == null) { + return ExprEval.ofLong(null); } DateTimes.UtcFormatter formatter = DateTimes.ISO_DATE_OPTIONAL_TIME; @@ -3194,7 +3182,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) DateTimeZone timeZone = DateTimes.inferTzFromString(args.get(2).eval(bindings).asString()); if (left == null || right == null) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } else { return ExprEval.of(DateTimes.subMonths(right, left, timeZone)); } @@ -3411,18 +3399,9 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) ExpressionType arrayElementType = null; - // Try first to determine the element type, only considering nonnull values. + // Determine the element type, considering null and nonnull values, for consistency with getOutputType. for (final ExprEval eval : outEval) { - if (eval.value() != null) { - arrayElementType = ExpressionTypeConversion.leastRestrictiveType(arrayElementType, eval.type()); - } - } - - if (arrayElementType == null) { - // Try again to determine the element type, this time considering nulls. - for (final ExprEval eval : outEval) { - arrayElementType = ExpressionTypeConversion.leastRestrictiveType(arrayElementType, eval.type()); - } + arrayElementType = ExpressionTypeConversion.leastRestrictiveType(arrayElementType, eval.type()); } final Object[] out = new Object[length]; @@ -3470,7 +3449,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final ExprEval expr = args.get(0).eval(bindings); final Object[] array = expr.asArray(); if (array == null) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } return ExprEval.ofLong(array.length); @@ -3536,7 +3515,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final ExprEval expr = args.get(0).eval(bindings); final String arrayString = expr.asString(); if (arrayString == null) { - return ExprEval.of(null); + return ExprEval.ofStringArray(null); } final String split = args.get(1).eval(bindings).asString(); @@ -3571,9 +3550,9 @@ ExprEval doApply(ExprEval arrayExpr, ExprEval scalarExpr) final String join = scalarExpr.asString(); final Object[] raw = arrayExpr.asArray(); if (raw == null || raw.length == 1 && raw[0] == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of( + return ExprEval.ofString( Arrays.stream(raw).map(String::valueOf).collect(Collectors.joining(join != null ? join : "")) ); } @@ -3603,7 +3582,7 @@ ExprEval doApply(ExprEval arrayExpr, ExprEval scalarExpr) if (array.length > position && position >= 0) { return ExprEval.ofType(arrayExpr.elementType(), array[position]); } - return ExprEval.of(null); + return ExprEval.ofType(arrayExpr.elementType(), null); } } @@ -3631,7 +3610,7 @@ ExprEval doApply(ExprEval arrayExpr, ExprEval scalarExpr) if (array.length > position && position >= 0) { return ExprEval.ofType(arrayExpr.elementType(), array[position]); } - return ExprEval.of(null); + return ExprEval.ofType(arrayExpr.elementType(), null); } } @@ -3803,7 +3782,7 @@ private static final class WithNullArray extends ScalarInArrayFunction @Override public ExprEval apply(List args, Expr.ObjectBinding bindings) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } } @@ -4569,7 +4548,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) final ExprEval expr = args.get(0).eval(bindings); final Object[] array = expr.asArray(); if (array == null) { - return ExprEval.of(null); + return ExprEval.ofArray(expr.asArrayType(), null); } final int start = args.get(1).eval(bindings).asInt(); @@ -4580,7 +4559,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) if (start < 0 || start > array.length || start > end) { // Arrays.copyOfRange will throw exception in these cases - return ExprEval.of(null); + return ExprEval.ofArray(expr.asArrayType(), null); } return ExprEval.ofArray(expr.asArrayType(), Arrays.copyOfRange(expr.asArray(), start, end)); @@ -4596,7 +4575,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) { final ExprEval valueParam = args.get(0).eval(bindings); if (valueParam.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofString(null); } /** @@ -4616,22 +4595,19 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) long precision = 2; if (args.size() > 1) { ExprEval precisionParam = args.get(1).eval(bindings); + if (precisionParam.value() == null) { + throw validationFailed("needs a LONG as its second argument but got null"); + } if (!precisionParam.type().is(ExprType.LONG)) { - throw validationFailed( - "needs a LONG as its second argument but got %s instead", - precisionParam.type() - ); + throw validationFailed("needs a LONG as its second argument but got %s instead", precisionParam.type()); } precision = precisionParam.asLong(); if (precision < 0 || precision > 3) { - throw validationFailed( - "given precision[%d] must be in the range of [0,3]", - precision - ); + throw validationFailed("given precision[%d] must be in the range of [0,3]", precision); } } - return ExprEval.of(HumanReadableBytes.format(valueParam.asLong(), precision, this.getUnitSystem())); + return ExprEval.ofString(HumanReadableBytes.format(valueParam.asLong(), precision, this.getUnitSystem())); } @Override diff --git a/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java b/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java index 97aad5add51b..60abd51a99da 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java @@ -135,7 +135,7 @@ public ExprEval eval(ObjectBinding bindings) } ExprEval ret = expr.eval(bindings); if (ret.value() == null) { - return ExprEval.of(null); + return ExprEval.ofType(ret.type(), null); } if (ret.type().is(ExprType.LONG)) { return ExprEval.of(-ret.asLong()); @@ -178,7 +178,7 @@ public ExprEval eval(ObjectBinding bindings) { ExprEval ret = expr.eval(bindings); if (ret.value() == null) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } return ExprEval.ofLongBoolean(!ret.asBoolean()); } diff --git a/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java b/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java index 115c9eed397d..0f08fca74f44 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java +++ b/processing/src/main/java/org/apache/druid/math/expr/vector/FallbackVectorProcessor.java @@ -44,7 +44,7 @@ public abstract class FallbackVectorProcessor implements ExprVectorProcessor< final Supplier> fn; final List adaptedArgs; - private final ExpressionType outputType; + protected final ExpressionType outputType; private FallbackVectorProcessor( final Supplier> fn, @@ -200,10 +200,10 @@ public ExprEvalVector evalVector(Expr.VectorInputBinding vectorBinding adaptedArg.setRowNumber(i); } - outValues[i] = fn.get().value(); + outValues[i] = fn.get().castTo(outputType).value(); } - return new ExprEvalObjectVector(outValues, getOutputType()); + return new ExprEvalObjectVector(outValues, outputType); } @Override @@ -368,7 +368,7 @@ public ExprEval eval(ObjectBinding bindings) final boolean isNull = results.getNullVector() != null && results.getNullVector()[rowNum]; return ExprEval.ofDouble(isNull ? null : results.getDoubleVector()[rowNum]); } else { - return ExprEval.ofType(type, results.getObjectVector()[rowNum]); + return ExprEval.bestEffortOf(results.getObjectVector()[rowNum]); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java index e56d6a1146fb..f6a83700b738 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java @@ -80,7 +80,7 @@ public ExprEval eval(final ObjectBinding bindings) case LONG: return evalAsLong(eval); default: - return ExprEval.of(null); + return ExprEval.ofString(null); } } @@ -100,19 +100,19 @@ private static ExprEval evalAsString(ExprEval eval) if (IPv4AddressExprUtils.isValidIPv4Address(eval.asString())) { return eval; } - return ExprEval.of(null); + return ExprEval.ofString(null); } private static ExprEval evalAsLong(ExprEval eval) { if (eval.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofString(null); } IPv4Address address = IPv4AddressExprUtils.parse(eval.asLong()); if (address == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of(IPv4AddressExprUtils.toString(address)); + return ExprEval.ofString(IPv4AddressExprUtils.toString(address)); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java index 7a1d1345a895..559623d170c4 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java @@ -92,7 +92,7 @@ private LookupExpr(final List args) @Override public ExprEval eval(final ObjectBinding bindings) { - return ExprEval.of(extractionFn.apply(arg.eval(bindings).asString())); + return ExprEval.ofString(extractionFn.apply(arg.eval(bindings).asString())); } @Nullable diff --git a/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java b/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java index 3939dd8c39ee..66b4321ee827 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java +++ b/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java @@ -447,7 +447,7 @@ public ExprEval eval(ObjectBinding bindings) if (valAtPath.type().isPrimitive() || valAtPath.type().isPrimitiveArray()) { return valAtPath; } - return ExprEval.of(null); + return ExprEval.ofMissing(); } @Nullable @@ -527,7 +527,7 @@ public ExprEval eval(ObjectBinding bindings) if (valAtPath.type().isPrimitive() || valAtPath.type().isPrimitiveArray()) { return castTo == null ? valAtPath : valAtPath.castTo(castTo); } - return castTo == null ? ExprEval.of(null) : ExprEval.ofType(castTo, null); + return castTo == null ? ExprEval.ofMissing() : ExprEval.ofType(castTo, null); } @Nullable diff --git a/processing/src/main/java/org/apache/druid/query/expression/RegexpExtractExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/RegexpExtractExprMacro.java index 9d95ec1e9f1d..6a690787735d 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/RegexpExtractExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/RegexpExtractExprMacro.java @@ -80,11 +80,11 @@ public ExprEval eval(final ObjectBinding bindings) if (s == null) { // True nulls do not match anything. - return ExprEval.of(null); + return ExprEval.ofString(null); } else { final Matcher matcher = pattern.matcher(s); final String retVal = matcher.find() ? matcher.group(index) : null; - return ExprEval.of(retVal); + return ExprEval.ofString(retVal); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/RegexpReplaceExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/RegexpReplaceExprMacro.java index cf7dbdd7de87..835fdcd6c915 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/RegexpReplaceExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/RegexpReplaceExprMacro.java @@ -105,17 +105,17 @@ private RegexpReplaceExpr(List args) public ExprEval eval(final ObjectBinding bindings) { if (pattern == null || replacement == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } final String s = arg.eval(bindings).asString(); if (s == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } else { final Matcher matcher = pattern.matcher(s); final String retVal = matcher.replaceAll(replacement); - return ExprEval.of(retVal); + return ExprEval.ofString(retVal); } } } @@ -139,11 +139,11 @@ public ExprEval eval(final ObjectBinding bindings) final String replacement = args.get(2).eval(bindings).asString(); if (s == null || pattern == null || replacement == null) { - return ExprEval.of(null); + return ExprEval.ofString(null); } else { final Matcher matcher = Pattern.compile(pattern).matcher(s); final String retVal = matcher.replaceAll(replacement); - return ExprEval.of(retVal); + return ExprEval.ofString(retVal); } } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java index 3c5102ae7a2c..782725634083 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java @@ -73,7 +73,7 @@ public ExprEval eval(final ObjectBinding bindings) ExprEval eval = args.get(0).eval(bindings); if (eval.isNumericNull()) { // Return null if the argument if null. - return ExprEval.of(null); + return ExprEval.ofLong(null); } long argTime = eval.asLong(); long bucketStartTime = granularity.bucketStart(argTime); diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampExtractExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampExtractExprMacro.java index 2fac53ec8d58..4dab92c5f771 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampExtractExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampExtractExprMacro.java @@ -171,7 +171,7 @@ public ExprEval eval(final ObjectBinding bindings) Object val = args.get(0).eval(bindings).value(); if (val == null) { // Return null if the argument if null. - return ExprEval.of(null); + return ExprEval.ofType(getOutputExpressionType(unit), null); } final DateTime dateTime = new DateTime(val, chronology); return getExprEval(dateTime, unit); @@ -202,7 +202,7 @@ public ExprEval eval(final ObjectBinding bindings) Object val = args.get(0).eval(bindings).value(); if (val == null) { // Return null if the argument if null. - return ExprEval.of(null); + return ExprEval.ofType(getOutputExpressionType(unit), null); } final ISOChronology chronology = computeChronology(args, bindings); final DateTime dateTime = new DateTime(val, chronology); diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java index e71412c6ddac..d81fc5277614 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java @@ -111,7 +111,7 @@ public ExprEval eval(final ObjectBinding bindings) ExprEval eval = args.get(0).eval(bindings); if (eval.isNumericNull()) { // Return null if the argument if null. - return ExprEval.of(null); + return ExprEval.ofLong(null); } return ExprEval.of(granularity.bucketStart(eval.asLong())); } diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampFormatExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampFormatExprMacro.java index 299cb19fcfc5..8b0e6e58bec1 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampFormatExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampFormatExprMacro.java @@ -82,9 +82,9 @@ public ExprEval eval(final ObjectBinding bindings) ExprEval eval = arg.eval(bindings); if (eval.isNumericNull()) { // Return null if the argument if null. - return ExprEval.of(null); + return ExprEval.ofString(null); } - return ExprEval.of(formatter.print(arg.eval(bindings).asLong())); + return ExprEval.ofString(formatter.print(arg.eval(bindings).asLong())); } @Nullable diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampParseExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampParseExprMacro.java index dc1fc3b73ff7..070254cf69aa 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampParseExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampParseExprMacro.java @@ -78,7 +78,7 @@ public ExprEval eval(final ObjectBinding bindings) { final String value = arg.eval(bindings).asString(); if (value == null) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } try { @@ -87,7 +87,7 @@ public ExprEval eval(final ObjectBinding bindings) catch (IllegalArgumentException e) { // Catch exceptions potentially thrown by formatter.parseDateTime. Our docs say that unparseable timestamps // are returned as nulls. - return ExprEval.of(null); + return ExprEval.ofLong(null); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampShiftExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampShiftExprMacro.java index 3e2535d787f0..dbc86b6b77d1 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampShiftExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampShiftExprMacro.java @@ -101,7 +101,7 @@ public ExprEval eval(final ObjectBinding bindings) { ExprEval timestamp = args.get(0).eval(bindings); if (timestamp.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } return ExprEval.of(chronology.add(period, timestamp.asLong(), step)); } @@ -144,7 +144,7 @@ public ExprEval eval(final ObjectBinding bindings) { ExprEval timestamp = args.get(0).eval(bindings); if (timestamp.isNumericNull()) { - return ExprEval.of(null); + return ExprEval.ofLong(null); } final Period period = getPeriod(args, bindings); final Chronology chronology = getTimeZone(args, bindings); diff --git a/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java index 33bb58cb7b05..62997e1be795 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java @@ -157,7 +157,7 @@ public ExprEval eval(final ObjectBinding bindings) if (start == 0 && end == s.length()) { return stringEval; } else { - return ExprEval.of(s.substring(start, end)); + return ExprEval.ofString(s.substring(start, end)); } } @@ -232,7 +232,7 @@ public ExprEval eval(final ObjectBinding bindings) if (start == 0 && end == s.length()) { return stringEval; } else { - return ExprEval.of(s.substring(start, end)); + return ExprEval.ofString(s.substring(start, end)); } } diff --git a/processing/src/test/java/org/apache/druid/math/expr/ConstantExprTest.java b/processing/src/test/java/org/apache/druid/math/expr/ConstantExprTest.java index a7812f2d160e..c61b1967896d 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/ConstantExprTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/ConstantExprTest.java @@ -102,8 +102,8 @@ public void testNullDoubleExpr() new NullDoubleExpr(), "null", "null", - // the expressions 'null' is always parsed as a StringExpr(null) - new StringExpr(null) + // the expressions 'null' is always parsed as a NullLongExpr() + new NullLongExpr() ); } @@ -114,8 +114,8 @@ public void testNullLongExpr() new NullLongExpr(), "null", "null", - // the expressions 'null' is always parsed as a StringExpr(null) - new StringExpr(null) + // the expressions 'null' is always parsed as a NullLongExpr() + new NullLongExpr() ); } @@ -148,7 +148,8 @@ public void testStringNull() new StringExpr(null), null, "null", - new StringExpr(null) + // the expressions 'null' is always parsed as a NullLongExpr() + new NullLongExpr() ); } diff --git a/processing/src/test/java/org/apache/druid/math/expr/EvalTest.java b/processing/src/test/java/org/apache/druid/math/expr/EvalTest.java index 047c73ffe2e7..e0f64a7c36bd 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/EvalTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/EvalTest.java @@ -522,7 +522,7 @@ public void testNestedDataCast() Assert.assertEquals("hello", ExprEval.ofComplex(ExpressionType.NESTED_DATA, "hello").asString()); Assert.assertEquals(ExpressionType.STRING, cast.type()); - cast = ExprEval.of("hello").castTo(ExpressionType.NESTED_DATA); + cast = ExprEval.ofString("hello").castTo(ExpressionType.NESTED_DATA); Assert.assertEquals("hello", cast.value()); Assert.assertEquals(ExpressionType.NESTED_DATA, cast.type()); @@ -613,7 +613,7 @@ public void testNestedDataCast() new Object[]{1234L}, cast.asArray() ); - cast = ExprEval.of("hello").castTo(nestedArray); + cast = ExprEval.ofString("hello").castTo(nestedArray); Assert.assertEquals(nestedArray, cast.type()); Assert.assertArrayEquals( new Object[]{"hello"}, @@ -770,7 +770,7 @@ public void testNonNestedComplexCastThrows() t = Assert.assertThrows( IllegalArgumentException.class, - () -> ExprEval.of("hello").castTo(someComplex) + () -> ExprEval.ofString("hello").castTo(someComplex) ); Assert.assertEquals("Invalid type, cannot cast [STRING] to [COMPLEX]", t.getMessage()); @@ -820,9 +820,9 @@ public void testIsNumericNull() Assert.assertFalse(ExprEval.ofDouble(1.0).isNumericNull()); Assert.assertTrue(ExprEval.ofDouble(null).isNumericNull()); - Assert.assertTrue(ExprEval.of(null).isNumericNull()); - Assert.assertTrue(ExprEval.of("one").isNumericNull()); - Assert.assertFalse(ExprEval.of("1").isNumericNull()); + Assert.assertTrue(ExprEval.ofString(null).isNumericNull()); + Assert.assertTrue(ExprEval.ofString("one").isNumericNull()); + Assert.assertFalse(ExprEval.ofString("1").isNumericNull()); Assert.assertFalse(ExprEval.ofLongArray(new Long[]{1L}).isNumericNull()); Assert.assertTrue(ExprEval.ofLongArray(new Long[]{null, 2L, 3L}).isNumericNull()); diff --git a/processing/src/test/java/org/apache/druid/math/expr/ExprEvalTest.java b/processing/src/test/java/org/apache/druid/math/expr/ExprEvalTest.java index 528d4844bf1b..04ec0492bb2a 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/ExprEvalTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/ExprEvalTest.java @@ -77,7 +77,7 @@ public void testStringSerdeTooBig() 10, 16 )); - assertExpr(0, ExprEval.of("hello world"), 10); + assertExpr(0, ExprEval.ofString("hello world"), 10); } @@ -541,7 +541,7 @@ public void test_coerceListToArray() @Test public void testCastString() { - ExprEval eval = ExprEval.of("hello"); + ExprEval eval = ExprEval.ofString("hello"); ExprEval cast = eval.castTo(ExpressionType.DOUBLE); Assert.assertNull(cast.value()); @@ -564,7 +564,7 @@ public void testCastString() cast = eval.castTo(ExpressionTypeFactory.getInstance().ofArray(ExpressionType.NESTED_DATA)); Assert.assertArrayEquals(new Object[]{"hello"}, (Object[]) cast.value()); - eval = ExprEval.of("1234.3"); + eval = ExprEval.ofString("1234.3"); cast = eval.castTo(ExpressionType.DOUBLE); Assert.assertEquals(1234.3, cast.value()); diff --git a/processing/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/processing/src/test/java/org/apache/druid/math/expr/FunctionTest.java index 22030ac4928f..51068feefd44 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/FunctionTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/FunctionTest.java @@ -953,7 +953,7 @@ public void testSizeFormatWithEdgeCases() } @Test - public void testSizeForatInvalidArgumentType() + public void testSizeFormatInvalidArgumentType() { // x = "foo" Throwable t = Assert.assertThrows( @@ -981,7 +981,7 @@ public void testSizeForatInvalidArgumentType() .eval(bestEffortBindings) ); Assert.assertEquals( - "Function[human_readable_binary_byte_format] needs a LONG as its second argument but got STRING instead", + "Function[human_readable_binary_byte_format] needs a LONG as its second argument but got null", t.getMessage() ); } diff --git a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java index 088df4c9ef58..267ce565ff94 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java @@ -396,8 +396,8 @@ public void testApplyFunctions() @Test public void testEvalAutoConversion() { - final ExprEval nullStringEval = ExprEval.of(null); - final ExprEval stringEval = ExprEval.of("wat"); + final ExprEval nullStringEval = ExprEval.ofString(null); + final ExprEval stringEval = ExprEval.ofString("wat"); final ExprEval longEval = ExprEval.of(1L); final ExprEval doubleEval = ExprEval.of(1.0); final ExprEval arrayEval = ExprEval.ofLongArray(new Long[]{1L, 2L, 3L}); diff --git a/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java b/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java index 11580eb9f5bc..0413ec43cb90 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java @@ -387,6 +387,30 @@ public void testArrayFns() } } + @Test + public void testReduceFns() + { + try { + ExpressionProcessing.initializeForFallback(); + testExpression("greatest(s1, s2)", types); + testExpression("greatest(l1, l2)", types); + testExpression("greatest(l1, nonexistent)", types); + testExpression("greatest(d1, d2)", types); + testExpression("greatest(l1, d2)", types); + testExpression("greatest(s1, l2)", types); + + testExpression("least(s1, s2)", types); + testExpression("least(l1, l2)", types); + testExpression("least(l1, nonexistent)", types); + testExpression("least(d1, d2)", types); + testExpression("least(l1, d2)", types); + testExpression("least(s1, l2)", types); + } + finally { + ExpressionProcessing.initializeForTests(); + } + } + @Test public void testJsonFns() { @@ -532,11 +556,14 @@ public ExpressionType getType(String name) NonnullPair bindings = makeRandomizedBindings(VECTOR_SIZE, types); ExprEvalVector vectorEval = processor.evalVector(bindings.rhs); final Object[] vectorVals = vectorEval.getObjectVector(); + if (outputType != null) { + Assert.assertEquals("vector eval type", outputType, vectorEval.getType()); + } for (int i = 0; i < VECTOR_SIZE; i++) { ExprEval eval = parsed.eval(bindings.lhs[i]); // 'null' expressions can have an output type of null, but still evaluate in default mode, so skip type checks - if (outputType != null && !eval.isNumericNull()) { - Assert.assertEquals(eval.type(), outputType); + if (outputType != null && eval.value() != null) { + Assert.assertEquals("nonvector eval type", eval.type(), outputType); } if (outputType != null && outputType.isArray()) { Assert.assertArrayEquals( @@ -637,6 +664,7 @@ static NonnullPair populateBindin { SettableVectorInputBinding vectorBinding = new SettableVectorInputBinding(vectorSize); SettableObjectBinding[] objectBindings = new SettableObjectBinding[vectorSize]; + Expr.InputBindingInspector inspector = InputBindings.inspectorFromTypeMap(types); for (Map.Entry entry : types.entrySet()) { boolean[] nulls = new boolean[vectorSize]; @@ -648,7 +676,7 @@ static NonnullPair populateBindin nulls[i] = nullsFn.getAsBoolean(); longs[i] = nulls[i] ? 0L : longsFn.getAsLong(); if (objectBindings[i] == null) { - objectBindings[i] = new SettableObjectBinding(); + objectBindings[i] = new SettableObjectBinding().withInspector(inspector); } objectBindings[i].withBinding(entry.getKey(), nulls[i] ? null : longs[i]); } @@ -660,7 +688,7 @@ static NonnullPair populateBindin nulls[i] = nullsFn.getAsBoolean(); doubles[i] = nulls[i] ? 0.0 : doublesFn.getAsDouble(); if (objectBindings[i] == null) { - objectBindings[i] = new SettableObjectBinding(); + objectBindings[i] = new SettableObjectBinding().withInspector(inspector); } objectBindings[i].withBinding(entry.getKey(), nulls[i] ? null : doubles[i]); } @@ -676,7 +704,7 @@ static NonnullPair populateBindin strings[i] = nulls[i] ? null : String.valueOf(stringFn.get()); } if (objectBindings[i] == null) { - objectBindings[i] = new SettableObjectBinding(); + objectBindings[i] = new SettableObjectBinding().withInspector(inspector); } objectBindings[i].withBinding(entry.getKey(), nulls[i] ? null : strings[i]); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacroTest.java index bd3298e59de6..502518953851 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacroTest.java @@ -33,15 +33,15 @@ public class IPv4AddressMatchExprMacroTest extends MacroTestBase { - private static final Expr IPV4 = ExprEval.of("192.168.0.1").toExpr(); + private static final Expr IPV4 = ExprEval.ofString("192.168.0.1").toExpr(); private static final Expr IPV4_LONG = ExprEval.of(3232235521L).toExpr(); - private static final Expr IPV4_UINT = ExprEval.of("3232235521").toExpr(); - private static final Expr IPV4_NETWORK = ExprEval.of("192.168.0.0").toExpr(); - private static final Expr IPV4_BROADCAST = ExprEval.of("192.168.255.255").toExpr(); - private static final Expr IPV6_COMPATIBLE = ExprEval.of("::192.168.0.1").toExpr(); - private static final Expr IPV6_MAPPED = ExprEval.of("::ffff:192.168.0.1").toExpr(); - private static final Expr SUBNET_192_168 = ExprEval.of("192.168.0.0/16").toExpr(); - private static final Expr SUBNET_10 = ExprEval.of("10.0.0.0/8").toExpr(); + private static final Expr IPV4_UINT = ExprEval.ofString("3232235521").toExpr(); + private static final Expr IPV4_NETWORK = ExprEval.ofString("192.168.0.0").toExpr(); + private static final Expr IPV4_BROADCAST = ExprEval.ofString("192.168.255.255").toExpr(); + private static final Expr IPV6_COMPATIBLE = ExprEval.ofString("::192.168.0.1").toExpr(); + private static final Expr IPV6_MAPPED = ExprEval.ofString("::ffff:192.168.0.1").toExpr(); + private static final Expr SUBNET_192_168 = ExprEval.ofString("192.168.0.0/16").toExpr(); + private static final Expr SUBNET_10 = ExprEval.ofString("10.0.0.0/8").toExpr(); private static final Expr NOT_LITERAL = Parser.parse("\"notliteral\"", ExprMacroTable.nil()); public IPv4AddressMatchExprMacroTest() @@ -78,14 +78,14 @@ public void testSubnetArgInvalid() { expectException(IllegalArgumentException.class, "subnet arg has an invalid format"); - Expr invalidSubnet = ExprEval.of("192.168.0.1/invalid").toExpr(); + Expr invalidSubnet = ExprEval.ofString("192.168.0.1/invalid").toExpr(); apply(Arrays.asList(IPV4, invalidSubnet)); } @Test public void testNullStringArg() { - Expr nullString = ExprEval.of(null).toExpr(); + Expr nullString = ExprEval.ofString(null).toExpr(); Assert.assertFalse(eval(nullString, SUBNET_192_168)); } @@ -142,7 +142,7 @@ public void testNotMatchingStringArgIPv6Compatible() @Test public void testNotIpAddress() { - Expr notIpAddress = ExprEval.of("druid.apache.org").toExpr(); + Expr notIpAddress = ExprEval.ofString("druid.apache.org").toExpr(); Assert.assertFalse(eval(notIpAddress, SUBNET_192_168)); } @@ -182,26 +182,26 @@ public void testInclusive() @Test public void testMatchesPrefix() { - Assert.assertTrue(eval(ExprEval.of("192.168.1.250").toExpr(), ExprEval.of("192.168.1.251/31").toExpr())); - Assert.assertFalse(eval(ExprEval.of("192.168.1.240").toExpr(), ExprEval.of("192.168.1.251/31").toExpr())); - Assert.assertFalse(eval(ExprEval.of("192.168.1.250").toExpr(), ExprEval.of("192.168.1.251/32").toExpr())); - Assert.assertTrue(eval(ExprEval.of("192.168.1.251").toExpr(), ExprEval.of("192.168.1.251/32").toExpr())); + Assert.assertTrue(eval(ExprEval.ofString("192.168.1.250").toExpr(), ExprEval.ofString("192.168.1.251/31").toExpr())); + Assert.assertFalse(eval(ExprEval.ofString("192.168.1.240").toExpr(), ExprEval.ofString("192.168.1.251/31").toExpr())); + Assert.assertFalse(eval(ExprEval.ofString("192.168.1.250").toExpr(), ExprEval.ofString("192.168.1.251/32").toExpr())); + Assert.assertTrue(eval(ExprEval.ofString("192.168.1.251").toExpr(), ExprEval.ofString("192.168.1.251/32").toExpr())); Assert.assertTrue(eval( ExprEval.of(IPv4AddressExprUtils.parse("192.168.1.250").longValue()).toExpr(), - ExprEval.of("192.168.1.251/31").toExpr() + ExprEval.ofString("192.168.1.251/31").toExpr() )); Assert.assertFalse(eval( ExprEval.of(IPv4AddressExprUtils.parse("192.168.1.240").longValue()).toExpr(), - ExprEval.of("192.168.1.251/31").toExpr() + ExprEval.ofString("192.168.1.251/31").toExpr() )); Assert.assertFalse(eval( ExprEval.of(IPv4AddressExprUtils.parse("192.168.1.250").longValue()).toExpr(), - ExprEval.of("192.168.1.251/32").toExpr() + ExprEval.ofString("192.168.1.251/32").toExpr() )); Assert.assertTrue(eval( ExprEval.of(IPv4AddressExprUtils.parse("192.168.1.251").longValue()).toExpr(), - ExprEval.of("192.168.1.251/32").toExpr() + ExprEval.ofString("192.168.1.251/32").toExpr() )); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressParseExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressParseExprMacroTest.java index d48955d02cea..dbafc12a07a6 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressParseExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressParseExprMacroTest.java @@ -30,7 +30,7 @@ public class IPv4AddressParseExprMacroTest extends MacroTestBase { - private static final Expr VALID = ExprEval.of("192.168.0.1").toExpr(); + private static final Expr VALID = ExprEval.ofString("192.168.0.1").toExpr(); private static final long EXPECTED = 3232235521L; public IPv4AddressParseExprMacroTest() @@ -57,7 +57,7 @@ public void testTooManyArgs() @Test public void testnullStringArg() { - Expr nullString = ExprEval.of(null).toExpr(); + Expr nullString = ExprEval.ofString(null).toExpr(); Assert.assertNull(eval(nullString)); } @@ -78,21 +78,21 @@ public void testInvalidArgType() @Test public void testInvalidStringArgNotIPAddress() { - Expr notIpAddress = ExprEval.of("druid.apache.org").toExpr(); + Expr notIpAddress = ExprEval.ofString("druid.apache.org").toExpr(); Assert.assertNull(eval(notIpAddress)); } @Test public void testInvalidStringArgIPv6Compatible() { - Expr ipv6Compatible = ExprEval.of("::192.168.0.1").toExpr(); + Expr ipv6Compatible = ExprEval.ofString("::192.168.0.1").toExpr(); Assert.assertNull(eval(ipv6Compatible)); } @Test public void testValidStringArgIPv6Mapped() { - Expr ipv6Mapped = ExprEval.of("::ffff:192.168.0.1").toExpr(); + Expr ipv6Mapped = ExprEval.ofString("::ffff:192.168.0.1").toExpr(); Assert.assertNull(eval(ipv6Mapped)); } @@ -105,7 +105,7 @@ public void testValidStringArgIPv4() @Test public void testValidStringArgUnsignedInt() { - Expr unsignedInt = ExprEval.of("3232235521").toExpr(); + Expr unsignedInt = ExprEval.ofString("3232235521").toExpr(); Assert.assertNull(eval(unsignedInt)); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacroTest.java index aefc4f24bea1..c369bfe7b547 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacroTest.java @@ -105,28 +105,28 @@ public void testInvalidLongArgTooLarge() @Test public void testNullStringArg() { - Expr nullString = ExprEval.of(null).toExpr(); + Expr nullString = ExprEval.ofString(null).toExpr(); Assert.assertNull(eval(nullString)); } @Test public void testInvalidStringArgNotIPAddress() { - Expr notIpAddress = ExprEval.of("druid.apache.org").toExpr(); + Expr notIpAddress = ExprEval.ofString("druid.apache.org").toExpr(); Assert.assertNull(eval(notIpAddress)); } @Test public void testInvalidStringArgIPv6Compatible() { - Expr ipv6Compatible = ExprEval.of("::192.168.0.1").toExpr(); + Expr ipv6Compatible = ExprEval.ofString("::192.168.0.1").toExpr(); Assert.assertNull(eval(ipv6Compatible)); } @Test public void testValidStringArgIPv6Mapped() { - Expr ipv6Mapped = ExprEval.of("::ffff:192.168.0.1").toExpr(); + Expr ipv6Mapped = ExprEval.ofString("::ffff:192.168.0.1").toExpr(); Assert.assertNull(eval(ipv6Mapped)); } @@ -139,7 +139,7 @@ public void testValidStringArgIPv4() @Test public void testValidStringArgUnsignedInt() { - Expr unsignedInt = ExprEval.of("3232235521").toExpr(); + Expr unsignedInt = ExprEval.ofString("3232235521").toExpr(); Assert.assertNull(eval(unsignedInt)); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/IPv6AddressMatchExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/IPv6AddressMatchExprMacroTest.java index d8371298dcfa..fd04e634decc 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/IPv6AddressMatchExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/IPv6AddressMatchExprMacroTest.java @@ -32,8 +32,8 @@ public class IPv6AddressMatchExprMacroTest extends MacroTestBase { - private static final Expr IPV6 = ExprEval.of("201:ef:168::").toExpr(); - private static final Expr IPV6_CIDR = ExprEval.of("201:ef:168::/32").toExpr(); + private static final Expr IPV6 = ExprEval.ofString("201:ef:168::").toExpr(); + private static final Expr IPV6_CIDR = ExprEval.ofString("201:ef:168::/32").toExpr(); public IPv6AddressMatchExprMacroTest() { @@ -50,7 +50,7 @@ public void testTooFewArgs() @Test public void testTooManyArgs() { - Expr extraArgument = ExprEval.of("An extra argument").toExpr(); + Expr extraArgument = ExprEval.ofString("An extra argument").toExpr(); expectException(ExpressionValidationException.class, "requires 2 arguments"); apply(Arrays.asList(IPV6, IPV6_CIDR, extraArgument)); } @@ -59,14 +59,14 @@ public void testTooManyArgs() public void testSubnetArgInvalid() { expectException(ExpressionProcessingException.class, "Function[ipv6_match] failed to parse address"); - Expr invalidSubnet = ExprEval.of("201:ef:168::/invalid").toExpr(); + Expr invalidSubnet = ExprEval.ofString("201:ef:168::/invalid").toExpr(); apply(Arrays.asList(IPV6, invalidSubnet)); } @Test public void testNullStringArg() { - Expr nullString = ExprEval.of(null).toExpr(); + Expr nullString = ExprEval.ofString(null).toExpr(); Assert.assertFalse(eval(nullString, IPV6_CIDR)); } @@ -79,14 +79,14 @@ public void testMatchingStringArgIPv6() @Test public void testNotMatchingStringArgIPv6() { - Expr nonMatchingIpv6 = ExprEval.of("2002:ef:168::").toExpr(); + Expr nonMatchingIpv6 = ExprEval.ofString("2002:ef:168::").toExpr(); Assert.assertFalse(eval(nonMatchingIpv6, IPV6_CIDR)); } @Test public void testNotIpAddress() { - Expr notIpAddress = ExprEval.of("druid.apache.org").toExpr(); + Expr notIpAddress = ExprEval.ofString("druid.apache.org").toExpr(); Assert.assertFalse(eval(notIpAddress, IPV6_CIDR)); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java index cef936729e7c..5d1c477903a9 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java @@ -93,7 +93,7 @@ private List getArgs(List args) { return args.stream().map(a -> { if (a != null && a instanceof String) { - return ExprEval.of(a.toString()).toExpr(); + return ExprEval.ofString(a.toString()).toExpr(); } return ExprEval.bestEffortOf(null).toExpr(); }).collect(Collectors.toList()); diff --git a/processing/src/test/java/org/apache/druid/query/expression/TimestampExtractExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/TimestampExtractExprMacroTest.java index 94f9e74439e5..059fcf234ff0 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/TimestampExtractExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/TimestampExtractExprMacroTest.java @@ -49,8 +49,8 @@ public void testApplyExtractDecadeShouldExtractTheCorrectDecade() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2001-02-16").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.DECADE.toString()).toExpr() + ExprEval.ofString("2001-02-16").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.DECADE.toString()).toExpr() )); Assert.assertEquals(200, expression.eval(InputBindings.nilBindings()).asInt()); } @@ -60,8 +60,8 @@ public void testApplyExtractCenturyShouldExtractTheCorrectCentury() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2000-12-16").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr() + ExprEval.ofString("2000-12-16").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr() )); Assert.assertEquals(20, expression.eval(InputBindings.nilBindings()).asInt()); } @@ -71,8 +71,8 @@ public void testApplyExtractCenturyShouldBeTwentyFirstCenturyIn2001() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2001-02-16").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr() + ExprEval.ofString("2001-02-16").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr() )); Assert.assertEquals(21, expression.eval(InputBindings.nilBindings()).asInt()); } @@ -82,8 +82,8 @@ public void testApplyExtractMilleniumShouldExtractTheCorrectMillenium() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2000-12-16").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr() + ExprEval.ofString("2000-12-16").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr() )); Assert.assertEquals(2, expression.eval(InputBindings.nilBindings()).asInt()); } @@ -93,8 +93,8 @@ public void testApplyExtractMilleniumShouldBeThirdMilleniumIn2001() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2001-02-16").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr() + ExprEval.ofString("2001-02-16").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr() )); Assert.assertEquals(3, expression.eval(InputBindings.nilBindings()).asInt()); } @@ -104,9 +104,9 @@ public void testApplyExtractDowWithTimeZoneShouldBeFriday() { Expr expression = target.apply( ImmutableList.of( - ExprEval.of("2023-12-15").toExpr(), - ExprEval.of(TimestampExtractExprMacro.Unit.DOW.toString()).toExpr(), - ExprEval.of("UTC").toExpr() + ExprEval.ofString("2023-12-15").toExpr(), + ExprEval.ofString(TimestampExtractExprMacro.Unit.DOW.toString()).toExpr(), + ExprEval.ofString("UTC").toExpr() )); Assert.assertEquals(5, expression.eval(InputBindings.nilBindings()).asInt()); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/TimestampShiftMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/TimestampShiftMacroTest.java index ef5985b440d0..04e09bc9f8af 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/TimestampShiftMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/TimestampShiftMacroTest.java @@ -73,7 +73,7 @@ public void testTwoArguments() apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1M").toExpr() + ExprEval.ofString("P1M").toExpr() )); } @@ -84,10 +84,10 @@ public void testMoreThanFourArguments() apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1M").toExpr(), - ExprEval.of("1").toExpr(), - ExprEval.of("+08:00").toExpr(), - ExprEval.of("extra").toExpr() + ExprEval.ofString("P1M").toExpr(), + ExprEval.ofString("1").toExpr(), + ExprEval.ofString("+08:00").toExpr(), + ExprEval.ofString("extra").toExpr() )); } @@ -98,7 +98,7 @@ public void testZeroStep() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1M").toExpr(), + ExprEval.ofString("P1M").toExpr(), ExprEval.of(step).toExpr() )); @@ -115,7 +115,7 @@ public void testPositiveStep() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1M").toExpr(), + ExprEval.ofString("P1M").toExpr(), ExprEval.of(step).toExpr() )); @@ -132,7 +132,7 @@ public void testNegativeStep() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1M").toExpr(), + ExprEval.ofString("P1M").toExpr(), ExprEval.of(step).toExpr() )); @@ -148,7 +148,7 @@ public void testPeriodMinute() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("PT1M").toExpr(), + ExprEval.ofString("PT1M").toExpr(), ExprEval.of(1).toExpr() )); @@ -164,7 +164,7 @@ public void testPeriodDay() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1D").toExpr(), + ExprEval.ofString("P1D").toExpr(), ExprEval.of(1).toExpr() )); @@ -180,9 +180,9 @@ public void testPeriodYearAndTimeZone() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1Y").toExpr(), + ExprEval.ofString("P1Y").toExpr(), ExprEval.of(1).toExpr(), - ExprEval.of("America/Los_Angeles").toExpr() + ExprEval.ofString("America/Los_Angeles").toExpr() )); Assert.assertEquals( @@ -198,9 +198,9 @@ public void testDynamicExpression() Expr expr = apply( ImmutableList.of( ExprEval.of(timestamp.getMillis()).toExpr(), - ExprEval.of("P1Y").toExpr(), + ExprEval.ofString("P1Y").toExpr(), Parser.parse("\"step\"", ExprMacroTable.nil()), // "step" is not a literal - ExprEval.of("America/Los_Angeles").toExpr() + ExprEval.ofString("America/Los_Angeles").toExpr() )); final int step = 3; @@ -235,7 +235,7 @@ public void testNull() Expr expr = apply( ImmutableList.of( ExprEval.ofLong(null).toExpr(), - ExprEval.of("P1M").toExpr(), + ExprEval.ofString("P1M").toExpr(), ExprEval.of(1L).toExpr() ) ); diff --git a/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java b/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java index 2a155ffde9df..4946bc962582 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java @@ -1624,7 +1624,7 @@ public void test_equals() "optimizedFilterNoIncludeUnknown" ) .withPrefabValues(ColumnType.class, ColumnType.STRING, ColumnType.DOUBLE) - .withPrefabValues(ExprEval.class, ExprEval.of("hello"), ExprEval.of(1.0)) + .withPrefabValues(ExprEval.class, ExprEval.ofString("hello"), ExprEval.of(1.0)) .withIgnoredFields( "predicateFactory", "optimizedFilterIncludeUnknown", diff --git a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java index dee5b5247949..bf1a8794d1cf 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java @@ -213,7 +213,7 @@ public void testMvOverlap() { assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, '1')"), List.of("0")); assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, '4')"), List.of("4", "5")); - assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, array(1, 2, 3, 4)"), List.of("0", "3", "4", "5")); + assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, array(1, 2, 3, 4))"), List.of("0", "3", "4", "5")); assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, dim3)"), List.of("5", "9")); assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, null)"), List.of("1", "6", "7", "8")); assertFilterMatchesSkipVectorize(edf("mv_overlap(dim4, [])"), List.of()); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java index 650135ef43d8..51cceac7aa1a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java @@ -43,6 +43,7 @@ import org.apache.druid.query.LookupDataSource; import org.apache.druid.query.Order; import org.apache.druid.query.Query; +import org.apache.druid.query.QueryContext; import org.apache.druid.query.QueryContexts; import org.apache.druid.query.QueryDataSource; import org.apache.druid.query.QueryException; @@ -4521,11 +4522,15 @@ public void testTopNFilterJoin(Map queryContext) @ParameterizedTest(name = "{0}") public void testTopNFilterJoinWithProjection(Map queryContext) { - // Cannot vectorize JOIN operator. - cannotVectorize(); + if (QueryContext.of(queryContext).getEnableRewriteJoinToFilter()) { + // Join is eliminated. Cannot vectorize substring function unless fallback vectorization is on. + cannotVectorizeUnlessFallback(); + } else { + // Cannot vectorize the join or the substring function. + cannotVectorize(); + } // Filters on top N values of some dimension by using an inner join. Also projects the outer dimension. - testQuery( "SELECT SUBSTRING(t1.dim1, 1, 10), SUM(t1.cnt)\n" + "FROM druid.foo t1\n" diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteLookupFunctionQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteLookupFunctionQueryTest.java index 0553164a3121..f4b65da38850 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteLookupFunctionQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteLookupFunctionQueryTest.java @@ -102,7 +102,7 @@ public void testFilterEquals() @Test public void testFilterLookupOfFunction() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("LOOKUP(LOWER(dim1), 'lookyloo') = 'xabc'"), @@ -118,7 +118,7 @@ public void testFilterLookupOfFunction() @Test public void testFilterFunctionOfLookup() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("LOWER(LOOKUP(dim1, 'lookyloo')) = 'xabc'"), @@ -686,7 +686,7 @@ public void testFilterMvContains() @Test public void testFilterMvContainsNull() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("MV_CONTAINS(LOOKUP(dim1, 'lookyloo'), NULL)"), @@ -700,7 +700,7 @@ public void testFilterMvContainsNull() @Test public void testFilterMvContainsNullInjective() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("MV_CONTAINS(LOOKUP(dim1, 'lookyloo121'), NULL)"), @@ -724,7 +724,7 @@ public void testFilterMvOverlap() @Test public void testFilterMvOverlapNull() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("MV_OVERLAP(lookup(dim1, 'lookyloo'), ARRAY['xabc', 'x6', 'nonexistent', NULL])"), @@ -755,7 +755,7 @@ public void testFilterMvOverlapNullInjective() @Test public void testFilterNotMvContains() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("NOT MV_CONTAINS(lookup(dim1, 'lookyloo'), 'xabc')"), @@ -797,7 +797,7 @@ public void testFilterNotMvContainsInjective() @Test public void testFilterNotMvOverlap() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("NOT MV_OVERLAP(lookup(dim1, 'lookyloo'), ARRAY['xabc', 'x6', 'nonexistent'])"), @@ -1119,7 +1119,7 @@ public void testFilterInCoalesceSameLiteralInjective() @Test public void testFilterMvContainsCoalesceSameLiteral() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("MV_CONTAINS(COALESCE(LOOKUP(dim1, 'lookyloo'), 'x6'), 'x6')"), @@ -1134,7 +1134,7 @@ public void testFilterMvContainsCoalesceSameLiteral() @Test public void testFilterMvOverlapCoalesceSameLiteral() { - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( buildFilterTestSql("MV_OVERLAP(COALESCE(LOOKUP(dim1, 'lookyloo'), 'x6'), ARRAY['xabc', 'x6', 'nonexistent'])"), diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index f749ad0cc52c..73e247480e92 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -7504,11 +7504,9 @@ public void testMultipleExactCountDistinctWithGroupingUsingGroupingSets() @Test public void testApproxCountDistinct() { + cannotVectorizeUnlessFallback(); msqIncompatible(); - // Cannot vectorize due to multi-valued dim2. - cannotVectorize(); - testQuery( "SELECT\n" + " SUM(cnt),\n" @@ -8169,8 +8167,8 @@ public void testCountDistinctArithmetic() @Test public void testCountDistinctOfSubstring() { - // Cannot vectorize due to extraction dimension spec. - cannotVectorize(); + // Cannot vectorize due to substring function. + cannotVectorizeUnlessFallback(); testQuery( "SELECT COUNT(DISTINCT SUBSTRING(dim1, 1, 1)) FROM druid.foo WHERE dim1 <> ''", @@ -8315,7 +8313,7 @@ public void testSillyQuarters() public void testRegexpExtract() { // Cannot vectorize due to regexp_extract function. - cannotVectorize(); + cannotVectorizeUnlessFallback(); testQuery( "SELECT DISTINCT\n" diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index a04617b8513a..b8f66a8439ce 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -2246,27 +2246,6 @@ public void testReverse() ); } - @Test - public void testAbnormalReverseWithWrongType() - { - Throwable t = Assert.assertThrows( - DruidException.class, - () -> testHelper.testExpression( - new ReverseOperatorConversion().calciteOperator(), - testHelper.makeInputRef("a"), - DruidExpression.ofExpression( - ColumnType.STRING, - DruidExpression.functionCall("reverse"), - ImmutableList.of( - DruidExpression.ofColumn(ColumnType.LONG, "a") - ) - ), - null - ) - ); - Assert.assertEquals("Function[reverse] needs a STRING argument but got LONG instead", t.getMessage()); - } - @Test public void testRight() {