Skip to content

[SPARK-24313][SQL] Fix collection operations' interpreted evaluation for complex types #21361

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

Closed
mgaido91 wants to merge 8 commits into from
Closed

[SPARK-24313][SQL] Fix collection operations' interpreted evaluation for complex types #21361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35370cd
[SPARK-24313][SQL] Fix array_contains interpreted evaluation for comp…
mgaido91 May 18, 2018
8c9553f
add orderable check
mgaido91 May 18, 2018
b7d7249
fix array position
mgaido91 May 18, 2018
a819a97
fix element_at and GetValueMap
mgaido91 May 18, 2018
71916d9
address comments
mgaido91 May 18, 2018
c74ddd7
address comments
mgaido91 May 20, 2018
3f8624b
add end to end test
mgaido91 May 21, 2018
6315775
address comment
mgaido91 May 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...alyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,9 @@ case class ArrayContains(left: Expression, right: Expression)

override def dataType: DataType = BooleanType

@transient private lazy val ordering: Ordering[Any] =
TypeUtils.getInterpretedOrdering(right.dataType)
Copy link
Member

@viirya viirya May 18, 2018

Choose a reason for hiding this comment

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

Then in checkInputDataTypes we should check if there is Ordering for right.dataType. Otherwise for example MapType will throw a match error.

Copy link
Contributor Author

@mgaido91 mgaido91 May 18, 2018

Choose a reason for hiding this comment

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

yes, thanks


override def inputTypes: Seq[AbstractDataType] = right.dataType match {
case NullType => Seq.empty
case _ => left.dataType match {
Expand Down Expand Up @@ -533,7 +536,7 @@ case class ArrayContains(left: Expression, right: Expression)
arr.asInstanceOf[ArrayData].foreach(right.dataType, (i, v) =>
if (v == null) {
hasNull = true
} else if (v == value) {
} else if (ordering.equiv(v, value)) {
Copy link
Member

@viirya viirya May 18, 2018

Choose a reason for hiding this comment

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

Previously does this work for Map? No?

Copy link
Contributor

@cloud-fan cloud-fan May 18, 2018

Choose a reason for hiding this comment

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

MapType is not supported in comparison, even =

return true
}
)
Expand Down
26 changes: 26 additions & 0 deletions ...src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,32 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper

checkEvaluation(ArrayContains(a3, Literal("")), null)
checkEvaluation(ArrayContains(a3, Literal.create(null, StringType)), null)

// complex data types
val b0 = Literal.create(Seq[Array[Byte]](Array[Byte](5, 6), Array[Byte](1, 2)),
ArrayType(BinaryType))
val b1 = Literal.create(Seq[Array[Byte]](Array[Byte](2, 1), Array[Byte](4, 3)),
ArrayType(BinaryType))
val b2 = Literal.create(Seq[Array[Byte]](Array[Byte](2, 1), null),
ArrayType(BinaryType))
val b3 = Literal.create(Seq[Array[Byte]](null, Array[Byte](1, 2)),
ArrayType(BinaryType))
val be = Literal.create(Array[Byte](1, 2), BinaryType)
Copy link
Contributor

@cloud-fan cloud-fan May 19, 2018

Choose a reason for hiding this comment

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

ditto, binary type is not complex type

val nullBinary = Literal.create(null, BinaryType)

checkEvaluation(ArrayContains(b0, be), true)
checkEvaluation(ArrayContains(b1, be), false)
checkEvaluation(ArrayContains(b0, nullBinary), null)
checkEvaluation(ArrayContains(b2, be), null)
checkEvaluation(ArrayContains(b3, be), true)

val aa0 = Literal.create(Seq[Seq[Int]](Seq[Int](1, 2), Seq[Int](3, 4)),
ArrayType(ArrayType(IntegerType)))
val aa1 = Literal.create(Seq[Seq[Int]](Seq[Int](5, 6), Seq[Int](2, 1)),
ArrayType(ArrayType(IntegerType)))
val aae = Literal.create(Seq[Int](1, 2), ArrayType(IntegerType))
checkEvaluation(ArrayContains(aa0, aae), true)
checkEvaluation(ArrayContains(aa1, aae), false)
}

test("ArraysOverlap") {
Expand Down