Skip to content

[SPARK-4670] [SQL] wrong symbol for bitwise not #3528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[SPARK-4670] [SQL] wrong symbol for bitwise not #3528

wants to merge 3 commits into from

Conversation

adrian-wang
Copy link
Contributor

We should use ~ instead of - for bitwise NOT.

@SparkQA
Copy link

SparkQA commented Dec 1, 2014

Test build #23975 has started for PR 3528 at commit f55fbae.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 1, 2014

Test build #23975 has finished for PR 3528 at commit f55fbae.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23975/
Test PASSed.

marmbrus
marmbrus reviewed Dec 1, 2014
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
case IntegerType => ~(evalE.asInstanceOf[Int])
case LongType => ~(evalE.asInstanceOf[Long])
case other => sys.error(s"Unsupported bitwise ~ operation on ${other}")
case ByteType => ~evalE.asInstanceOf[Byte]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is actually introducing a bug. Above we set def dataType = child.dataType but this is changing the type to an Int.

scala> ~1.toByte
res0: Int = -2

Can you fix and add some test cases to the expression evaluation suite to make sure this doesn't regress again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the correction, I'll follow up very soon.

@SparkQA
Copy link

SparkQA commented Dec 2, 2014

Test build #24022 has started for PR 3528 at commit 56efb79.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 2, 2014

Test build #24022 has finished for PR 3528 at commit 56efb79.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24022/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Dec 2, 2014

Test build #24029 has started for PR 3528 at commit affd4ad.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 2, 2014

Test build #24029 has finished for PR 3528 at commit affd4ad.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24029/
Test PASSed.

asfgit pushed a commit that referenced this pull request Dec 2, 2014
@adrian-wang @marmbrus
[SPARK-4670] [SQL] wrong symbol for bitwise not
adc5d6f 
We should use `~` instead of `-` for bitwise NOT.

Author: Daoyuan Wang <[email protected]>

Closes #3528 from adrian-wang/symbol and squashes the following commits:

affd4ad [Daoyuan Wang] fix code gen test case
56efb79 [Daoyuan Wang] ensure bitwise NOT over byte and short persist data type
f55fbae [Daoyuan Wang] wrong symbol for bitwise not

(cherry picked from commit 1f5ddf1)
Signed-off-by: Michael Armbrust <[email protected]>
@asfgit asfgit closed this in 1f5ddf1 Dec 2, 2014
@marmbrus
Copy link
Contributor

marmbrus commented Dec 2, 2014

Thanks for fixing this! Merged to master and 1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adrian-wang @SparkQA @AmplabJenkins @marmbrus