You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2017/11/24 10:47:04 UTC
spark git commit: [SPARK-22591][SQL] GenerateOrdering shouldn't
change CodegenContext.INPUT_ROW
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 <vi...@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 <vi...@gmail.com>
Authored: Fri Nov 24 11:46:58 2017 +0100
Committer: Wenchen Fan <we...@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