spark icon indicating copy to clipboard operation
spark copied to clipboard

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

Open yaooqinn opened this issue 1 year ago • 4 comments

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

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

yaooqinn avatar May 07 '24 13:05 yaooqinn

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

yaooqinn avatar May 08 '24 03:05 yaooqinn

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

dongjoon-hyun avatar May 08 '24 03:05 dongjoon-hyun

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

dongjoon-hyun avatar May 08 '24 16:05 dongjoon-hyun

Thank you @dongjoon-hyun for providing the logs

yaooqinn avatar May 09 '24 05:05 yaooqinn

cc @cloud-fan

yaooqinn avatar May 15 '24 02:05 yaooqinn

thanks, merged to master(4.0.0)

ulysses-you avatar May 24 '24 10:05 ulysses-you

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

cloud-fan avatar May 25 '24 16:05 cloud-fan

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.

yaooqinn avatar May 27 '24 04:05 yaooqinn