You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "herunkang2018 (via GitHub)" <gi...@apache.org> on 2023/07/15 02:54:58 UTC
[GitHub] [calcite] herunkang2018 opened a new pull request, #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
herunkang2018 opened a new pull request, #3319:
URL: https://github.com/apache/calcite/pull/3319
(no comment)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1662172527
@tanclary would you like to help review if you have time? Thanks very much.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290124839
##########
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:
The supported type is `NUMERIC` type.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290122207
##########
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:
Would you mean to reuse the test code by parameterized test?
By the way, currently `testGetBitFunc` only contains basic test, and `testBitGetFunc` contains the full test cases.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290150636
##########
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:
Good advice, I will update it soon.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1636646831
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3319)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list) [91.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290127212
##########
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:
I will update the doc to address this.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT functions (enabled in Spark library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1685741551
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3319)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list) [93.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan merged pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT functions (enabled in Spark library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan merged PR #3319:
URL: https://github.com/apache/calcite/pull/3319
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290144365
##########
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:
According to Spark's [PR](https://github.com/apache/spark/pull/31198/files) to support `BIT_GET` function, the return type should be `TINYINT`. I will fix this soon.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT functions (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1685711557
@zoudan Good catch. This is fixed.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1685508934
Sorry, one minor comment use `functions` instead of `function` in the commit message.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290107324
##########
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:
It would be more concise. I will fix it soon.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290838068
##########
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:
Yes, in that way we could test all cases for both of them.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1676285492
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3319)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list) [93.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1676282705
@zoudan Thanks for the review.
All comments resolved, please help to review if you have time.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1684987911
@zoudan The commits is squashed. Thanks for the valuable comments in the review.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1684990539
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3319)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list) [93.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1684944411
LGTM, would you mind rebase and squash your commits?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3319:
URL: https://github.com/apache/calcite/pull/3319#issuecomment-1636641044
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3319)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3319&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3319&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list) [90.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3319&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1292059006
##########
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:
I checked this is the expected behavior. This implementation is the same as Spark's and just returns the bit of the position, not specially considering the meaning of signed bit. I find that currently the related test case is missing in Spark, and I will add it in this PR.
Besides, the test cases for negative value are missing. I will add them to improve code coverage.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] herunkang2018 commented on a diff in pull request #3319: [CALCITE-5848] Add BIT_GET and GETBIT function (enabled in Spark library)
Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3319:
URL: https://github.com/apache/calcite/pull/3319#discussion_r1290149728
##########
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:
According to Spark's [PR](https://github.com/apache/spark/pull/31198/files) to implement BIT_GET function, currently we don't support decimal type. For consistency, I think we should not take other number types like Decimal into consideration.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org