From 8c07d3cb7ded827917713c313a6cff555d858d50 Mon Sep 17 00:00:00 2001 From: Dilip Biswal Date: Thu, 30 Aug 2018 20:17:39 -0700 Subject: [PATCH 1/4] [SPARK-25307] ArraySort function may return an error in the code generation phase. --- .../sql/catalyst/expressions/collectionOperations.scala | 3 ++- .../catalyst/expressions/CollectionExpressionsSuite.scala | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index cf9796ef1948f..37f1603e0d9b2 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -1047,7 +1047,8 @@ trait ArraySortLike extends ExpectsInputTypes { s"int $c = ${ctx.genComp(elementType, s"(($jt) $o1)", s"(($jt) $o2)")};" } val nonNullPrimitiveAscendingSort = - if (CodeGenerator.isPrimitiveType(elementType) && !containsNull) { + if (CodeGenerator.isPrimitiveType(elementType) && elementType != BooleanType + && !containsNull) { val javaType = CodeGenerator.javaType(elementType) val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType) s""" diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index 7b345aabd19c8..30c763afae716 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -326,12 +326,18 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper val d2 = new Decimal().set(100) val a4 = Literal.create(Seq(d2, d1), ArrayType(DecimalType(10, 0))) val a5 = Literal.create(Seq(null, null), ArrayType(NullType)) + val a6 = Literal.create(Seq(true, false, true, false), ArrayType(BooleanType, false)) + val a7 = Literal.create(Seq(true, false, true, false), ArrayType(BooleanType)) + val a8 = Literal.create(Seq(true, false, true, null, false), ArrayType(BooleanType)) checkEvaluation(new SortArray(a0), Seq(1, 2, 3)) checkEvaluation(new SortArray(a1), Seq[Integer]()) checkEvaluation(new SortArray(a2), Seq("a", "b")) checkEvaluation(new SortArray(a3), Seq(null, "a", "b")) checkEvaluation(new SortArray(a4), Seq(d1, d2)) + checkEvaluation(new SortArray(a6), Seq(false, false, true, true)) + checkEvaluation(new SortArray(a7), Seq(false, false, true, true)) + checkEvaluation(new SortArray(a8), Seq(null, false, false, true, true)) checkEvaluation(SortArray(a0, Literal(true)), Seq(1, 2, 3)) checkEvaluation(SortArray(a1, Literal(true)), Seq[Integer]()) checkEvaluation(SortArray(a2, Literal(true)), Seq("a", "b")) From 1547ac77ac47c71880d7c6c1379f0639b7713f7f Mon Sep 17 00:00:00 2001 From: Dilip Biswal Date: Sun, 2 Sep 2018 23:13:28 -0700 Subject: [PATCH 2/4] Code review --- .../sql/catalyst/expressions/collectionOperations.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index 37f1603e0d9b2..a9ce7cb6073f0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -1021,6 +1021,9 @@ trait ArraySortLike extends ExpectsInputTypes { } def sortCodegen(ctx: CodegenContext, ev: ExprCode, base: String, order: String): String = { + def canPerformFastSort(): Boolean = { + CodeGenerator.isPrimitiveType(elementType) && elementType != BooleanType && !containsNull + } val arrayData = classOf[ArrayData].getName val genericArrayData = classOf[GenericArrayData].getName val unsafeArrayData = classOf[UnsafeArrayData].getName @@ -1046,9 +1049,7 @@ trait ArraySortLike extends ExpectsInputTypes { } else { s"int $c = ${ctx.genComp(elementType, s"(($jt) $o1)", s"(($jt) $o2)")};" } - val nonNullPrimitiveAscendingSort = - if (CodeGenerator.isPrimitiveType(elementType) && elementType != BooleanType - && !containsNull) { + val nonNullPrimitiveAscendingSort = if (canPerformFastSort()) { val javaType = CodeGenerator.javaType(elementType) val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType) s""" From 364543832b030414d69a6854965eb261891bf265 Mon Sep 17 00:00:00 2001 From: Dilip Biswal Date: Sun, 2 Sep 2018 23:41:29 -0700 Subject: [PATCH 3/4] fix --- .../sql/catalyst/expressions/collectionOperations.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index a9ce7cb6073f0..770d9695ee8bd 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -1021,9 +1021,6 @@ trait ArraySortLike extends ExpectsInputTypes { } def sortCodegen(ctx: CodegenContext, ev: ExprCode, base: String, order: String): String = { - def canPerformFastSort(): Boolean = { - CodeGenerator.isPrimitiveType(elementType) && elementType != BooleanType && !containsNull - } val arrayData = classOf[ArrayData].getName val genericArrayData = classOf[GenericArrayData].getName val unsafeArrayData = classOf[UnsafeArrayData].getName @@ -1049,7 +1046,9 @@ trait ArraySortLike extends ExpectsInputTypes { } else { s"int $c = ${ctx.genComp(elementType, s"(($jt) $o1)", s"(($jt) $o2)")};" } - val nonNullPrimitiveAscendingSort = if (canPerformFastSort()) { + val canPerformFastSort = + CodeGenerator.isPrimitiveType(elementType) && elementType != BooleanType && !containsNull + val nonNullPrimitiveAscendingSort = if (canPerformFastSort) { val javaType = CodeGenerator.javaType(elementType) val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType) s""" From d27256ec70868f3fc66901abec97b4ccd75977ad Mon Sep 17 00:00:00 2001 From: Dilip Biswal Date: Mon, 3 Sep 2018 10:34:17 -0700 Subject: [PATCH 4/4] minor --- .../sql/catalyst/expressions/CollectionExpressionsSuite.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index 30c763afae716..31e0a9b6a8ca7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -326,7 +326,8 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper val d2 = new Decimal().set(100) val a4 = Literal.create(Seq(d2, d1), ArrayType(DecimalType(10, 0))) val a5 = Literal.create(Seq(null, null), ArrayType(NullType)) - val a6 = Literal.create(Seq(true, false, true, false), ArrayType(BooleanType, false)) + val a6 = Literal.create(Seq(true, false, true, false), + ArrayType(BooleanType, containsNull = false)) val a7 = Literal.create(Seq(true, false, true, false), ArrayType(BooleanType)) val a8 = Literal.create(Seq(true, false, true, null, false), ArrayType(BooleanType))