[SPARK-48168][SQL] Add bitwise shifting operators support #46440

yaooqinn · 2024-05-07T13:10:30Z

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

For other systems that we can refer to:

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

Does this PR introduce any user-facing change?

Yes, new operators were added but no behavior change

How was this patch tested?

new tests

Was this patch authored or co-authored using generative AI tooling?

no

dongjoon-hyun

It seems that this causes SQL parser errors. Could you make the CI happy? :)

- SPARK-43333: Stable field names when converting Union type *** FAILED *** (266 milliseconds)
[info]   org.apache.spark.sql.catalyst.parser.ParseException: [PARSE_SYNTAX_ERROR] Syntax error at or near '<'. SQLSTATE: 42601 (line 1, pos 57)
[info] 
[info] == SQL ==
[info] field1 struct<member_array: array<float>, member_map: map<string, int>>
[info] ---------------------------------------------------------^^^

yaooqinn · 2024-05-08T03:43:30Z

Thank you @dongjoon-hyun
Let me look into this issue, it might take a while

dongjoon-hyun · 2024-05-08T03:46:50Z

No problem, @yaooqinn . We have enough time for 4.0.0. :)

…lass sun.util.calendar.ZoneInfo'

…sible: class sun.util.calendar.ZoneInfo'" This reverts commit 6b5a1ae.

dongjoon-hyun · 2024-05-08T16:45:58Z

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

yaooqinn · 2024-05-09T05:04:15Z

Thank you @dongjoon-hyun for providing the logs

dongjoon-hyun · 2024-05-09T19:33:54Z

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


nit. #shiftExpression is not aligned.

dongjoon-hyun · 2024-05-09T19:38:41Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala

-      > SELECT _FUNC_(2, 1);
+      > SELECT shiftleft(2, 1);
+       4
+      > SELECT 2 << 1;
       4
  """,
  since = "1.5.0",


This 1.5.0 doesn't match with the new usage base << exp.

Sorry for being late, I have been figuring out the syntax issues.

I have made some clarifications in the note field.

dongjoon-hyun

New SQL syntax looks incorrect to me. Could you revise more?

dongjoon-hyun · 2024-05-09T19:40:59Z

sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4

+shiftOperator
+    : LT LT
+    | GT GT
+    | GT GT GT


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

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 rule has been refined and whitespace-discrete operators now result in error as expected

yaooqinn · 2024-05-15T02:19:44Z

cc @cloud-fan

cloud-fan · 2024-05-15T06:37:03Z

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")),


do they exist in other databases?

cc @srielau

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.

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

ulysses-you · 2024-05-24T06:23:27Z

sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4

+   * GT token and we do nothing.
+   */
+  public void decComplexTypeLevelCounter() {
+    if (complex_type_level_counter > 0) complex_type_level_counter++;


According to the method comment , shall it be complex_type_level_counter--; ? BTW, is there a test case for it, e.g. array + shiftleft ?

Nice catch! Thank you！

new tests added in the last commits

ulysses-you

lgtm if tests pass

ulysses-you · 2024-05-24T10:20:33Z

thanks, merged to master(4.0.0)

cloud-fan · 2024-05-24T15:24:59Z

...tor/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]


can we keep the pretty string unchanged? if people explicitly use shiftleft function, then the generated alias name should still be shiftleft.

cloud-fan · 2024-05-25T16:40:42Z

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

yaooqinn · 2024-05-27T04:02:14Z

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. #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.

[SPARK-48168][SQL] Add bitwise shifting operators support

1cfdba7

github-actions bot added the SQL label May 7, 2024

dongjoon-hyun requested changes May 7, 2024

View reviewed changes

yaooqinn marked this pull request as draft May 8, 2024 03:43

yaooqinn added 5 commits May 8, 2024 12:08

[SPARK-48185][SQL] Fix 'symbolic reference class is not accessible: c…

6b5a1ae

…lass sun.util.calendar.ZoneInfo'

de-tokenize shift operators

53f34d0

Revert "[SPARK-48185][SQL] Fix 'symbolic reference class is not acces…

07ad804

…sible: class sun.util.calendar.ZoneInfo'" This reverts commit 6b5a1ae.

regenareate golden files

02c746a

regenareate golden files

04bfbc3

github-actions bot added the CONNECT label May 8, 2024

regenareate golden files for tpc ds

0d088b1

yaooqinn marked this pull request as ready for review May 9, 2024 05:03

dongjoon-hyun reviewed May 9, 2024

View reviewed changes

dongjoon-hyun requested changes May 9, 2024

View reviewed changes

yaooqinn added 5 commits May 13, 2024 13:01

conditional token

ae5a0e4

fix some tests/ add some comments / handle dangling right GT

3e99dc8

refactoring

8601154

add more comments

0f8de4a

nit

81d1c2d

cloud-fan reviewed May 15, 2024

View reviewed changes

ulysses-you reviewed May 24, 2024

View reviewed changes

yaooqinn added 3 commits May 24, 2024 14:29

Merge branch 'master' into SPARK-48168

5e888a1

fix

98c788f

add tests

42111e9

Merge branch 'master' into SPARK-48168

da7c99e

ulysses-you approved these changes May 24, 2024

View reviewed changes

ulysses-you closed this in 7ae939a May 24, 2024

cloud-fan reviewed May 24, 2024

View reviewed changes

yaooqinn deleted the SPARK-48168 branch May 27, 2024 04:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-48168][SQL] Add bitwise shifting operators support #46440

[SPARK-48168][SQL] Add bitwise shifting operators support #46440

yaooqinn commented May 7, 2024 •

edited

dongjoon-hyun left a comment

yaooqinn commented May 8, 2024

dongjoon-hyun commented May 8, 2024

dongjoon-hyun commented May 8, 2024

yaooqinn commented May 9, 2024

dongjoon-hyun May 9, 2024

dongjoon-hyun May 9, 2024

yaooqinn May 13, 2024

dongjoon-hyun left a comment •

edited

dongjoon-hyun May 9, 2024

yaooqinn May 10, 2024

yaooqinn May 13, 2024

yaooqinn commented May 15, 2024

cloud-fan May 15, 2024

yaooqinn May 15, 2024 •

edited

yaooqinn May 15, 2024

ulysses-you May 24, 2024

yaooqinn May 24, 2024

ulysses-you left a comment

ulysses-you commented May 24, 2024

cloud-fan May 24, 2024

cloud-fan commented May 25, 2024

yaooqinn commented May 27, 2024

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

[SPARK-48168][SQL] Add bitwise shifting operators support #46440

[SPARK-48168][SQL] Add bitwise shifting operators support #46440

Conversation

yaooqinn commented May 7, 2024 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

dongjoon-hyun left a comment

Choose a reason for hiding this comment

yaooqinn commented May 8, 2024

dongjoon-hyun commented May 8, 2024

dongjoon-hyun commented May 8, 2024

yaooqinn commented May 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaooqinn commented May 15, 2024

Choose a reason for hiding this comment

yaooqinn May 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulysses-you left a comment

Choose a reason for hiding this comment

ulysses-you commented May 24, 2024

Choose a reason for hiding this comment

cloud-fan commented May 25, 2024

yaooqinn commented May 27, 2024

yaooqinn commented May 7, 2024 •

edited

dongjoon-hyun left a comment •

edited

yaooqinn May 15, 2024 •

edited