suneet-s commented on a change in pull request #10084:
URL: https://github.com/apache/druid/pull/10084#discussion_r446482630
##########
File path: core/src/test/java/org/apache/druid/math/expr/ParserTest.java
##########
@@ -172,6 +173,43 @@ public void testMixed()
validateFlatten("min(1, max(3, 4))", "(min [1, (max [3, 4])])", "1");
}
+ @Test
+ public void testBitwiseOps()
+ {
+ validateFlatten("3 & 1", "(& 3 1)", "1");
+ validateFlatten("3 & 1", "(& 3 1)", "1");
Review comment:
nit: duplicate test, did you intend to test the inverse of the previous
line here?
```suggestion
validateFlatten("1 & 3", "(& 1 3)", "1");
```
##########
File path: core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
##########
@@ -468,7 +468,7 @@ public void testGreatest()
{
// Same types
assertExpr("greatest(y, 0)", 2L);
- assertExpr("greatest(34.0, z, 5.0, 767.0", 767.0);
+ assertExpr("greatest(34.0, z, 5.0, 767.0)", 767.0);
Review comment:
👀 I'm surprised this didn't fail with a parse exception before
nice catch!
##########
File path: docs/misc/math-expr.md
##########
@@ -35,13 +35,15 @@ This expression language supports the following operators
(listed in decreasing
|Operators|Description|
|---------|-----------|
-|!, -|Unary NOT and Minus|
+|!, -, ^|Unary NOT, Minus, and bitwise Negate|
Review comment:
```suggestion
|!, -, ~|Unary NOT, Minus, and bitwise Negate|
```
##########
File path: core/src/test/java/org/apache/druid/math/expr/ParserTest.java
##########
@@ -172,6 +173,43 @@ public void testMixed()
validateFlatten("min(1, max(3, 4))", "(min [1, (max [3, 4])])", "1");
}
+ @Test
+ public void testBitwiseOps()
+ {
+ validateFlatten("3 & 1", "(& 3 1)", "1");
+ validateFlatten("3 & 1", "(& 3 1)", "1");
+ validateFlatten("2 & 1", "(& 2 1)", "0");
+ validateFlatten("3 | 1", "(| 3 1)", "3");
+ validateFlatten("2 | 1", "(| 2 1)", "3");
+ validateFlatten("(~1) & 7", "(& ~1 7)", "6");
+
+ validateFlatten("'2' & '1'", "(& 2 1)", "0");
+ validateFlatten("'3' | '1'", "(| 3 1)", "3");
+ validateFlatten("(~'1') & 7", "(& ~1 7)", "6");
+
+ validateFlatten("'notanumber' & '1'", "(& notanumber 1)", null);
+ validateFlatten("'3' | 'notanumber'", "(| 3 notanumber)", null);
+ validateFlatten("~'notanumber'", "~notanumber", null);
+ validateFlatten("(~'notanumber') & '7'", "(& ~notanumber 7)", null);
+ validateFlatten("(~'notanumber') | '7'", "(| ~notanumber 7)", null);
Review comment:
could you add some tests with nulls for the bitwise operators
----------------------------------------------------------------
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]
