fix bug with aggregator expressions on realtime index with string columns always producing 0 values (#11185)
* fix bug with aggregator expressions on realtime index with string columns always producing 0 values
* more test
* rework some stuff
* javadocs
diff --git a/core/src/main/java/org/apache/druid/math/expr/ExprEval.java b/core/src/main/java/org/apache/druid/math/expr/ExprEval.java
index c72be2e..38bb045 100644
--- a/core/src/main/java/org/apache/druid/math/expr/ExprEval.java
+++ b/core/src/main/java/org/apache/druid/math/expr/ExprEval.java
@@ -627,7 +627,18 @@
}
/**
- * returns true if numeric primitive value for this ExprEval is null, otherwise false.
+ * The method returns true if numeric primitive value for this {@link ExprEval} is null, otherwise false.
+ *
+ * If this method returns false, then the values returned by {@link #asLong()}, {@link #asDouble()},
+ * and {@link #asInt()} are "valid", since this method is primarily used during {@link Expr} evaluation to decide
+ * if primitive numbers can be fetched to use.
+ *
+ * If a type cannot sanely convert into a primitive numeric value, then this method should always return true so that
+ * these primitive numeric getters are not called, since returning false is assumed to mean these values are valid.
+ *
+ * Note that all types must still return values for {@link #asInt()}, {@link #asLong()}}, and {@link #asDouble()},
+ * since this can still happen if {@link NullHandling#sqlCompatible()} is false, but it should be assumed that this
+ * can only happen in that mode and 0s are typical and expected for values that would otherwise be null.
*/
public abstract boolean isNumericNull();
@@ -636,10 +647,25 @@
return false;
}
+ /**
+ * Get the primtive integer value. Callers should check {@link #isNumericNull()} prior to calling this method,
+ * otherwise it may improperly return placeholder a value (typically zero, which is expected if
+ * {@link NullHandling#sqlCompatible()} is false)
+ */
public abstract int asInt();
+ /**
+ * Get the primtive long value. Callers should check {@link #isNumericNull()} prior to calling this method,
+ * otherwise it may improperly return a placeholder value (typically zero, which is expected if
+ * {@link NullHandling#sqlCompatible()} is false)
+ */
public abstract long asLong();
+ /**
+ * Get the primtive double value. Callers should check {@link #isNumericNull()} prior to calling this method,
+ * otherwise it may improperly return a placeholder value (typically zero, which is expected if
+ * {@link NullHandling#sqlCompatible()} is false)
+ */
public abstract double asDouble();
public abstract boolean asBoolean();
@@ -1042,6 +1068,12 @@
if (!isStringValueCached()) {
if (value == null) {
cacheStringValue(null);
+ } else if (value.length == 1) {
+ if (value[0] == null) {
+ cacheStringValue(null);
+ } else {
+ cacheStringValue(String.valueOf(value[0]));
+ }
} else {
cacheStringValue(Arrays.toString(value));
}
@@ -1053,7 +1085,11 @@
@Override
public boolean isNumericNull()
{
- return false;
+ if (isScalar()) {
+ return getScalarValue() == null;
+ }
+
+ return true;
}
@Override
@@ -1098,6 +1134,18 @@
{
return value == null ? null : value[index];
}
+
+ protected boolean isScalar()
+ {
+ return value != null && value.length == 1;
+ }
+
+ @Nullable
+ protected T getScalarValue()
+ {
+ assert value != null && value.length == 1;
+ return value[0];
+ }
}
private static class LongArrayExprEval extends ArrayExprEval<Long>
@@ -1115,6 +1163,62 @@
return ExprType.LONG_ARRAY;
}
+ @Override
+ public int asInt()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.intValue();
+ }
+ return super.asInt();
+ }
+
+ @Override
+ public long asLong()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.longValue();
+ }
+ return super.asLong();
+ }
+
+ @Override
+ public double asDouble()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.doubleValue();
+ }
+ return super.asDouble();
+ }
+
+ @Override
+ public boolean asBoolean()
+ {
+ if (isScalar()) {
+ Number scalarValue = getScalarValue();
+ if (scalarValue == null) {
+ assert NullHandling.replaceWithDefault();
+ return false;
+ }
+ return Evals.asBoolean(scalarValue.longValue());
+ }
+ return super.asBoolean();
+ }
+
@Nullable
@Override
public String[] asStringArray()
@@ -1143,6 +1247,21 @@
return StringExprEval.OF_NULL;
}
switch (castTo) {
+ case STRING:
+ if (value.length == 1) {
+ return ExprEval.of(asString());
+ }
+ break;
+ case LONG:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofLong(null) : ExprEval.ofLong(asLong());
+ }
+ break;
+ case DOUBLE:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofDouble(null) : ExprEval.ofDouble(asDouble());
+ }
+ break;
case LONG_ARRAY:
return this;
case DOUBLE_ARRAY:
@@ -1176,6 +1295,62 @@
return ExprType.DOUBLE_ARRAY;
}
+ @Override
+ public int asInt()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.intValue();
+ }
+ return super.asInt();
+ }
+
+ @Override
+ public long asLong()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.longValue();
+ }
+ return super.asLong();
+ }
+
+ @Override
+ public double asDouble()
+ {
+ if (isScalar()) {
+ Number scalar = getScalarValue();
+ if (scalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return scalar.doubleValue();
+ }
+ return super.asDouble();
+ }
+
+ @Override
+ public boolean asBoolean()
+ {
+ if (isScalar()) {
+ Number scalarValue = getScalarValue();
+ if (scalarValue == null) {
+ assert NullHandling.replaceWithDefault();
+ return false;
+ }
+ return Evals.asBoolean(scalarValue.longValue());
+ }
+ return super.asBoolean();
+ }
+
@Nullable
@Override
public String[] asStringArray()
@@ -1206,6 +1381,21 @@
return StringExprEval.OF_NULL;
}
switch (castTo) {
+ case STRING:
+ if (value.length == 1) {
+ return ExprEval.of(asString());
+ }
+ break;
+ case LONG:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofLong(null) : ExprEval.ofLong(asLong());
+ }
+ break;
+ case DOUBLE:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofDouble(null) : ExprEval.ofDouble(asDouble());
+ }
+ break;
case LONG_ARRAY:
return ExprEval.ofLongArray(asLongArray());
case DOUBLE_ARRAY:
@@ -1230,8 +1420,13 @@
private boolean longValueValid = false;
private boolean doubleValueValid = false;
+ @Nullable
private Long[] longValues;
+ @Nullable
private Double[] doubleValues;
+ @Nullable
+ private Number computedNumericScalar;
+ private boolean isScalarNumberValid;
private StringArrayExprEval(@Nullable String[] value)
{
@@ -1244,6 +1439,67 @@
return ExprType.STRING_ARRAY;
}
+ @Override
+ public boolean isNumericNull()
+ {
+ if (isScalar()) {
+ computeScalarNumericIfNeeded();
+ return computedNumericScalar == null;
+ }
+ return true;
+ }
+
+ @Override
+ public int asInt()
+ {
+ if (isScalar()) {
+ computeScalarNumericIfNeeded();
+ if (computedNumericScalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0;
+ }
+ return computedNumericScalar.intValue();
+ }
+ return super.asInt();
+ }
+
+ @Override
+ public long asLong()
+ {
+ if (isScalar()) {
+ computeScalarNumericIfNeeded();
+ if (computedNumericScalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0L;
+ }
+ return computedNumericScalar.longValue();
+ }
+ return super.asLong();
+ }
+
+ @Override
+ public double asDouble()
+ {
+ if (isScalar()) {
+ computeScalarNumericIfNeeded();
+ if (computedNumericScalar == null) {
+ assert NullHandling.replaceWithDefault();
+ return 0.0;
+ }
+ return computedNumericScalar.doubleValue();
+ }
+ return super.asDouble();
+ }
+
+ @Override
+ public boolean asBoolean()
+ {
+ if (isScalar()) {
+ return Evals.asBoolean(getScalarValue());
+ }
+ return super.asBoolean();
+ }
+
@Nullable
@Override
public String[] asStringArray()
@@ -1280,6 +1536,21 @@
return StringExprEval.OF_NULL;
}
switch (castTo) {
+ case STRING:
+ if (value.length == 1) {
+ return ExprEval.of(asString());
+ }
+ break;
+ case LONG:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofLong(null) : ExprEval.ofLong(asLong());
+ }
+ break;
+ case DOUBLE:
+ if (value.length == 1) {
+ return isNumericNull() ? ExprEval.ofDouble(null) : ExprEval.ofDouble(asDouble());
+ }
+ break;
case STRING_ARRAY:
return this;
case LONG_ARRAY:
@@ -1330,5 +1601,17 @@
return Doubles.tryParse(val);
}).toArray(Double[]::new);
}
+
+
+ /**
+ * must not be called unless array has a single element
+ */
+ private void computeScalarNumericIfNeeded()
+ {
+ if (!isScalarNumberValid) {
+ computedNumericScalar = computeNumber(getScalarValue());
+ isScalarNumberValid = true;
+ }
+ }
}
}
diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index 7b9fe527..711acad 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -1437,9 +1437,11 @@
public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
{
ExprEval value1 = args.get(0).eval(bindings);
+
if (NullHandling.sqlCompatible() && value1.isNumericNull()) {
return ExprEval.of(null);
}
+
if (value1.type() != ExprType.LONG && value1.type() != ExprType.DOUBLE) {
throw new IAE(
"The first argument to the function[%s] should be integer or double type but got the type: %s",
diff --git a/core/src/test/java/org/apache/druid/math/expr/EvalTest.java b/core/src/test/java/org/apache/druid/math/expr/EvalTest.java
index a80edba..82624b9 100644
--- a/core/src/test/java/org/apache/druid/math/expr/EvalTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/EvalTest.java
@@ -21,14 +21,20 @@
import com.google.common.collect.ImmutableMap;
import org.apache.druid.common.config.NullHandling;
+import org.apache.druid.java.util.common.IAE;
import org.apache.druid.testing.InitializedNullHandlingTest;
import org.junit.Assert;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
/**
*/
public class EvalTest extends InitializedNullHandlingTest
{
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
private long evalLong(String x, Expr.ObjectBinding bindings)
{
ExprEval ret = eval(x, bindings);
@@ -145,6 +151,198 @@
}
@Test
+ public void testArrayToScalar()
+ {
+ Assert.assertEquals(1L, ExprEval.ofLongArray(new Long[]{1L}).asLong());
+ Assert.assertEquals(1.0, ExprEval.ofLongArray(new Long[]{1L}).asDouble(), 0.0);
+ Assert.assertEquals(1, ExprEval.ofLongArray(new Long[]{1L}).asInt());
+ Assert.assertEquals(true, ExprEval.ofLongArray(new Long[]{1L}).asBoolean());
+ Assert.assertEquals("1", ExprEval.ofLongArray(new Long[]{1L}).asString());
+
+
+ Assert.assertEquals(null, ExprEval.ofLongArray(new Long[]{null}).asString());
+
+ Assert.assertEquals(0L, ExprEval.ofLongArray(new Long[]{1L, 2L}).asLong());
+ Assert.assertEquals(0.0, ExprEval.ofLongArray(new Long[]{1L, 2L}).asDouble(), 0.0);
+ Assert.assertEquals("[1, 2]", ExprEval.ofLongArray(new Long[]{1L, 2L}).asString());
+ Assert.assertEquals(0, ExprEval.ofLongArray(new Long[]{1L, 2L}).asInt());
+ Assert.assertEquals(false, ExprEval.ofLongArray(new Long[]{1L, 2L}).asBoolean());
+
+ Assert.assertEquals(1.1, ExprEval.ofDoubleArray(new Double[]{1.1}).asDouble(), 0.0);
+ Assert.assertEquals(1L, ExprEval.ofDoubleArray(new Double[]{1.1}).asLong());
+ Assert.assertEquals("1.1", ExprEval.ofDoubleArray(new Double[]{1.1}).asString());
+ Assert.assertEquals(1, ExprEval.ofDoubleArray(new Double[]{1.1}).asInt());
+ Assert.assertEquals(true, ExprEval.ofDoubleArray(new Double[]{1.1}).asBoolean());
+
+ Assert.assertEquals(null, ExprEval.ofDoubleArray(new Double[]{null}).asString());
+
+ Assert.assertEquals(0.0, ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).asDouble(), 0.0);
+ Assert.assertEquals(0L, ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).asLong());
+ Assert.assertEquals("[1.1, 2.2]", ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).asString());
+ Assert.assertEquals(0, ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).asInt());
+ Assert.assertEquals(false, ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).asBoolean());
+
+ Assert.assertEquals("foo", ExprEval.ofStringArray(new String[]{"foo"}).asString());
+
+ Assert.assertEquals("1", ExprEval.ofStringArray(new String[]{"1"}).asString());
+ Assert.assertEquals(1L, ExprEval.ofStringArray(new String[]{"1"}).asLong());
+ Assert.assertEquals(1.0, ExprEval.ofStringArray(new String[]{"1"}).asDouble(), 0.0);
+ Assert.assertEquals(1, ExprEval.ofStringArray(new String[]{"1"}).asInt());
+ Assert.assertEquals(false, ExprEval.ofStringArray(new String[]{"1"}).asBoolean());
+ Assert.assertEquals(true, ExprEval.ofStringArray(new String[]{"true"}).asBoolean());
+
+ Assert.assertEquals("[1, 2.2]", ExprEval.ofStringArray(new String[]{"1", "2.2"}).asString());
+ Assert.assertEquals(0L, ExprEval.ofStringArray(new String[]{"1", "2.2"}).asLong());
+ Assert.assertEquals(0.0, ExprEval.ofStringArray(new String[]{"1", "2.2"}).asDouble(), 0.0);
+ Assert.assertEquals(0, ExprEval.ofStringArray(new String[]{"1", "2.2"}).asInt());
+ Assert.assertEquals(false, ExprEval.ofStringArray(new String[]{"1", "2.2"}).asBoolean());
+
+ // test casting arrays to scalars
+ Assert.assertEquals(1L, ExprEval.ofLongArray(new Long[]{1L}).castTo(ExprType.LONG).value());
+ Assert.assertEquals(NullHandling.defaultLongValue(), ExprEval.ofLongArray(new Long[]{null}).castTo(ExprType.LONG).value());
+ Assert.assertEquals(1.0, ExprEval.ofLongArray(new Long[]{1L}).castTo(ExprType.DOUBLE).asDouble(), 0.0);
+ Assert.assertEquals("1", ExprEval.ofLongArray(new Long[]{1L}).castTo(ExprType.STRING).value());
+
+ Assert.assertEquals(1.1, ExprEval.ofDoubleArray(new Double[]{1.1}).castTo(ExprType.DOUBLE).asDouble(), 0.0);
+ Assert.assertEquals(NullHandling.defaultDoubleValue(), ExprEval.ofDoubleArray(new Double[]{null}).castTo(ExprType.DOUBLE).value());
+ Assert.assertEquals(1L, ExprEval.ofDoubleArray(new Double[]{1.1}).castTo(ExprType.LONG).value());
+ Assert.assertEquals("1.1", ExprEval.ofDoubleArray(new Double[]{1.1}).castTo(ExprType.STRING).value());
+
+ Assert.assertEquals("foo", ExprEval.ofStringArray(new String[]{"foo"}).castTo(ExprType.STRING).value());
+ Assert.assertEquals(NullHandling.defaultLongValue(), ExprEval.ofStringArray(new String[]{"foo"}).castTo(ExprType.LONG).value());
+ Assert.assertEquals(NullHandling.defaultDoubleValue(), ExprEval.ofStringArray(new String[]{"foo"}).castTo(ExprType.DOUBLE).value());
+ Assert.assertEquals("1", ExprEval.ofStringArray(new String[]{"1"}).castTo(ExprType.STRING).value());
+ Assert.assertEquals(1L, ExprEval.ofStringArray(new String[]{"1"}).castTo(ExprType.LONG).value());
+ Assert.assertEquals(1.0, ExprEval.ofStringArray(new String[]{"1"}).castTo(ExprType.DOUBLE).value());
+ }
+
+ @Test
+ public void testStringArrayToScalarStringBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type STRING");
+ ExprEval.ofStringArray(new String[]{"foo", "bar"}).castTo(ExprType.STRING);
+ }
+
+ @Test
+ public void testStringArrayToScalarLongBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type LONG");
+ ExprEval.ofStringArray(new String[]{"foo", "bar"}).castTo(ExprType.LONG);
+ }
+
+ @Test
+ public void testStringArrayToScalarDoubleBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type DOUBLE");
+ ExprEval.ofStringArray(new String[]{"foo", "bar"}).castTo(ExprType.DOUBLE);
+ }
+
+ @Test
+ public void testLongArrayToScalarStringBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type STRING");
+ ExprEval.ofLongArray(new Long[]{1L, 2L}).castTo(ExprType.STRING);
+ }
+
+ @Test
+ public void testLongArrayToScalarLongBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type LONG");
+ ExprEval.ofLongArray(new Long[]{1L, 2L}).castTo(ExprType.LONG);
+ }
+
+ @Test
+ public void testLongArrayToScalarDoubleBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type DOUBLE");
+ ExprEval.ofLongArray(new Long[]{1L, 2L}).castTo(ExprType.DOUBLE);
+ }
+
+ @Test
+ public void testDoubleArrayToScalarStringBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type STRING");
+ ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).castTo(ExprType.STRING);
+ }
+
+ @Test
+ public void testDoubleArrayToScalarLongBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type LONG");
+ ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).castTo(ExprType.LONG);
+ }
+
+ @Test
+ public void testDoubleArrayToScalarDoubleBadCast()
+ {
+ expectedException.expect(IAE.class);
+ expectedException.expectMessage("invalid type DOUBLE");
+ ExprEval.ofDoubleArray(new Double[]{1.1, 2.2}).castTo(ExprType.DOUBLE);
+ }
+
+ @Test
+ public void testIsNumericNull()
+ {
+ if (NullHandling.sqlCompatible()) {
+ Assert.assertFalse(ExprEval.ofLong(1L).isNumericNull());
+ Assert.assertTrue(ExprEval.ofLong(null).isNumericNull());
+
+ 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.assertFalse(ExprEval.ofLongArray(new Long[]{1L}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofLongArray(new Long[]{null, 2L, 3L}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofLongArray(new Long[]{null}).isNumericNull());
+
+ Assert.assertFalse(ExprEval.ofDoubleArray(new Double[]{1.1}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofDoubleArray(new Double[]{null, 1.1, 2.2}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofDoubleArray(new Double[]{null}).isNumericNull());
+
+ Assert.assertFalse(ExprEval.ofStringArray(new String[]{"1"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{null, "1", "2"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{"one"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{null}).isNumericNull());
+ } else {
+ Assert.assertFalse(ExprEval.ofLong(1L).isNumericNull());
+ Assert.assertFalse(ExprEval.ofLong(null).isNumericNull());
+
+ Assert.assertFalse(ExprEval.ofDouble(1.0).isNumericNull());
+ Assert.assertFalse(ExprEval.ofDouble(null).isNumericNull());
+
+ // strings are still null
+ Assert.assertTrue(ExprEval.of(null).isNumericNull());
+ Assert.assertTrue(ExprEval.of("one").isNumericNull());
+ Assert.assertFalse(ExprEval.of("1").isNumericNull());
+
+ // arrays can still have nulls
+ Assert.assertFalse(ExprEval.ofLongArray(new Long[]{1L}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofLongArray(new Long[]{null, 2L, 3L}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofLongArray(new Long[]{null}).isNumericNull());
+
+ Assert.assertFalse(ExprEval.ofDoubleArray(new Double[]{1.1}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofDoubleArray(new Double[]{null, 1.1, 2.2}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofDoubleArray(new Double[]{null}).isNumericNull());
+
+ Assert.assertFalse(ExprEval.ofStringArray(new String[]{"1"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{null, "1", "2"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{"one"}).isNumericNull());
+ Assert.assertTrue(ExprEval.ofStringArray(new String[]{null}).isNumericNull());
+ }
+ }
+
+ @Test
public void testBooleanReturn()
{
Expr.ObjectBinding bindings = InputBindings.withMap(
diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
index bc283d3..1557749 100644
--- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
@@ -430,11 +430,14 @@
}
@Test
- public void testRoundWithNullValue()
+ public void testRoundWithNullValueOrInvalid()
{
Set<Pair<String, String>> invalidArguments = ImmutableSet.of(
Pair.of("null", "STRING"),
- Pair.of("x", "STRING")
+ Pair.of("x", "STRING"),
+ Pair.of("b", "LONG_ARRAY"),
+ Pair.of("c", "DOUBLE_ARRAY"),
+ Pair.of("a", "STRING_ARRAY")
);
for (Pair<String, String> argAndType : invalidArguments) {
if (NullHandling.sqlCompatible()) {
@@ -446,8 +449,7 @@
}
catch (IllegalArgumentException e) {
Assert.assertEquals(
- String.format(
- Locale.ENGLISH,
+ StringUtils.format(
"The first argument to the function[round] should be integer or double type but got the type: %s",
argAndType.rhs
),
@@ -459,33 +461,6 @@
}
@Test
- public void testRoundWithInvalidFirstArgument()
- {
- Set<Pair<String, String>> invalidArguments = ImmutableSet.of(
- Pair.of("b", "LONG_ARRAY"),
- Pair.of("c", "DOUBLE_ARRAY"),
- Pair.of("a", "STRING_ARRAY")
-
- );
- for (Pair<String, String> argAndType : invalidArguments) {
- try {
- assertExpr(String.format(Locale.ENGLISH, "round(%s)", argAndType.lhs), null);
- Assert.fail("Did not throw IllegalArgumentException");
- }
- catch (IllegalArgumentException e) {
- Assert.assertEquals(
- String.format(
- Locale.ENGLISH,
- "The first argument to the function[round] should be integer or double type but got the type: %s",
- argAndType.rhs
- ),
- e.getMessage()
- );
- }
- }
- }
-
- @Test
public void testRoundWithInvalidSecondArgument()
{
Set<Pair<String, String>> invalidArguments = ImmutableSet.of(
@@ -502,8 +477,7 @@
}
catch (IllegalArgumentException e) {
Assert.assertEquals(
- String.format(
- Locale.ENGLISH,
+ StringUtils.format(
"The second argument to the function[round] should be integer type but got the type: %s",
argAndType.rhs
),
diff --git a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
index a861803..b20fe00 100644
--- a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
+++ b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
@@ -49,7 +49,6 @@
import org.apache.druid.java.util.common.guava.Sequences;
import org.apache.druid.java.util.common.io.Closer;
import org.apache.druid.js.JavaScriptConfig;
-import org.apache.druid.math.expr.ExprMacroTable;
import org.apache.druid.query.BySegmentResultValue;
import org.apache.druid.query.BySegmentResultValueClass;
import org.apache.druid.query.ChainedExecutionQueryRunner;
@@ -11183,29 +11182,36 @@
"v",
"qualityDouble * qualityLong",
ValueType.LONG,
- ExprMacroTable.nil()
+ TestExprMacroTable.INSTANCE
+ ),
+ new ExpressionVirtualColumn(
+ "two",
+ "2",
+ null,
+ TestExprMacroTable.INSTANCE
)
)
.setDimensions(
new DefaultDimensionSpec("v", "v", ValueType.LONG)
)
- .setAggregatorSpecs(QueryRunnerTestHelper.ROWS_COUNT)
+ .setAggregatorSpecs(QueryRunnerTestHelper.ROWS_COUNT, new LongSumAggregatorFactory("twosum", null, "1 + two", TestExprMacroTable.INSTANCE))
.setGranularity(QueryRunnerTestHelper.ALL_GRAN)
.setLimit(5)
.build();
List<ResultRow> expectedResults = Arrays.asList(
- makeRow(query, "2011-04-01", "v", 10000000L, "rows", 2L),
- makeRow(query, "2011-04-01", "v", 12100000L, "rows", 2L),
- makeRow(query, "2011-04-01", "v", 14400000L, "rows", 2L),
- makeRow(query, "2011-04-01", "v", 16900000L, "rows", 2L),
- makeRow(query, "2011-04-01", "v", 19600000L, "rows", 6L)
+ makeRow(query, "2011-04-01", "v", 10000000L, "rows", 2L, "twosum", 6L),
+ makeRow(query, "2011-04-01", "v", 12100000L, "rows", 2L, "twosum", 6L),
+ makeRow(query, "2011-04-01", "v", 14400000L, "rows", 2L, "twosum", 6L),
+ makeRow(query, "2011-04-01", "v", 16900000L, "rows", 2L, "twosum", 6L),
+ makeRow(query, "2011-04-01", "v", 19600000L, "rows", 6L, "twosum", 18L)
);
Iterable<ResultRow> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
TestHelper.assertExpectedObjects(expectedResults, results, "groupBy");
}
+
@Test
public void testGroupByWithExpressionAggregator()
{
diff --git a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
index 806d5d8..c1de43c 100644
--- a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
+++ b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
@@ -46,8 +46,10 @@
import org.apache.druid.query.aggregation.CountAggregatorFactory;
import org.apache.druid.query.aggregation.DoubleMaxAggregatorFactory;
import org.apache.druid.query.aggregation.DoubleMinAggregatorFactory;
+import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory;
import org.apache.druid.query.aggregation.ExpressionLambdaAggregatorFactory;
import org.apache.druid.query.aggregation.FilteredAggregatorFactory;
+import org.apache.druid.query.aggregation.FloatSumAggregatorFactory;
import org.apache.druid.query.aggregation.LongSumAggregatorFactory;
import org.apache.druid.query.aggregation.cardinality.CardinalityAggregatorFactory;
import org.apache.druid.query.aggregation.first.DoubleFirstAggregatorFactory;
@@ -2205,10 +2207,12 @@
Lists.newArrayList(
Iterables.concat(
aggregatorFactoryList,
- Collections.singletonList(new FilteredAggregatorFactory(
- new CountAggregatorFactory("filteredAgg"),
- new SelectorDimFilter(QueryRunnerTestHelper.MARKET_DIMENSION, "spot", null)
- ))
+ ImmutableList.of(
+ new FilteredAggregatorFactory(
+ new CountAggregatorFactory("filteredAgg"),
+ new SelectorDimFilter(QueryRunnerTestHelper.MARKET_DIMENSION, "spot", null)
+ )
+ )
)
)
)
@@ -2222,15 +2226,83 @@
new Result<>(
DateTimes.of("2011-04-01"),
new TimeseriesResultValue(
- ImmutableMap.of(
- "filteredAgg", 18L,
- "addRowsIndexConstant", 12486.361190795898d,
- "index", 12459.361190795898d,
- "uniques", 9.019833517963864d,
- "rows", 26L
+ ImmutableMap.<String, Object>builder()
+ .put("filteredAgg", 18L)
+ .put("addRowsIndexConstant", 12486.361190795898d)
+ .put("index", 12459.361190795898d)
+ .put("uniques", 9.019833517963864d)
+ .put("rows", 26L)
+ .build()
+ )
+ )
+ );
+
+ assertExpectedResults(expectedResults, actualResults);
+ }
+
+ @Test
+ public void testTimeSeriesWithFilteredAggAndExpressionFilteredAgg()
+ {
+ // can't vectorize if expression
+ cannotVectorize();
+ TimeseriesQuery query = Druids
+ .newTimeseriesQueryBuilder()
+ .dataSource(QueryRunnerTestHelper.DATA_SOURCE)
+ .granularity(QueryRunnerTestHelper.ALL_GRAN)
+ .intervals(QueryRunnerTestHelper.FIRST_TO_THIRD)
+ .aggregators(
+ Lists.newArrayList(
+ Iterables.concat(
+ aggregatorFactoryList,
+ ImmutableList.of(
+ new FilteredAggregatorFactory(
+ new CountAggregatorFactory("filteredAgg"),
+ new SelectorDimFilter(QueryRunnerTestHelper.MARKET_DIMENSION, "spot", null)
+ ),
+ new LongSumAggregatorFactory(
+ "altLongCount",
+ null,
+ "if (market == 'spot', 1, 0)",
+ TestExprMacroTable.INSTANCE
+ ),
+ new DoubleSumAggregatorFactory(
+ "altDoubleCount",
+ null,
+ "if (market == 'spot', 1, 0)",
+ TestExprMacroTable.INSTANCE
+ ),
+ new FloatSumAggregatorFactory(
+ "altFloatCount",
+ null,
+ "if (market == 'spot', 1, 0)",
+ TestExprMacroTable.INSTANCE
+ )
+ )
)
)
)
+ .postAggregators(QueryRunnerTestHelper.ADD_ROWS_INDEX_CONSTANT)
+ .descending(descending)
+ .context(makeContext())
+ .build();
+
+ Iterable<Result<TimeseriesResultValue>> actualResults = runner.run(QueryPlus.wrap(query)).toList();
+ List<Result<TimeseriesResultValue>> expectedResults = Collections.singletonList(
+ new Result<>(
+ DateTimes.of("2011-04-01"),
+ new TimeseriesResultValue(
+ ImmutableMap.<String, Object>builder()
+ .put("filteredAgg", 18L)
+ .put("addRowsIndexConstant", 12486.361190795898d)
+ .put("index", 12459.361190795898d)
+ .put("uniques", 9.019833517963864d)
+ .put("rows", 26L)
+ .put("altLongCount", 18L)
+ .put("altDoubleCount", 18.0)
+ .put("altFloatCount", 18.0f)
+ .build()
+ )
+ )
);
assertExpectedResults(expectedResults, actualResults);
