dzamo commented on a change in pull request #2264:
URL: https://github.com/apache/drill/pull/2264#discussion_r659149206
##########
File path: exec/java-exec/src/test/resources/functions/testBitTwiddlers.json
##########
@@ -0,0 +1,34 @@
+{
Review comment:
Thanks for the review @paul-rogers, I'll certain allow more time for
reviews to arrive, in future. The naming of these functions put me into a bit
of a spin, to the point where I couldn't formulate very good questions about
naming when I asked on the Slack channel.
1. I started out trying for names of `AND`, `OR` and `XOR` but the first two
seem to be taken elsewhere Drill and would not work, while the last is fine and
indeed was already synonymous with `^` before this patch (but only working for
32-bit arguments).
2. Investigating `AND` and `OR`, I found `booleanAnd` and `booleanOr` in
FunctionNames.java and then also logic in other code using these names to
decide if something is a boolean expression, which bitwise logical operators
are not. I decided at that point that I should not overload function names
that were intended to be boolean, and abandoned `AND` and `OR`.
3. I considered `BIT_AND`, `BIT_OR` and `BIT_XOR` but already these belong
to the aggregate bitwise functions and do not allow overloading for the scalar
case (I got runtime errors when I tried).
4. I considered `INT_AND`, `INT_OR` and `INT_XOR` and indeed these did work
in testing.
5. Somehow I uncovered that `&`, `|`, `^` are _already_ working scalar
bitwise functions in Drill, in spite of my being unable to find any
implementation of them in the code. Looking at physical query plans did not
give me any clues and I told myself that some "magic" is happening upstream,
possibly code gen by Calcite?
6. Since Drill already has the names in (5) for bitwise functions, even if
they're made of special characters and nasty, I stuck with them because it's
consistent with what's already there.
I'd be grateful for any suggestion of where to take this in a follow-up
commit...
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
