[SPARK-17251][SQL] Support `OuterReference` in projection list of IN correlated subqueries #16012

dongjoon-hyun · 2016-11-25T11:30:48Z

What changes were proposed in this pull request?

Currently, correlated subqueries do not allow OuterReference columns in projection lists of IN correlated subqueries. This PR aims to support that by making OuterReference as NamedExpression and extending pullOutCorrelatedPredicates into pullOutCorrelatedProjectionAndPredicates.

scala> sql("CREATE TEMPORARY VIEW t1 AS SELECT * FROM VALUES 1, 2 AS t1(a)")
scala> sql("CREATE TEMPORARY VIEW t2 AS SELECT * FROM VALUES 1 AS t2(b)")
scala> sql("SELECT a FROM t1 WHERE a IN (SELECT a FROM t2)").show
java.lang.ClassCastException: org.apache.spark.sql.catalyst.expressions.OuterReference cannot be cast to org.apache.spark.sql.catalyst.expressions.NamedExpression

How was this patch tested?

Pass the Jenkins test with new test cases.

…orrelated subquery

hvanhovell · 2016-11-25T13:06:32Z

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

  override def nullable: Boolean = e.nullable
  override def prettyName: String = "outer"
+
+  override def name: String = throw new UnsupportedOperationException


Implement as much of these as possible (delegate to the NamedExpression). It is a resolved expression after all.

hvanhovell · 2016-11-25T13:09:18Z

sql/core/src/test/resources/sql-tests/inputs/correlated.sql

@@ -0,0 +1,18 @@
+CREATE TEMPORARY VIEW t1 AS SELECT * FROM VALUES 1, 2 AS t1(a);


Naming: lets call this subqueries.sql. Correlation has a different meaning in different contexts

SparkQA · 2016-11-25T14:07:45Z

Test build #69163 has finished for PR 16012 at commit 58449b3.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class OuterReference(e: NamedExpression)

dongjoon-hyun · 2016-11-25T18:14:04Z

Thank you, @hvanhovell ! I'll fix like that!

hvanhovell · 2016-11-25T20:22:02Z

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

 */
-case class OuterReference(e: NamedExpression) extends LeafExpression with Unevaluable {
+case class OuterReference(e: NamedExpression)(
+  val exprId: ExprId = NamedExpression.newExprId)


Use the exprId of the NamedExpression.

Is it okay? I thought it works like 'Alias'. Anyway, no problem. I'll update like that.

hvanhovell · 2016-11-25T20:22:23Z

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

+  override def name: String = e.name
+  override def qualifier: Option[String] = e.qualifier
+  override def toAttribute: Attribute = e.toAttribute
+  override def newInstance(): NamedExpression = OuterReference(e)()


OuterReference(e.newInstance())()?

hvanhovell · 2016-11-25T20:43:22Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

                try {
                  outer.resolve(nameParts, resolver) match {
-                    case Some(outerAttr) => OuterReference(outerAttr)
+                    case Some(outerAttr) => OuterReference(outerAttr)()


I am not sure the analyzer change has the desired effect. This just remove the outer reference from the tree, and this won't work if we use the attribute anywhere in the tree. For example:

select * from tbl_a where id in (select x from (select tbl_b.id, tbl_a.id + 1 as x, tbl_a.id + tbl_b.id as y from tbl_b) where y > 0)

I think we need to break this down into two steps:

Do not support this for now and just fix the named expression. That would be my goal for 2.1.

Try to see if we can rewrite the tree in such a way that we can extract the value. That would be my goal for 2.2. I am not sure how well we can make this work. In the end I think we need a dedicated subquery operator.

cc @nsyca what do you think.

Hmm. Correct. I'll check that again.

BTW, What about the predicates? It felt the predicates are handled the same way.

I have not looked at the code changes closely but got a general idea of what the originally reported problem is. I second @hvanhovell to not support outer reference in a SELECT clause of a subquery in 2.1. Just fix the named expression first.

IN subquery might be okay as it reflects the inner join semantics more or less. NOT IN subquery is converted to a special case of an anti-join with extra logic for the null value.

select * from tbl_a where tbl_a.c1 not in (select tbl_a.c2 from tbl_b)

Does the LeftAnti with effectively no join predicate, i.e.,

(isnull(tbl_a.c1 = tbl_a.c2) || (tbl_a.c1 = tbl_a.c2))

work correctly today? And if it returns a correct result, is it by design, not by chance?

Another interesting case to consider:

select ... from t1 where t1.c1 in (select sum(t1.c2) from t2)

If we support correlated columns in SELECT clause, do we build the Aggregate on T2 or T1?

The LEFT ANTI join should produce the correct result. Unfortunately we push down the tbl_a.c1 = tbl_a.c2 expression into the tbl_a side of the plan. So we need to fix this. I have created https://issues.apache.org/jira/browse/SPARK-18597 to track this.

Thank you @hvanhovell .
BTW, may I rebase this PR and try to the second plan for 2.2 here?

Try to see if we can rewrite the tree in such a way that we can extract the value. That would be my goal for 2.2. I am not sure how well we can make this work. In the end I think we need a dedicated subquery operator.

SparkQA · 2016-11-25T22:13:39Z

Test build #69173 has finished for PR 16012 at commit 3de9419.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class OuterReference(e: NamedExpression)(

dongjoon-hyun · 2016-11-26T03:35:02Z

Thank you for review, @hvanhovell and @nsyca .
I agree with you. We need enough time for this.
So, the option one for 2.1 is spun off into #16015.

dongjoon-hyun · 2016-11-28T00:16:54Z

Hi, @hvanhovell and @nsyca .

I created a JIRA issue for Option 2 with simple and complex example (@hvanhovell 's).

https://issues.apache.org/jira/browse/SPARK-18603

Now, I'm closing this PR. Thank you for reviews, @hvanhovell and @nsyca .

[SPARK-17251][SQL] Support OuterReference in projection list of a c…

58449b3

…orrelated subquery

dongjoon-hyun changed the title ~~[SPARK-17251][SQL] Support OuterReference in projection list of a correlated subquery~~ [SPARK-17251][SQL] Support OuterReference in projection list of IN correlated subqueries Nov 25, 2016

hvanhovell reviewed Nov 25, 2016

View reviewed changes

Remove UnsupportedOperationException and rename testcase file.

3de9419

hvanhovell requested changes Nov 25, 2016

View reviewed changes

dongjoon-hyun closed this Nov 28, 2016

dongjoon-hyun deleted the SPARK-17251 branch November 28, 2016 00:17

		@@ -0,0 +1,18 @@
		CREATE TEMPORARY VIEW t1 AS SELECT * FROM VALUES 1, 2 AS t1(a);

[SPARK-17251][SQL] Support OuterReference in projection list of IN correlated subqueries #16012

[SPARK-17251][SQL] Support OuterReference in projection list of IN correlated subqueries #16012

Uh oh!

Conversation

dongjoon-hyun commented Nov 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 25, 2016

Uh oh!

dongjoon-hyun commented Nov 25, 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!

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 Nov 25, 2016

Uh oh!

dongjoon-hyun commented Nov 26, 2016

Uh oh!

dongjoon-hyun commented Nov 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPARK-17251][SQL] Support `OuterReference` in projection list of IN correlated subqueries #16012

[SPARK-17251][SQL] Support `OuterReference` in projection list of IN correlated subqueries #16012

dongjoon-hyun commented Nov 25, 2016 •

edited

Loading