Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
cloud-fan commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1616688868 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: if people use the new shift syntax, then the pretty string should match it. Otherwise, we should keep the previous behavior. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1616629781 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: One more question here, shall we revert or keep changes in auto-generated shift expressions? Plans that are represented with symbols appear to have better readability than lowercased function names -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1616626455 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: One more question here, shall we revert or keep changes in auto-generated shift expressions? Plans that are represented with symbols appear to have good readability. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1616548808 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: I will check 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
cloud-fan commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1616546647 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: @yaooqinn can we fix this? I think it's wrong to use `<<` in the pretty string if users call `shiftleft` -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2132599664 Hi @cloud-fan Thank you for the check. I have made a follow-up to fix the precedence of these operators, the new precedence is between '+/-' and '&' with a left-to-right logic. https://github.com/apache/spark/pull/46753 As for the documentation you mentioned, it's orthogonal as there is no existing doc for this topic yet. So, I created SPARK-48426 to follow. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
cloud-fan commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2131357541 Have we considered the operator precedence? Does it follow C? If yes let's add tests and document it. https://en.cppreference.com/w/c/language/operator_precedence -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
cloud-fan commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1613651414 ## connector/connect/common/src/test/resources/query-tests/explain-results/function_shiftleft.explain: ## @@ -1,2 +1,2 @@ -Project [shiftleft(cast(b#0 as int), 2) AS shiftleft(b, 2)#0] +Project [(cast(b#0 as int) << 2) AS (b << 2)#0] Review Comment: can we keep the pretty string unchanged? if people explicitly use `shiftleft` function, then the generated alias name should still be `shiftleft`. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
ulysses-you closed pull request #46440: [SPARK-48168][SQL] Add bitwise shifting operators support URL: https://github.com/apache/spark/pull/46440 -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
ulysses-you commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2129180382 thanks, merged to master(4.0.0) -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1612922269
##
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##
@@ -69,6 +69,35 @@ lexer grammar SqlBaseLexer;
public void markUnclosedComment() {
has_unclosed_bracketed_comment = true;
}
+
+ /**
+ * When greater than zero, it's in the middle of parsing ARRAY/MAP/STRUCT
type.
+ */
+ public int complex_type_level_counter = 0;
+
+ /**
+ * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT'.
+ */
+ public void incComplexTypeLevelCounter() {
+complex_type_level_counter++;
+ }
+
+ /**
+ * Decrease the counter by one when hits close tag '>' && the counter
greater than zero
+ * which means we are in the middle of complex type parsing. Otherwise, it's
a dangling
+ * GT token and we do nothing.
+ */
+ public void decComplexTypeLevelCounter() {
+if (complex_type_level_counter > 0) complex_type_level_counter++;
Review Comment:
Nice catch! Thank you！
new tests added in the last 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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
ulysses-you commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1612891246
##
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##
@@ -69,6 +69,35 @@ lexer grammar SqlBaseLexer;
public void markUnclosedComment() {
has_unclosed_bracketed_comment = true;
}
+
+ /**
+ * When greater than zero, it's in the middle of parsing ARRAY/MAP/STRUCT
type.
+ */
+ public int complex_type_level_counter = 0;
+
+ /**
+ * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT'.
+ */
+ public void incComplexTypeLevelCounter() {
+complex_type_level_counter++;
+ }
+
+ /**
+ * Decrease the counter by one when hits close tag '>' && the counter
greater than zero
+ * which means we are in the middle of complex type parsing. Otherwise, it's
a dangling
+ * GT token and we do nothing.
+ */
+ public void decComplexTypeLevelCounter() {
+if (complex_type_level_counter > 0) complex_type_level_counter++;
Review Comment:
According to the method comment , shall it be
`complex_type_level_counter--;` ? BTW, is there a test case for it, e.g. `array
+ shiftleft` ?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1601043173
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##
@@ -799,6 +799,9 @@ object FunctionRegistry {
expression[BitwiseNot]("~"),
expression[BitwiseOr]("|"),
expression[BitwiseXor]("^"),
+expression[ShiftLeft]("<<", true, Some("4.0.0")),
+expression[ShiftRight](">>", true, Some("4.0.0")),
+expression[ShiftRightUnsigned](">>>", true, Some("4.0.0")),
Review Comment:
https://www.sqlite.org/lang_expr.html
https://www.ibm.com/docs/en/i/7.3?topic=expressions-bitwise-left-right-shift-operators
More evidences.
BTW, `>>>` is not seen from any other systems but added for self-consistency
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1601026514
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##
@@ -799,6 +799,9 @@ object FunctionRegistry {
expression[BitwiseNot]("~"),
expression[BitwiseOr]("|"),
expression[BitwiseXor]("^"),
+expression[ShiftLeft]("<<", true, Some("4.0.0")),
+expression[ShiftRight](">>", true, Some("4.0.0")),
+expression[ShiftRightUnsigned](">>>", true, Some("4.0.0")),
Review Comment:
https://learn.microsoft.com/en-us/sql/t-sql/functions/left-shift-transact-sql?view=sql-server-ver16
https://www.postgresql.org/docs/9.4/functions-bitstring.html
https://dev.mysql.com/doc/refman/8.0/en/bit-functions.html
I remember attaching these examples in the pull request description, but
they disappeared.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1601026514
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##
@@ -799,6 +799,9 @@ object FunctionRegistry {
expression[BitwiseNot]("~"),
expression[BitwiseOr]("|"),
expression[BitwiseXor]("^"),
+expression[ShiftLeft]("<<", true, Some("4.0.0")),
+expression[ShiftRight](">>", true, Some("4.0.0")),
+expression[ShiftRightUnsigned](">>>", true, Some("4.0.0")),
Review Comment:
https://learn.microsoft.com/en-us/cpp/cpp/left-shift-and-right-shift-operators-input-and-output?view=msvc-170
https://www.postgresql.org/docs/9.4/functions-bitstring.html
https://dev.mysql.com/doc/refman/8.0/en/bit-functions.html
I remember attaching these examples in the pull request description, but
they disappeared.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
cloud-fan commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1601022197
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##
@@ -799,6 +799,9 @@ object FunctionRegistry {
expression[BitwiseNot]("~"),
expression[BitwiseOr]("|"),
expression[BitwiseXor]("^"),
+expression[ShiftLeft]("<<", true, Some("4.0.0")),
+expression[ShiftRight](">>", true, Some("4.0.0")),
+expression[ShiftRightUnsigned](">>>", true, Some("4.0.0")),
Review Comment:
do they exist in other databases?
cc @srielau
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2111459264 cc @cloud-fan -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1598026716 ## sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4: ## @@ -980,9 +981,16 @@ valueExpression | left=valueExpression operator=AMPERSAND right=valueExpression #arithmeticBinary | left=valueExpression operator=HAT right=valueExpression #arithmeticBinary | left=valueExpression operator=PIPE right=valueExpression #arithmeticBinary +| left=valueExpression shiftOperator right=valueExpression #shiftExpression | left=valueExpression comparisonOperator right=valueExpression #comparison ; +shiftOperator +: LT LT +| GT GT +| GT GT GT Review Comment: This rule has been refined and whitespace-discrete operators now result in error as expected -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1598018214 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala: ## @@ -1269,111 +1304,71 @@ case class Pow(left: Expression, right: Expression) * @param right number of bits to left shift. */ @ExpressionDescription( - usage = "_FUNC_(base, expr) - Bitwise left shift.", + usage = "base << exp - Bitwise left shift.", examples = """ Examples: - > SELECT _FUNC_(2, 1); + > SELECT shiftleft(2, 1); + 4 + > SELECT 2 << 1; 4 """, since = "1.5.0", Review Comment: Sorry for being late, I have been figuring out the syntax issues. I have made some clarifications in the note field. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1596326588 ## sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4: ## @@ -980,9 +981,16 @@ valueExpression | left=valueExpression operator=AMPERSAND right=valueExpression #arithmeticBinary | left=valueExpression operator=HAT right=valueExpression #arithmeticBinary | left=valueExpression operator=PIPE right=valueExpression #arithmeticBinary +| left=valueExpression shiftOperator right=valueExpression #shiftExpression | left=valueExpression comparisonOperator right=valueExpression #comparison ; +shiftOperator +: LT LT +| GT GT +| GT GT GT Review Comment: I haven't found a way to retrieve the whitespace back to fail such a case. Tokenize these operators works but it fails to parse nested map and struct types -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
dongjoon-hyun commented on code in PR #46440:
URL: https://github.com/apache/spark/pull/46440#discussion_r1595894126
##
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##
@@ -980,9 +981,16 @@ valueExpression
| left=valueExpression operator=AMPERSAND right=valueExpression
#arithmeticBinary
| left=valueExpression operator=HAT right=valueExpression
#arithmeticBinary
| left=valueExpression operator=PIPE right=valueExpression
#arithmeticBinary
+| left=valueExpression shiftOperator right=valueExpression
#shiftExpression
| left=valueExpression comparisonOperator right=valueExpression
#comparison
;
+shiftOperator
+: LT LT
+| GT GT
+| GT GT GT
Review Comment:
This syntax is wrong because we should not allow spaces between `LT` and
`GT`.
For example, `< <` is a syntax error in other DBMS.
```
postgres=# SELECT 2 < < 1;
ERROR: syntax error at or near "<"
LINE 1: SELECT 2 < < 1;
```
But, this PR allows the following.
```
scala> sql("SELECT 2 < < 1").show()
++
|(2 << 1)|
++
| 4|
++
```
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
dongjoon-hyun commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1595891960 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala: ## @@ -1269,111 +1304,71 @@ case class Pow(left: Expression, right: Expression) * @param right number of bits to left shift. */ @ExpressionDescription( - usage = "_FUNC_(base, expr) - Bitwise left shift.", + usage = "base << exp - Bitwise left shift.", examples = """ Examples: - > SELECT _FUNC_(2, 1); + > SELECT shiftleft(2, 1); + 4 + > SELECT 2 << 1; 4 """, since = "1.5.0", Review Comment: This `1.5.0` doesn't match with the new usage `base << exp`. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
dongjoon-hyun commented on code in PR #46440: URL: https://github.com/apache/spark/pull/46440#discussion_r1595887736 ## sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4: ## @@ -980,9 +981,16 @@ valueExpression | left=valueExpression operator=AMPERSAND right=valueExpression #arithmeticBinary | left=valueExpression operator=HAT right=valueExpression #arithmeticBinary | left=valueExpression operator=PIPE right=valueExpression #arithmeticBinary +| left=valueExpression shiftOperator right=valueExpression #shiftExpression Review Comment: nit. `#shiftExpression` is not aligned. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2101941240 Thank you @dongjoon-hyun for providing the logs -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
dongjoon-hyun commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2100995752 It seems that TPCDS golden files are affected still. ``` [info] *** 23 TESTS FAILED *** [error] Failed: Total 3499, Failed 23, Errors 0, Passed 3476, Ignored 4 [error] Failed tests: [error] org.apache.spark.sql.SQLQuerySuite [error] org.apache.spark.sql.TPCDSV1_4_PlanStabilitySuite [error] org.apache.spark.sql.TPCDSV2_7_PlanStabilityWithStatsSuite [error] org.apache.spark.sql.TPCDSV1_4_PlanStabilityWithStatsSuite [error] org.apache.spark.sql.TPCDSV2_7_PlanStabilitySuite [error] (sql / Test / test) sbt.TestsFailedException: Tests unsuccessful ``` -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
dongjoon-hyun commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2099685242 No problem, @yaooqinn . We have enough time for 4.0.0. :) -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn commented on PR #46440: URL: https://github.com/apache/spark/pull/46440#issuecomment-2099683026 Thank you @dongjoon-hyun Let me look into this issue, it might take a while -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[PR] [SPARK-48168][SQL] Add bitwise shifting operators support [spark]
yaooqinn opened a new pull request, #46440: URL: https://github.com/apache/spark/pull/46440 ### What changes were proposed in this pull request? This PR introduces three bitwise shifting operators as aliases for existing shifting functions. ### Why are the changes needed? The bit shifting functions named in alphabet form vary from one platform to anthor. Take our shiftleft as an example, - Hive, shiftleft (where we copied it from) - MsSQL Server LEFT_SHIFT - MySQL, N/A - PostgreSQL, N/A - Presto, bitwise_left_shift The [bit shifting operators](https://en.wikipedia.org/wiki/Bitwise_operations_in_C) share a much more common and consistent way for users to port their queries. For self-consistent with existing bit operators in Spark, `AND &`, `OR |`, `XOR ^` and `NOT ~`, we now add `<<`, `>>` and `>>>`. ### Does this PR introduce _any_ user-facing change? Yes, new operators added but not behavior change ### How was this patch tested? new tests ### Was this patch authored or co-authored using generative AI tooling? no -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
