tanelk commented on a change in pull request #31170:
URL: https://github.com/apache/spark/pull/31170#discussion_r556569592
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitwiseExpressions.scala
##########
@@ -204,3 +206,51 @@ case class BitwiseCount(child: Expression)
case LongType => java.lang.Long.bitCount(input.asInstanceOf[Long])
}
}
+
+object BitwiseGet {
+ def calculate(target: Int, pos: Int): Int = {
+ val source = target.toBinaryString
Review comment:
Casting to string seems like a major overhead. It would be better to
handle this with bitwise operations.
`(target >> pos) & 1`
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
##########
@@ -550,6 +550,8 @@ object FunctionRegistry {
expression[BitAndAgg]("bit_and"),
expression[BitOrAgg]("bit_or"),
expression[BitXorAgg]("bit_xor"),
+ expression[BitwiseGet]("bit_get"),
Review comment:
All the other databases seemed to have the `getbit` only. Perhaps it's
better to not pollute the function list with aliases.
##########
File path: sql/core/src/test/resources/sql-tests/inputs/bitwise.sql
##########
@@ -68,3 +68,9 @@ SELECT b1, bit_xor(b2) FROM bitwise_test GROUP BY b1 HAVING
bit_and(b2) < 7;
-- window
SELECT b1, b2, bit_xor(b2) OVER (PARTITION BY b1 ORDER BY b2) FROM
bitwise_test;
+
+-- getbit
Review comment:
Its odd, that the bitwise functions are almost only expressions that
have dedicated SQL test.
This suite does not add any more coverage compared to
`BitwiseExpressionsSuite`, it just increases the testing overhead.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitwiseExpressions.scala
##########
@@ -204,3 +206,51 @@ case class BitwiseCount(child: Expression)
case LongType => java.lang.Long.bitCount(input.asInstanceOf[Long])
}
}
+
+object BitwiseGet {
+ def calculate(target: Int, pos: Int): Int = {
+ val source = target.toBinaryString
+ if (pos < 0) {
+ throw new IllegalArgumentException(s"Invalid bit position: $pos less
than zero")
+ } else if (source.length <= pos) {
+ throw new IllegalArgumentException(s"Invalid bit position: $pos exceeds
the " +
+ "bit length of the target integer")
+ }
+ val subString =
UTF8String.fromString(source).substringSQL(pos.asInstanceOf[Int], 1)
+ val intWrapper = new UTF8String.IntWrapper()
+ subString.toInt(intWrapper)
+ intWrapper.value
+ }
+}
+
+@ExpressionDescription(
+ usage = "_FUNC_(expr, pos) - Returns the value of the bit (0 or 1) at the
specified position.",
+ examples = """
+ Examples:
+ > SELECT _FUNC_(11, 0);
+ 1
+ > SELECT _FUNC_(11, 2);
+ 0
+ """,
+ since = "3.2.0",
+ group = "bitwise_funcs")
+case class BitwiseGet(left: Expression, right: Expression)
+ extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant {
+
+ override def inputTypes: Seq[IntegerType] = Seq(IntegerType, IntegerType)
Review comment:
`IntegralType` seems better choice as all the other bitwise functions
use it.
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/BitwiseExpressionsSuite.scala
##########
@@ -131,4 +132,25 @@ class BitwiseExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
checkConsistencyBetweenInterpretedAndCodegen(BitwiseXor, dt, dt)
}
}
+
+ test("BitGet") {
Review comment:
The `checkConsistencyBetweenInterpretedAndCodegen` should also be used:
```
DataTypeTestUtils.integralType.foreach { dt =>
checkConsistencyBetweenInterpretedAndCodegen(BitwiseXor, dt, dt)
}
```
----------------------------------------------------------------
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]
