gianm commented on a change in pull request #10605:
URL: https://github.com/apache/druid/pull/10605#discussion_r553775571
##########
File path: core/src/main/java/org/apache/druid/math/expr/Evals.java
##########
@@ -71,4 +71,14 @@ public static boolean asBoolean(@Nullable String x)
{
return !NullHandling.isNullOrEquivalent(x) && Boolean.parseBoolean(x);
}
+
+ public static long doubleToLongBits(double x)
+ {
+ return Double.doubleToLongBits(x);
Review comment:
Why add this instead of calling the thing in `Double`?
##########
File path: docs/misc/math-expr.md
##########
@@ -119,6 +119,13 @@ See javadoc of java.lang.Math for detailed explanation for
each function.
|acos|acos(x) would return the arc cosine of x|
|asin|asin(x) would return the arc sine of x|
|atan|atan(x) would return the arc tangent of x|
+|bitwiseAnd|bitwiseAnd(x,y) would return the result of x & y. Double values
will be converted to their bit representation|
Review comment:
Why convert doubles to their bit representations instead of casting them
to longs? Casting to long would, I think, make more sense since we can think of
it as an implicit cast of double-typed arguments to a function that only
accepts longs.
##########
File path: docs/misc/math-expr.md
##########
@@ -119,6 +119,13 @@ See javadoc of java.lang.Math for detailed explanation for
each function.
|acos|acos(x) would return the arc cosine of x|
|asin|asin(x) would return the arc sine of x|
|atan|atan(x) would return the arc tangent of x|
+|bitwiseAnd|bitwiseAnd(x,y) would return the result of x & y. Double values
will be converted to their bit representation|
+|bitwiseComplement|bitwiseComplement(x) would return the result of ~x. Double
values will be converted to their bit representation|
+|bitwiseConvertDouble|bitwiseConvertDouble(x) would convert the IEEE 754
floating-point "double" bits stored in a long into a double value if the input
is a long, or the copy bits of a double value into a long if the input is a
double.|
Review comment:
This function is kind of weird because it doesn't have a fixpoint. I'd
think that `bitwiseConvertDouble(bitwiseConvertDouble(x))` would be identical
to `bitwiseConvertDouble(x)`. The lack of fixpoint makes it hard to reason
about what the result of this function is going to be. Is there a specific
reason it's designed this way? If not, I'd suggest splitting into two functions
for each direction of the conversion.
##########
File path: core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
##########
@@ -519,6 +519,31 @@ public void testLeast()
assertExpr("least(1, null, 'A')", "1");
}
+ @Test
+ public void testBitwise()
+ {
+ assertExpr("bitwiseAnd(3, 1)", 1L);
+ assertExpr("bitwiseAnd(2, 1)", 0L);
+ assertExpr("bitwiseOr(3, 1)", 3L);
+ assertExpr("bitwiseOr(2, 1)", 3L);
+ assertExpr("bitwiseXor(3, 1)", 2L);
+ assertExpr("bitwiseXor(2, 1)", 3L);
+ assertExpr("bitwiseShiftLeft(2, 1)", 4L);
+ assertExpr("bitwiseShiftRight(2, 1)", 1L);
+ assertExpr("bitwiseAnd(bitwiseComplement(1), 7)", 6L);
+ assertExpr("bitwiseAnd('2', '1')", null);
+ assertExpr("bitwiseAnd(2, '1')", 0L);
+
+ assertExpr("bitwiseOr(2.345, 1)", 4612462889363109315L);
Review comment:
Please include (double, double) and (long, double) in addition to
(double, long) args.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
