[SPARK-15677][SQL] Query with scalar sub-query in the SELECT list throws UnsupportedOperationException #13418

ioana-delaney · 2016-05-31T18:59:49Z

What changes were proposed in this pull request?

Queries with scalar sub-query in the SELECT list run against a local, in-memory relation throw
UnsupportedOperationException exception.

Problem repro:

scala> Seq((1, 1), (2, 2)).toDF("c1", "c2").createOrReplaceTempView("t1")
scala> Seq((1, 1), (2, 2)).toDF("c1", "c2").createOrReplaceTempView("t2")
scala> sql("select (select min(c1) from t2) from t1").show()

java.lang.UnsupportedOperationException: Cannot evaluate expression: scalar-subquery#62 []
  at org.apache.spark.sql.catalyst.expressions.Unevaluable$class.eval(Expression.scala:215)
  at org.apache.spark.sql.catalyst.expressions.ScalarSubquery.eval(subquery.scala:62)
  at org.apache.spark.sql.catalyst.expressions.Alias.eval(namedExpressions.scala:142)
  at org.apache.spark.sql.catalyst.expressions.InterpretedProjection.apply(Projection.scala:45)
  at org.apache.spark.sql.catalyst.expressions.InterpretedProjection.apply(Projection.scala:29)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.immutable.List.foreach(List.scala:381)
  at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
  at scala.collection.immutable.List.map(List.scala:285)
  at org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation$$anonfun$apply$37.applyOrElse(Optimizer.scala:1473)

The problem is specific to local, in memory relations. It is caused by rule ConvertToLocalRelation, which attempts to push down
a scalar-subquery expression to the local tables.

The solution prevents the rule to apply if Project references scalar subqueries.

How was this patch tested?

Added regression tests to SubquerySuite.scala

…nsupportedOperationException.

gatorsmile · 2016-05-31T20:37:49Z

LGTM CC @hvanhovell @davies @cloud-fan

davies · 2016-05-31T22:15:31Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

  def apply(plan: LogicalPlan): LogicalPlan = plan transform {
-    case Project(projectList, LocalRelation(output, data)) =>
+    case p @ Project(projectList, LocalRelation(output, data))
+        if !p.expressions.exists(ScalarSubquery.hasScalarSubquery) =>


Is it more general to check unvaluable expressions?

@davies Thank you for the comment. I am looking into generalizing the condition.

@davies Sorry for the delay in replying. I am new to the Spark code. I've looked at Unevaluable expressions. My findings are that checking for Unevaluable expressions would be too general since a lot of expressions mix in this trait. For example, AttributeReference is one of them. If we explicitly check for Unevaluable expressions, a simple query of the form "select c1 from t1"
would be regressed. Let me know I misunderstood your requirement. Thanks.

I think AttributeReference is the only exception, it will be replaced to BoundReference when create an Projection, we could have a special case for that.

+1 to catch Unevaluable and special case AttributeReference

SparkQA · 2016-05-31T22:54:52Z

Test build #3033 has finished for PR 13418 at commit faded1d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2016-06-01T04:58:43Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala

  }
