You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by DonnyZone <gi...@git.apache.org> on 2017/09/10 09:31:37 UTC
[GitHub] spark pull request #19178: [SPARK-21966][SQL]ResolveMissingReference rule sh...
GitHub user DonnyZone opened a pull request:
https://github.com/apache/spark/pull/19178
[SPARK-21966][SQL]ResolveMissingReference rule should not ignore Union
## What changes were proposed in this pull request?
https://issues.apache.org/jira/browse/SPARK-21966
The problem can be reproduced by following example.
`val df1 = spark.createDataFrame(Seq((1, 1), (2, 1), (2, 2))).toDF("a", "b")
val df2 = spark.createDataFrame(Seq((1, 1), (1, 2), (2, 3))).toDF("a", "b")
val df3 = df1.cube("a").sum("b")
val df4 = df2.cube("a").sum("b")
val df5 = df3.union(df4).filter("grouping_id()=0").show()`
The `org.apache.spark.sql.AnalysisException: cannot resolve '`spark_grouping_id`' given input columns`
is thrown as the ResolveMissingReference rule ignore the Union operator. This PR fix the issue.
## How was this patch tested?
unit tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/DonnyZone/spark ResolveMissingReference
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19178.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19178
----
commit 29ae28598c69bb4cb26ad66c86e6a73054e5fbef
Author: donnyzone <we...@gmail.com>
Date: 2017-09-10T09:26:36Z
SPARK-21966
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19178: [SPARK-21966][SQL]ResolveMissingReference rule sh...
Posted by DonnyZone <gi...@git.apache.org>.
Github user DonnyZone commented on a diff in the pull request:
https://github.com/apache/spark/pull/19178#discussion_r137979887
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1115,6 +1115,8 @@ class Analyzer(
g.copy(join = true, child = addMissingAttr(g.child, missing))
case d: Distinct =>
throw new AnalysisException(s"Can't add $missingAttrs to $d")
+ case u: Union =>
+ u.withNewChildren(u.children.map(addMissingAttr(_, missingAttrs)))
--- End diff --
Yeah, I agree with you. Current implementation only checks UnaryNode.
It is necessary to take all node types into consideration.
Thanks for suggestion, I will work on a general solution.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19178: [SPARK-21966][SQL]ResolveMissingReference rule sh...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/19178#discussion_r137970319
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1115,6 +1115,8 @@ class Analyzer(
g.copy(join = true, child = addMissingAttr(g.child, missing))
case d: Distinct =>
throw new AnalysisException(s"Can't add $missingAttrs to $d")
+ case u: Union =>
+ u.withNewChildren(u.children.map(addMissingAttr(_, missingAttrs)))
--- End diff --
This is not the only issue in `Union` and I think binary operators have the same issue, e.g.,
```
scala> df3.join(df4).filter("grouping_id()=0").show()
org.apache.spark.sql.AnalysisException: cannot resolve '`spark_grouping_id`' given input columns: [a, sum(b), a, sum(b)];;
'Filter ('spark_grouping_id = 0)
+- Join Inner
:- Aggregate [a#27, spark_grouping_id#25], [a#27, sum(cast(b#6 as bigint)) AS sum(b)#24L]
: +- Expand [List(a#5, b#6, a#26, 0), List(a#5, b#6, null, 1)], [a#5, b#6, a#27, spark_grouping_id#25]
: +- Project [a#5, b#6, a#5 AS a#26]
: +- Project [_1#0 AS a#5, _2#1 AS b#6]
: +- LocalRelation [_1#0, _2#1]
+- Aggregate [a#38, spark_grouping_id#36], [a#38, sum(cast(b#16 as bigint)) AS sum(b)#35L]
+- Expand [List(a#15, b#16, a#37, 0), List(a#15, b#16, null, 1)], [a#15, b#16, a#38, spark_grouping_id#36]
+- Project [a#15, b#16, a#15 AS a#37]
+- Project [_1#10 AS a#15, _2#11 AS b#16]
+- LocalRelation [_1#10, _2#11]
```
So, we need more general solution for this case, I think.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19178
yea, thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19178
Aha, you mean this is an expected design and we won't change this logic in `ResolveMissingReference` now, right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by DonnyZone <gi...@git.apache.org>.
Github user DonnyZone commented on the issue:
https://github.com/apache/spark/pull/19178
Thanks, I will make a try in our private repository as there are several such cases and the users want to in a seamless way. It is really complicated for a general support.
Should I close this PR now?
@gatorsmile @maropu
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19178
yea, also could you close the jira ticket as `Won't fix` (or, `Later`) and the reason why there. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19178
Yes. We do not plan to introduce extra complexity for improving this rule.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19178
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19178: [SPARK-21966][SQL]ResolveMissingReference rule should no...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19178
Could you trigger this test cuz I think this needs be fixed? @gatorsmile @HyukjinKwon
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19178: [SPARK-21966][SQL]ResolveMissingReference rule sh...
Posted by DonnyZone <gi...@git.apache.org>.
Github user DonnyZone closed the pull request at:
https://github.com/apache/spark/pull/19178
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org