Skip to content

Commit

Permalink
[SPARK-7286][SQL] Deprecate !== in favour of =!=
Browse files Browse the repository at this point in the history
This PR replaces #9925 which had issues with CI. **Please see the original PR for any previous discussions.**

## What changes were proposed in this pull request?
Deprecate the SparkSQL column operator !== and use =!= as an alternative.
Fixes subtle issues related to operator precedence (basically, !== does not have the same priority as its logical negation, ===).

## How was this patch tested?
All currently existing tests.

Author: Jakob Odersky <jodersky@gmail.com>

Closes #11588 from jodersky/SPARK-7286.
  • Loading branch information
jodersky authored and rxin committed Mar 9, 2016
1 parent cc4ab37 commit 035d3ac
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ package object dsl {
def >= (other: Expression): Predicate = GreaterThanOrEqual(expr, other)
def === (other: Expression): Predicate = EqualTo(expr, other)
def <=> (other: Expression): Predicate = EqualNullSafe(expr, other)
def !== (other: Expression): Predicate = Not(EqualTo(expr, other))
def =!= (other: Expression): Predicate = Not(EqualTo(expr, other))

def in(list: Expression*): Expression = In(expr, list)

Expand Down
22 changes: 20 additions & 2 deletions sql/core/src/main/scala/org/apache/spark/sql/Column.scala
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,23 @@ class Column(protected[sql] val expr: Expression) extends Logging {
*/
def equalTo(other: Any): Column = this === other

/**
* Inequality test.
* {{{
* // Scala:
* df.select( df("colA") =!= df("colB") )
* df.select( !(df("colA") === df("colB")) )
*
* // Java:
* import static org.apache.spark.sql.functions.*;
* df.filter( col("colA").notEqual(col("colB")) );
* }}}
*
* @group expr_ops
* @since 2.0.0
*/
def =!= (other: Any): Column = withExpr{ Not(EqualTo(expr, lit(other).expr)) }

/**
* Inequality test.
* {{{
Expand All @@ -267,8 +284,9 @@ class Column(protected[sql] val expr: Expression) extends Logging {
*
* @group expr_ops
* @since 1.3.0
*/
def !== (other: Any): Column = withExpr{ Not(EqualTo(expr, lit(other).expr)) }
*/
@deprecated("!== does not have the same precedence as ===, use =!= instead", "2.0.0")
def !== (other: Any): Column = this =!= other

/**
* Inequality test.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
testData2.collect().toSeq.filter(r => r.getInt(0) == r.getInt(1)))
}

test("!==") {
test("=!=") {
val nullData = sqlContext.createDataFrame(sparkContext.parallelize(
Row(1, 1) ::
Row(1, 2) ::
Expand Down
4 changes: 2 additions & 2 deletions sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ class JoinSuite extends QueryTest with SharedSQLContext {
Row(null, null, 6, "F") :: Nil)

checkAnswer(
left.join(right, ($"left.N" === $"right.N") && ($"left.N" !== 3), "full"),
left.join(right, ($"left.N" === $"right.N") && ($"left.N" =!= 3), "full"),
Row(1, "A", null, null) ::
Row(2, "B", null, null) ::
Row(3, "C", null, null) ::
Expand All @@ -357,7 +357,7 @@ class JoinSuite extends QueryTest with SharedSQLContext {
Row(null, null, 6, "F") :: Nil)

checkAnswer(
left.join(right, ($"left.N" === $"right.N") && ($"right.N" !== 3), "full"),
left.join(right, ($"left.N" === $"right.N") && ($"right.N" =!= 3), "full"),
Row(1, "A", null, null) ::
Row(2, "B", null, null) ::
Row(3, "C", null, null) ::
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex

checkFilterPredicate('_1 === true, classOf[Eq[_]], true)
checkFilterPredicate('_1 <=> true, classOf[Eq[_]], true)
checkFilterPredicate('_1 !== true, classOf[NotEq[_]], false)
checkFilterPredicate('_1 =!= true, classOf[NotEq[_]], false)
}
}

Expand All @@ -131,7 +131,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex

checkFilterPredicate('_1 === 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 <=> 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 !== 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))
checkFilterPredicate('_1 =!= 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))

checkFilterPredicate('_1 < 2, classOf[Lt[_]], 1)
checkFilterPredicate('_1 > 3, classOf[Gt[_]], 4)
Expand All @@ -157,7 +157,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex

checkFilterPredicate('_1 === 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 <=> 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 !== 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))
checkFilterPredicate('_1 =!= 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))

checkFilterPredicate('_1 < 2, classOf[Lt[_]], 1)
checkFilterPredicate('_1 > 3, classOf[Gt[_]], 4)
Expand All @@ -183,7 +183,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex

checkFilterPredicate('_1 === 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 <=> 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 !== 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))
checkFilterPredicate('_1 =!= 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))

checkFilterPredicate('_1 < 2, classOf[Lt[_]], 1)
checkFilterPredicate('_1 > 3, classOf[Gt[_]], 4)
Expand All @@ -209,7 +209,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex

checkFilterPredicate('_1 === 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 <=> 1, classOf[Eq[_]], 1)
checkFilterPredicate('_1 !== 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))
checkFilterPredicate('_1 =!= 1, classOf[NotEq[_]], (2 to 4).map(Row.apply(_)))

checkFilterPredicate('_1 < 2, classOf[Lt[_]], 1)
checkFilterPredicate('_1 > 3, classOf[Gt[_]], 4)
Expand Down Expand Up @@ -238,7 +238,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex
checkFilterPredicate('_1 === "1", classOf[Eq[_]], "1")
checkFilterPredicate('_1 <=> "1", classOf[Eq[_]], "1")
checkFilterPredicate(
'_1 !== "1", classOf[NotEq[_]], (2 to 4).map(i => Row.apply(i.toString)))
'_1 =!= "1", classOf[NotEq[_]], (2 to 4).map(i => Row.apply(i.toString)))

checkFilterPredicate('_1 < "2", classOf[Lt[_]], "1")
checkFilterPredicate('_1 > "3", classOf[Gt[_]], "4")
Expand Down Expand Up @@ -272,7 +272,7 @@ class ParquetFilterSuite extends QueryTest with ParquetTest with SharedSQLContex
'_1.isNotNull, classOf[NotEq[_]], (1 to 4).map(i => Row.apply(i.b)).toSeq)

checkBinaryFilterPredicate(
'_1 !== 1.b, classOf[NotEq[_]], (2 to 4).map(i => Row.apply(i.b)).toSeq)
'_1 =!= 1.b, classOf[NotEq[_]], (2 to 4).map(i => Row.apply(i.b)).toSeq)

checkBinaryFilterPredicate('_1 < 2.b, classOf[Lt[_]], 1.b)
checkBinaryFilterPredicate('_1 > 3.b, classOf[Gt[_]], 4.b)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ExpressionSQLBuilderSuite extends SQLBuilderTest {
test("binary comparisons") {
checkSQL('a.int === 'b.int, "(`a` = `b`)")
checkSQL('a.int <=> 'b.int, "(`a` <=> `b`)")
checkSQL('a.int !== 'b.int, "(NOT (`a` = `b`))")
checkSQL('a.int =!= 'b.int, "(NOT (`a` = `b`))")

checkSQL('a.int < 'b.int, "(`a` < `b`)")
checkSQL('a.int <= 'b.int, "(`a` <= `b`)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class OrcFilterSuite extends QueryTest with OrcTest {
|expr = (not leaf-0)""".stripMargin.trim
)
checkFilterPredicate(
'_1 !== 1,
'_1 =!= 1,
"""leaf-0 = (EQUALS _1 1)
|expr = (not leaf-0)""".stripMargin.trim
)
Expand Down

0 comments on commit 035d3ac

Please sign in to comment.