You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jchen5 (via GitHub)" <gi...@apache.org> on 2023/07/26 03:35:31 UTC
[GitHub] [spark] jchen5 opened a new pull request, #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
jchen5 opened a new pull request, #42163:
URL: https://github.com/apache/spark/pull/42163
### What changes were proposed in this pull request?
`null IN (empty list)` incorrectly evaluates to null, when it should evaluate to false. (The reason it should be false is because a IN (b1, b2) is defined as a = b1 OR a = b2, and an empty IN list is treated as an empty OR which is false. This is specified by ANSI SQL.)
Many places in Spark execution (In, InSet, InSubquery) and optimization (OptimizeIn, NullPropagation) implemented this wrong behavior. This is a longstanding correctness issue which has existed since null support for IN expressions was first added to Spark.
This PR fixes Spark execution (In, InSet, InSubquery). See previous PR https://github.com/apache/spark/pull/42007 for optimization fixes.
The behavior is under a flag, which will be available to revert to the legacy behavior if needed. This flag is set to disable the new behavior until all of the fix PRs are complete.
See [this doc](https://docs.google.com/document/d/1k8AY8oyT-GI04SnP7eXttPDnDj-Ek-c3luF2zL6DPNU/edit) for more information.
### Why are the changes needed?
Fix wrong SQL semantics
### Does this PR introduce _any_ user-facing change?
Not yet, but will fix wrong SQL semantics when enabled
### How was this patch tested?
Unit tests and sql tests added in previous PR https://github.com/apache/spark/pull/42007
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jchen5 commented on pull request #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
Posted by "jchen5 (via GitHub)" <gi...@apache.org>.
jchen5 commented on PR #42163:
URL: https://github.com/apache/spark/pull/42163#issuecomment-1650931952
@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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #42163:
URL: https://github.com/apache/spark/pull/42163#issuecomment-1670781492
thanks, merging to master/3.5!
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
URL: https://github.com/apache/spark/pull/42163
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jchen5 commented on a diff in pull request #42163: [SPARK-44551][SQL] Fix behavior of null IN (empty list) in expression execution
Posted by "jchen5 (via GitHub)" <gi...@apache.org>.
jchen5 commented on code in PR #42163:
URL: https://github.com/apache/spark/pull/42163#discussion_r1274337801
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala:
##########
@@ -481,88 +483,104 @@ case class In(value: Expression, list: Seq[Expression]) extends Predicate {
override def toString: String = s"$value IN ${list.mkString("(", ",", ")")}"
override def eval(input: InternalRow): Any = {
- val evaluatedValue = value.eval(input)
- if (evaluatedValue == null) {
- null
+ if (list.isEmpty && !legacyNullInEmptyBehavior) {
+ // IN (empty list) is always false under current behavior.
+ // Under legacy behavior it's null if the left side is null, otherwise false (SPARK-44550).
+ false
} else {
- var hasNull = false
- list.foreach { e =>
- val v = e.eval(input)
- if (v == null) {
- hasNull = true
- } else if (ordering.equiv(v, evaluatedValue)) {
- return true
- }
- }
- if (hasNull) {
+ val evaluatedValue = value.eval(input)
Review Comment:
Note: most of these changes are just whitespace, use hide whitespace mode for easier 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org