diff --git a/common/src/main/java/io/druid/math/expr/Evals.java b/common/src/main/java/io/druid/math/expr/Evals.java index f61488ff0c7..702037309a9 100644 --- a/common/src/main/java/io/druid/math/expr/Evals.java +++ b/common/src/main/java/io/druid/math/expr/Evals.java @@ -19,6 +19,7 @@ package io.druid.math.expr; +import com.google.common.base.Strings; import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.logger.Logger; @@ -81,4 +82,34 @@ public class Evals return binary; // best effort.. keep it working } } + + public static long asLong(boolean x) + { + return x ? 1L : 0L; + } + + public static double asDouble(boolean x) + { + return x ? 1D : 0D; + } + + public static String asString(boolean x) + { + return String.valueOf(x); + } + + public static boolean asBoolean(long x) + { + return x > 0; + } + + public static boolean asBoolean(double x) + { + return x > 0; + } + + public static boolean asBoolean(String x) + { + return !Strings.isNullOrEmpty(x) && Boolean.valueOf(x); + } } diff --git a/common/src/main/java/io/druid/math/expr/Expr.java b/common/src/main/java/io/druid/math/expr/Expr.java index 5f904d94488..a59e8e47ac3 100644 --- a/common/src/main/java/io/druid/math/expr/Expr.java +++ b/common/src/main/java/io/druid/math/expr/Expr.java @@ -23,7 +23,6 @@ import com.google.common.math.LongMath; import io.druid.java.util.common.IAE; import java.util.List; -import java.util.Objects; /** */ @@ -242,13 +241,9 @@ class UnaryNotExpr extends UnaryExpr public ExprEval eval(ObjectBinding bindings) { ExprEval ret = expr.eval(bindings); - if (ret.type() == ExprType.LONG) { - return ExprEval.of(ret.asBoolean() ? 0L : 1L); - } - if (ret.type() == ExprType.DOUBLE) { - return ExprEval.of(ret.asBoolean() ? 0.0d :1.0d); - } - throw new IllegalArgumentException("unsupported type " + ret.type()); + // conforming to other boolean-returning binary operators + ExprType retType = ret.type() == ExprType.DOUBLE ? ExprType.DOUBLE : ExprType.LONG; + return ExprEval.of(!ret.asBoolean(), retType); } @Override @@ -458,19 +453,19 @@ class BinLtExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(left.compareTo(right) < 0 ? 1L : 0L); + return ExprEval.of(left.compareTo(right) < 0, ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left < right ? 1L : 0L; + return Evals.asLong(left < right); } @Override protected final double evalDouble(double left, double right) { - return left < right ? 1.0d : 0.0d; + return Evals.asDouble(left < right); } } @@ -484,19 +479,19 @@ class BinLeqExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(left.compareTo(right) <= 0 ? 1L : 0L); + return ExprEval.of(left.compareTo(right) <= 0, ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left <= right ? 1L : 0L; + return Evals.asLong(left <= right); } @Override protected final double evalDouble(double left, double right) { - return left <= right ? 1.0d : 0.0d; + return Evals.asDouble(left <= right); } } @@ -510,19 +505,19 @@ class BinGtExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(left.compareTo(right) > 0 ? 1L : 0L); + return ExprEval.of(left.compareTo(right) > 0, ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left > right ? 1L : 0L; + return Evals.asLong(left > right); } @Override protected final double evalDouble(double left, double right) { - return left > right ? 1.0d : 0.0d; + return Evals.asDouble(left > right); } } @@ -536,19 +531,19 @@ class BinGeqExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(left.compareTo(right) >= 0 ? 1L : 0L); + return ExprEval.of(left.compareTo(right) >= 0, ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left >= right ? 1L : 0L; + return Evals.asLong(left >= right); } @Override protected final double evalDouble(double left, double right) { - return left >= right ? 1.0d : 0.0d; + return Evals.asDouble(left >= right); } } @@ -562,19 +557,19 @@ class BinEqExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(left.equals(right) ? 1L : 0L); + return ExprEval.of(left.equals(right), ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left == right ? 1L : 0L; + return Evals.asLong(left == right); } @Override protected final double evalDouble(double left, double right) { - return left == right ? 1.0d : 0.0d; + return Evals.asDouble(left == right); } } @@ -588,19 +583,19 @@ class BinNeqExpr extends BinaryEvalOpExprBase @Override protected ExprEval evalString(String left, String right) { - return ExprEval.of(!Objects.equals(left, right) ? 1L : 0L); + return ExprEval.of(!left.equals(right), ExprType.LONG); } @Override protected final long evalLong(long left, long right) { - return left != right ? 1L : 0L; + return Evals.asLong(left != right); } @Override protected final double evalDouble(double left, double right) { - return left != right ? 1.0d : 0.0d; + return Evals.asDouble(left != right); } } diff --git a/common/src/main/java/io/druid/math/expr/ExprEval.java b/common/src/main/java/io/druid/math/expr/ExprEval.java index 9a766076785..d8962061f18 100644 --- a/common/src/main/java/io/druid/math/expr/ExprEval.java +++ b/common/src/main/java/io/druid/math/expr/ExprEval.java @@ -55,9 +55,9 @@ public abstract class ExprEval { switch (type) { case DOUBLE: - return ExprEval.of(value ? 1D : 0D); + return ExprEval.of(Evals.asDouble(value)); case LONG: - return ExprEval.of(value ? 1L : 0L); + return ExprEval.of(Evals.asLong(value)); case STRING: return ExprEval.of(String.valueOf(value)); default: @@ -162,7 +162,7 @@ public abstract class ExprEval @Override public final boolean asBoolean() { - return asDouble() > 0; + return Evals.asBoolean(asDouble()); } @Override @@ -202,7 +202,7 @@ public abstract class ExprEval @Override public final boolean asBoolean() { - return asLong() > 0; + return Evals.asBoolean(asLong()); } @Override @@ -266,7 +266,7 @@ public abstract class ExprEval @Override public final boolean asBoolean() { - return Boolean.valueOf(value); + return Evals.asBoolean(value); } @Override diff --git a/common/src/test/java/io/druid/math/expr/EvalTest.java b/common/src/test/java/io/druid/math/expr/EvalTest.java index fefbce0fb62..1d457c364ac 100644 --- a/common/src/test/java/io/druid/math/expr/EvalTest.java +++ b/common/src/test/java/io/druid/math/expr/EvalTest.java @@ -143,4 +143,35 @@ public class EvalTest Assert.assertEquals("NULL", eval("nvl(if(x == 9223372036854775807, '', 'x'), 'NULL')", bindings).asString()); Assert.assertEquals("x", eval("nvl(if(x == 9223372036854775806, '', 'x'), 'NULL')", bindings).asString()); } + + @Test + public void testBooleanReturn() + { + Expr.ObjectBinding bindings = Parser.withMap( + ImmutableMap.of("x", 100L, "y", 100L, "z", 100D, "w", 100D) + ); + ExprEval eval = Parser.parse("x==y").eval(bindings); + Assert.assertTrue(eval.asBoolean()); + Assert.assertEquals(ExprType.LONG, eval.type()); + + eval = Parser.parse("x!=y").eval(bindings); + Assert.assertFalse(eval.asBoolean()); + Assert.assertEquals(ExprType.LONG, eval.type()); + + eval = Parser.parse("x==z").eval(bindings); + Assert.assertTrue(eval.asBoolean()); + Assert.assertEquals(ExprType.DOUBLE, eval.type()); + + eval = Parser.parse("x!=z").eval(bindings); + Assert.assertFalse(eval.asBoolean()); + Assert.assertEquals(ExprType.DOUBLE, eval.type()); + + eval = Parser.parse("z==w").eval(bindings); + Assert.assertTrue(eval.asBoolean()); + Assert.assertEquals(ExprType.DOUBLE, eval.type()); + + eval = Parser.parse("z!=w").eval(bindings); + Assert.assertFalse(eval.asBoolean()); + Assert.assertEquals(ExprType.DOUBLE, eval.type()); + } }