spark-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wenc...@apache.org
Subject spark git commit: [SPARK-22591][SQL] GenerateOrdering shouldn't change CodegenContext.INPUT_ROW
Date Fri, 24 Nov 2017 10:47:04 GMT
Repository: spark
Updated Branches:
  refs/heads/master c1217565e -> 62a826f17


[SPARK-22591][SQL] GenerateOrdering shouldn't change CodegenContext.INPUT_ROW

## What changes were proposed in this pull request?

When I played with codegen in developing another PR, I found the value of `CodegenContext.INPUT_ROW`
is not reliable. Under wholestage codegen, it is assigned to null first and then suddenly
changed to `i`.

The reason is `GenerateOrdering` changes `CodegenContext.INPUT_ROW` but doesn't restore it
back.

## How was this patch tested?

Added test.

Author: Liang-Chi Hsieh <viirya@gmail.com>

Closes #19800 from viirya/SPARK-22591.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/62a826f1
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/62a826f1
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/62a826f1

Branch: refs/heads/master
Commit: 62a826f17c549ed93300bdce562db56bddd5d959
Parents: c121756
Author: Liang-Chi Hsieh <viirya@gmail.com>
Authored: Fri Nov 24 11:46:58 2017 +0100
Committer: Wenchen Fan <wenchen@databricks.com>
Committed: Fri Nov 24 11:46:58 2017 +0100

----------------------------------------------------------------------
 .../expressions/codegen/GenerateOrdering.scala      | 16 ++++++++++------
 .../sql/catalyst/expressions/OrderingSuite.scala    | 11 ++++++++++-
 2 files changed, 20 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/62a826f1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
index 1639d1b..4a45957 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
@@ -72,13 +72,15 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
    * Generates the code for ordering based on the given order.
    */
   def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
+    val oldInputRow = ctx.INPUT_ROW
+    val oldCurrentVars = ctx.currentVars
+    val inputRow = "i"
+    ctx.INPUT_ROW = inputRow
+    // to use INPUT_ROW we must make sure currentVars is null
+    ctx.currentVars = null
+
     val comparisons = ordering.map { order =>
-      val oldCurrentVars = ctx.currentVars
-      ctx.INPUT_ROW = "i"
-      // to use INPUT_ROW we must make sure currentVars is null
-      ctx.currentVars = null
       val eval = order.child.genCode(ctx)
-      ctx.currentVars = oldCurrentVars
       val asc = order.isAscending
       val isNullA = ctx.freshName("isNullA")
       val primitiveA = ctx.freshName("primitiveA")
@@ -147,10 +149,12 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
           """
         }.mkString
       })
+    ctx.currentVars = oldCurrentVars
+    ctx.INPUT_ROW = oldInputRow
     // make sure INPUT_ROW is declared even if splitExpressions
     // returns an inlined block
     s"""
-       |InternalRow ${ctx.INPUT_ROW} = null;
+       |InternalRow $inputRow = null;
        |$code
      """.stripMargin
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/62a826f1/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
index aa61ba2..d0604b8 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
@@ -24,7 +24,7 @@ import org.apache.spark.serializer.KryoSerializer
 import org.apache.spark.sql.{RandomDataGenerator, Row}
 import org.apache.spark.sql.catalyst.{CatalystTypeConverters, InternalRow}
 import org.apache.spark.sql.catalyst.dsl.expressions._
-import org.apache.spark.sql.catalyst.expressions.codegen.{GenerateOrdering, LazilyGeneratedOrdering}
+import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, GenerateOrdering,
LazilyGeneratedOrdering}
 import org.apache.spark.sql.types._
 
 class OrderingSuite extends SparkFunSuite with ExpressionEvalHelper {
@@ -156,4 +156,13 @@ class OrderingSuite extends SparkFunSuite with ExpressionEvalHelper {
       assert(genOrdering.compare(rowB1, rowB2) < 0)
     }
   }
+
+  test("SPARK-22591: GenerateOrdering shouldn't change ctx.INPUT_ROW") {
+    val ctx = new CodegenContext()
+    ctx.INPUT_ROW = null
+
+    val schema = new StructType().add("field", FloatType, nullable = true)
+    GenerateOrdering.genComparisons(ctx, schema)
+    assert(ctx.INPUT_ROW == null)
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org


Mime
View raw message