zoudan commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1287871695


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -815,6 +815,39 @@ public static int bitLength(ByteString s) {
     return s.length() * 8;
   }
 
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(long value, int position) {
+    checkPosition(position, java.lang.Long.SIZE);
+    return (int) ((value >> position) & 1);
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(int value, int position) {
+    checkPosition(position, java.lang.Integer.SIZE);
+    return (value >> position) & 1;
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(short value, int position) {
+    checkPosition(position, java.lang.Short.SIZE);
+    return (value >> position) & 1;
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(byte value, int position) {
+    checkPosition(position, java.lang.Byte.SIZE);
+    return (value >> position) & 1;
+  }
+
+  private static void checkPosition(int position, int size) {
+    if (position < 0) {
+      throw RESOURCE.illegalNegativeBitGetPosition().ex();

Review Comment:
   It would be better to add `position` in the error message



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1842,4 +1842,17 @@ private static RelDataType 
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
           ReturnTypes.INTEGER_NULLABLE,
           OperandTypes.or(OperandTypes.CHARACTER, OperandTypes.BINARY),
           SqlFunctionCategory.NUMERIC);
+
+  /** The "BIT_GET(value, position)" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlBasicFunction BIT_GET =
+      SqlBasicFunction.create("BIT_GET",
+          ReturnTypes.INTEGER_NULLABLE,

Review Comment:
   Are you sure that the return type is `INTEGER_NULLABLE`? 



##########
site/_docs/reference.md:
##########
@@ -2728,6 +2729,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_DATETIME(string, timestamp)             | Formats *timestamp* 
according to the specified format *string*
 | b | FORMAT_TIME(string, time)                      | Formats *time* 
according to the specified format *string*
 | b | FORMAT_TIMESTAMP(string timestamp)             | Formats *timestamp* 
according to the specified format *string*
+| s | GETBIT(value, position)                        | Returns the bit (0 or 
1) value at the specified *position* of *value*. The positions are numbered 
from right to left, starting at zero. The *position* argument cannot be negative

Review Comment:
   Could be simplify to 'Equivalent to `BIT_GET(value, position)`' ?



##########
site/_docs/reference.md:
##########
@@ -2678,6 +2678,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | * | ATANH(numeric)                                 | Returns the inverse 
hyperbolic tangent of *numeric*
 | s | BIT_LENGTH(binary)                             | Returns the bit length 
of *binary*
 | s | BIT_LENGTH(string)                             | Returns the bit length 
of *string*
+| s | BIT_GET(value, position)                       | Returns the bit (0 or 
1) value at the specified *position* of *value*. The positions are numbered 
from right to left, starting at zero. The *position* argument cannot be negative

Review Comment:
   What types are supported for `value`?



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -3947,6 +3947,56 @@ static void checkRlikeFails(SqlOperatorFixture f) {
     f1.checkNull("BIT_LENGTH(cast(null as binary))");
   }
 
+  @Test void testBitGetFunc() {

Review Comment:
   Maybe we could reuse the test code for BIT_GET and GETBIT, as the only 
difference between them is the function name.



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -815,6 +815,39 @@ public static int bitLength(ByteString s) {
     return s.length() * 8;
   }
 
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(long value, int position) {

Review Comment:
   Shall we take other number types like Decimal into consideration?



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -815,6 +815,39 @@ public static int bitLength(ByteString s) {
     return s.length() * 8;
   }
 
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(long value, int position) {
+    checkPosition(position, java.lang.Long.SIZE);
+    return (int) ((value >> position) & 1);
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(int value, int position) {
+    checkPosition(position, java.lang.Integer.SIZE);
+    return (value >> position) & 1;
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(short value, int position) {
+    checkPosition(position, java.lang.Short.SIZE);
+    return (value >> position) & 1;
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(byte value, int position) {
+    checkPosition(position, java.lang.Byte.SIZE);
+    return (value >> position) & 1;
+  }
+
+  private static void checkPosition(int position, int size) {
+    if (position < 0) {
+      throw RESOURCE.illegalNegativeBitGetPosition().ex();
+    }
+    if (size <= position) {
+      throw RESOURCE.illegalBitGetPositionExceedsLimit().ex();

Review Comment:
   It would be better to add `size` and `position` in the error message



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -815,6 +815,39 @@ public static int bitLength(ByteString s) {
     return s.length() * 8;
   }
 
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(long value, int position) {
+    checkPosition(position, java.lang.Long.SIZE);
+    return (int) ((value >> position) & 1);
+  }
+
+  /** SQL BIT_GET(value, position) function. */
+  public static int bitGet(int value, int position) {
+    checkPosition(position, java.lang.Integer.SIZE);
+    return (value >> position) & 1;

Review Comment:
   Is this expected behavior when it is the signed bit?



-- 
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]

Reply via email to