From 5fe79f6a9db5adca587ee88f840c8529b1972797 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Mon, 8 Sep 2025 23:56:28 -0700 Subject: [PATCH 1/3] Additional expr type alignment. PR #16366 originally added fallback vectorization, a mechanism for making all expressions vectorizable. Later, #17098 fixed some issues that arose and #17248 disabled fallback vectorization in the out-of-box configuration. This patch fixes various remaining issues with inconsistent type handling between the vectorized and nonvectorized expr implementations. It does not yet re-enable fallback vectorization out of the box, due to remaining inconsistencies with conditional exprs like "case_searched", "case_simple", and "if". 1) Aligns the behavior of missing columns and literal nulls so they are always treated as null longs. This was already the case for vectorized identifiers, but non-vectorized identifiers and literal nulls were still represented as strings. 2) Replaces all occurrences of "ExprEval.of(null)" with either an explicit type, or a call to "ExprEval.ofMissing()". ofMissing is a new function for situations where an eval represents a null value of unknown type. It is equivalent to "ExprEval.ofLong(null)", but is a separate function for clarity at the call site. 3) Update "cast" to return the target type even for null values. 4) Update "greatest", "least", and "array" so they eval to types that match what is reported by "getOutputType". 5) Update "scalb" to coerce input strings as numbers, to better allow for type evolution and missing columns. 6) Update "reverse" to coerce inputs to strings, to better allow for type evolution and missing columns. --- .../theta/sql/ThetaPostAggMacros.java | 2 +- .../query/expressions/SleepExprMacro.java | 2 +- .../druid/testing/tools/SleepExprMacro.java | 2 +- .../apache/druid/math/expr/ApplyFunction.java | 8 +- .../druid/math/expr/BinaryEvalOpExprBase.java | 8 +- .../math/expr/BinaryMathOperatorExpr.java | 2 +- .../druid/math/expr/BuiltInExprMacros.java | 2 +- .../apache/druid/math/expr/ConstantExpr.java | 2 +- .../org/apache/druid/math/expr/ExprEval.java | 65 +++++-- .../druid/math/expr/ExprListenerImpl.java | 2 +- .../druid/math/expr/ExpressionProcessing.java | 10 +- .../math/expr/ExpressionProcessingConfig.java | 13 -- .../org/apache/druid/math/expr/Function.java | 182 ++++++++---------- .../druid/math/expr/UnaryOperatorExpr.java | 4 +- .../expr/vector/FallbackVectorProcessor.java | 8 +- .../IPv4AddressStringifyExprMacro.java | 10 +- .../query/expression/LookupExprMacro.java | 2 +- .../expression/NestedDataExpressions.java | 4 +- .../expression/RegexpExtractExprMacro.java | 4 +- .../expression/RegexpReplaceExprMacro.java | 10 +- .../expression/TimestampCeilExprMacro.java | 2 +- .../expression/TimestampExtractExprMacro.java | 4 +- .../expression/TimestampFloorExprMacro.java | 2 +- .../expression/TimestampFormatExprMacro.java | 4 +- .../expression/TimestampParseExprMacro.java | 4 +- .../expression/TimestampShiftExprMacro.java | 4 +- .../druid/query/expression/TrimExprMacro.java | 4 +- .../druid/math/expr/ConstantExprTest.java | 11 +- .../org/apache/druid/math/expr/EvalTest.java | 12 +- .../apache/druid/math/expr/ExprEvalTest.java | 6 +- .../apache/druid/math/expr/FunctionTest.java | 4 +- .../druid/math/expr/OutputTypeTest.java | 4 +- .../expr/VectorExprResultConsistencyTest.java | 48 +++-- .../IPv4AddressMatchExprMacroTest.java | 38 ++-- .../IPv4AddressParseExprMacroTest.java | 12 +- .../IPv4AddressStringifyExprMacroTest.java | 10 +- .../IPv6AddressMatchExprMacroTest.java | 14 +- .../query/expression/LookupExprMacroTest.java | 2 +- .../TimestampExtractExprMacroTest.java | 26 +-- .../expression/TimestampShiftMacroTest.java | 30 +-- .../segment/filter/EqualityFilterTests.java | 2 +- .../segment/filter/ExpressionFilterTest.java | 2 +- .../sql/calcite/CalciteJoinQueryTest.java | 11 +- .../CalciteLookupFunctionQueryTest.java | 18 +- .../druid/sql/calcite/CalciteQueryTest.java | 10 +- .../calcite/expression/ExpressionsTest.java | 21 -- 46 files changed, 313 insertions(+), 334 deletions(-) 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..7b2e1d26ae49 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,13 @@ 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); - } - - @VisibleForTesting - public static void initializeForFallback() - { - INSTANCE = new ExpressionProcessingConfig(null, null, null, true); + INSTANCE = new ExpressionProcessingConfig(null, true, null); } /** 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..1538e89898e7 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", precisionParam.type()); + } 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..e2d4fef6cae1 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 @@ -374,17 +374,29 @@ public void testLookup() @Test public void testArrayFns() { - try { - ExpressionProcessing.initializeForFallback(); - testExpression("array(s1, s2)", types); - testExpression("array(l1, l2)", types); - testExpression("array(d1, d2)", types); - testExpression("array(l1, d2)", types); - testExpression("array(s1, l2)", types); - } - finally { - ExpressionProcessing.initializeForTests(); - } + testExpression("array(s1, s2)", types); + testExpression("array(l1, l2)", types); + testExpression("array(d1, d2)", types); + testExpression("array(l1, d2)", types); + testExpression("array(s1, l2)", types); + } + + @Test + public void testReduceFns() + { + 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); } @Test @@ -532,11 +544,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 +652,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 +664,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 +676,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 +692,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() { From b63eaad1ed13a670ca929183cb97d6a4c7c43bd5 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Sep 2025 00:23:20 -0700 Subject: [PATCH 2/3] Restore fallback in testArrayFns. --- .../druid/math/expr/ExpressionProcessing.java | 6 ++++++ .../expr/VectorExprResultConsistencyTest.java | 16 +++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) 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 7b2e1d26ae49..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 @@ -54,6 +54,12 @@ public static void initializeForHomogenizeNullMultiValueStrings() INSTANCE = new ExpressionProcessingConfig(null, true, null); } + @VisibleForTesting + public static void initializeForFallback() + { + INSTANCE = new ExpressionProcessingConfig(null, null, true); + } + /** * All {@link ExprType#ARRAY} values will be converted to {@link ExpressionType#STRING} by their column selectors * (not within expression processing) to be treated as multi-value strings instead of native arrays. 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 e2d4fef6cae1..87dbe406ef60 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 @@ -374,11 +374,17 @@ public void testLookup() @Test public void testArrayFns() { - testExpression("array(s1, s2)", types); - testExpression("array(l1, l2)", types); - testExpression("array(d1, d2)", types); - testExpression("array(l1, d2)", types); - testExpression("array(s1, l2)", types); + try { + ExpressionProcessing.initializeForFallback(); + testExpression("array(s1, s2)", types); + testExpression("array(l1, l2)", types); + testExpression("array(d1, d2)", types); + testExpression("array(l1, d2)", types); + testExpression("array(s1, l2)", types); + } + finally { + ExpressionProcessing.initializeForTests(); + } } @Test From 97f9ac0de6dbf0bfaeab535b69e76cac6dd208fd Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Sep 2025 00:59:47 -0700 Subject: [PATCH 3/3] Fix issues. --- .../org/apache/druid/math/expr/Function.java | 2 +- .../expr/VectorExprResultConsistencyTest.java | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) 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 1538e89898e7..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 @@ -4596,7 +4596,7 @@ public ExprEval apply(List args, Expr.ObjectBinding bindings) 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", precisionParam.type()); + 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()); 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 87dbe406ef60..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 @@ -390,19 +390,25 @@ public void testArrayFns() @Test public void testReduceFns() { - 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); + 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