+
+  def hasScalarSubquery(e: Expression): Boolean = {
+    e.find {


e.find(_.isInstanceOf[ScalarSubquery]).isDefined

?

@rxin Thank you for the review. I aligned my code to the existing implementation. But I can replace the method call with your suggestion.

ioana-delaney · 2016-06-02T02:23:45Z

@gatorsmile @davies @rxin @cloud-fan I've incorporated the comments. Please advise. Thank you.

cloud-fan · 2016-06-02T05:10:33Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

  def apply(plan: LogicalPlan): LogicalPlan = plan transform {
-    case Project(projectList, LocalRelation(output, data)) =>
+    case p @ Project(projectList, LocalRelation(output, data))
+        if !p.expressions.exists(hasUnevaluableExpr) =>


p.expressions is just the projectList

ioana-delaney · 2016-06-02T18:34:47Z

@cloud-fan I replaced p.expressions with projectList. Thanks.

cloud-fan · 2016-06-02T18:37:50Z

sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala

      Array(Row("two"))
    )
+
+    checkAnswer(


I think it's better to create a new test case for it

cloud-fan · 2016-06-02T18:38:07Z

LGTM except one test comment.

ioana-delaney · 2016-06-02T19:47:40Z

@cloud-fan I moved the unit tests to a new test case. Thank you.

cloud-fan · 2016-06-02T20:06:55Z

sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala

    )
  }

+  test("SPARK-15677: Scalar sub-query in Select list against a DataFrame generated query") {


maybe we should mention that this bug only exists in local relation?

ioana-delaney · 2016-06-02T21:55:01Z

Thank you @cloud-fan. I mentioned the local relations in the test case description and move the test cases under withTempTable.

cloud-fan · 2016-06-02T21:56:25Z

LGTM pending jenkins

ioana-delaney · 2016-06-03T16:33:06Z

@cloud-fan Thank you.

hvanhovell · 2016-06-03T16:43:28Z

whoops triggered a build on this one... sorry about that

SparkQA · 2016-06-03T18:41:12Z

Test build #3045 has finished for PR 13418 at commit 77166bc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

…ows UnsupportedOperationException ## What changes were proposed in this pull request? Queries with scalar sub-query in the SELECT list run against a local, in-memory relation throw UnsupportedOperationException exception. Problem repro: ```SQL scala> Seq((1, 1), (2, 2)).toDF("c1", "c2").createOrReplaceTempView("t1") scala> Seq((1, 1), (2, 2)).toDF("c1", "c2").createOrReplaceTempView("t2") scala> sql("select (select min(c1) from t2) from t1").show() java.lang.UnsupportedOperationException: Cannot evaluate expression: scalar-subquery#62 [] at org.apache.spark.sql.catalyst.expressions.Unevaluable$class.eval(Expression.scala:215) at org.apache.spark.sql.catalyst.expressions.ScalarSubquery.eval(subquery.scala:62) at org.apache.spark.sql.catalyst.expressions.Alias.eval(namedExpressions.scala:142) at org.apache.spark.sql.catalyst.expressions.InterpretedProjection.apply(Projection.scala:45) at org.apache.spark.sql.catalyst.expressions.InterpretedProjection.apply(Projection.scala:29) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.immutable.List.foreach(List.scala:381) at scala.collection.TraversableLike$class.map(TraversableLike.scala:234) at scala.collection.immutable.List.map(List.scala:285) at org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation$$anonfun$apply$37.applyOrElse(Optimizer.scala:1473) ``` The problem is specific to local, in memory relations. It is caused by rule ConvertToLocalRelation, which attempts to push down a scalar-subquery expression to the local tables. The solution prevents the rule to apply if Project references scalar subqueries. ## How was this patch tested? Added regression tests to SubquerySuite.scala Author: Ioana Delaney <[email protected]> Closes #13418 from ioana-delaney/scalarSubV2. (cherry picked from commit 9e2eb13) Signed-off-by: Wenchen Fan <[email protected]>

cloud-fan · 2016-06-03T19:08:53Z

thanks, merging to master and 2.0!

ioana-delaney · 2016-06-03T20:39:59Z

@cloud-fan @gatorsmile @davies @rxin @hvanhovell Thank you all. This was my first PR!

[SPARK-15677] Query with scalar sub-query in the SELECT list throws U…

faded1d

…nsupportedOperationException.

davies reviewed May 31, 2016
View reviewed changes

rxin reviewed Jun 1, 2016
View reviewed changes

[SPARK-15677] Revised changes based on comments.

1123599

cloud-fan reviewed Jun 2, 2016
View reviewed changes

[SPARK-15677] Replace expressions by projectList.

c45a344

cloud-fan reviewed Jun 2, 2016
View reviewed changes

[SPARK-15677] Create a new test case.

3e6e211

cloud-fan reviewed Jun 2, 2016
View reviewed changes

[SPARK-15677] Add withTempTable to the test unit.

77166bc

asfgit closed this in 9e2eb13 Jun 3, 2016

JoshRosen mentioned this pull request Sep 19, 2016

[SPARK-17100] [SQL] fix Python udf in filter on top of outer join #15103

Closed

[SPARK-15677][SQL] Query with scalar sub-query in the SELECT list throws UnsupportedOperationException #13418

[SPARK-15677][SQL] Query with scalar sub-query in the SELECT list throws UnsupportedOperationException #13418

Uh oh!

Conversation

ioana-delaney commented May 31, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

gatorsmile commented May 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ioana-delaney commented Jun 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ioana-delaney commented Jun 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jun 2, 2016

Uh oh!

ioana-delaney commented Jun 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ioana-delaney commented Jun 2, 2016

Uh oh!

cloud-fan commented Jun 2, 2016

Uh oh!

ioana-delaney commented Jun 3, 2016

Uh oh!

hvanhovell commented Jun 3, 2016

Uh oh!

SparkQA commented Jun 3, 2016

Uh oh!

cloud-fan commented Jun 3, 2016

Uh oh!

ioana-delaney commented Jun 3, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants