You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by liancheng <gi...@git.apache.org> on 2015/03/22 15:01:38 UTC
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
GitHub user liancheng opened a pull request:
https://github.com/apache/spark/pull/5129
[SPARK-6452] [SQL] Checks for missing attributes and unresolved operator for all types of operator
In `CheckAnalysis`, `Filter` and `Aggregate` are checked in separate case clauses, thus never hit those clauses for unresolved operators and missing input attributes.
This PR also removes the `prettyString` call when generating error message for missing input attributes. Because result of `prettyString` doesn't contain expression ID, and may give confusing messages like
> resolved attributes a missing from a
cc @rxin
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liancheng/spark spark-6452
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5129.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 #5129
----
commit 46facb9e943a96fb1cda1301d1671a0c0cd051a8
Author: Cheng Lian <li...@databricks.com>
Date: 2015-03-22T13:51:48Z
Checks for missing attributes and unresolved operator for all types of operator
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26912753
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
--- End diff --
Because any `Aggregate` operator must match the two case branches above, and will never match those two for unresolved operator and missing input attribute(s).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26913115
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
--- End diff --
@chenghao-intel Thanks for the explanation, actually I didn't know what it is while writing this PR :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84855158
[Test build #28983 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28983/consoleFull) for PR 5129 at commit [`0c49010`](https://github.com/apache/spark/commit/0c4901092aa13f478d556e99983513b3a22a7d9b).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84887390
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28983/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26912596
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
--- End diff --
it was a rhetorical question btw -- my feedback was about we gotta be able to answer what grouping__id is, and why it is there, in inline comments for this code block.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84940249
Rebased after #5132 being merged. All comments are addressed. Thanks everyone for the review!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84611682
[Test build #28965 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28965/consoleFull) for PR 5129 at commit [`46facb9`](https://github.com/apache/spark/commit/46facb9e943a96fb1cda1301d1671a0c0cd051a8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84643069
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28965/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84643066
**[Test build #28965 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28965/consoleFull)** for PR 5129 at commit [`46facb9`](https://github.com/apache/spark/commit/46facb9e943a96fb1cda1301d1671a0c0cd051a8) after a configured wait of `120m`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26906097
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
--- End diff --
would be great to add some comment on what this case is checking here.
(what is "grouping__id"?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26912927
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
--- End diff --
ah ok makes sense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26906086
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
--- End diff --
why not just collapse both into a single pattern match?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84887386
[Test build #28983 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28983/consoleFull) for PR 5129 at commit [`0c49010`](https://github.com/apache/spark/commit/0c4901092aa13f478d556e99983513b3a22a7d9b).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-85391344
Thanks! Merged to master and 1.3
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84939647
[Test build #28991 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28991/consoleFull) for PR 5129 at commit [`52cdc69`](https://github.com/apache/spark/commit/52cdc69fcbf40968628b62366891fd5e43b80299).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26913783
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
!o.references.filter(_.name != "grouping__id").subsetOf(o.inputSet) =>
- val missingAttributes = (o.references -- o.inputSet).map(_.prettyString).mkString(",")
- val input = o.inputSet.map(_.prettyString).mkString(",")
+ val missingAttributes = (o.references -- o.inputSet).mkString(",")
--- End diff --
I'd like to take advantage over #5082, but I still have questions about that PR. So maybe I won't include related changes in this PR, but wait until #5082 is merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84964991
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28991/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5129#issuecomment-84964984
[Test build #28991 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28991/consoleFull) for PR 5129 at commit [`52cdc69`](https://github.com/apache/spark/commit/52cdc69fcbf40968628b62366891fd5e43b80299).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26912947
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
!o.references.filter(_.name != "grouping__id").subsetOf(o.inputSet) =>
- val missingAttributes = (o.references -- o.inputSet).map(_.prettyString).mkString(",")
- val input = o.inputSet.map(_.prettyString).mkString(",")
+ val missingAttributes = (o.references -- o.inputSet).mkString(",")
--- End diff --
@cloud-fan Agree, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5129
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26906084
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala ---
@@ -199,4 +199,21 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {
assert(pl(3).dataType == DecimalType.Unlimited)
assert(pl(4).dataType == DoubleType)
}
+
+ test("SPARK-6452: CheckAnalysis should throw when Aggregate contains missing attributes") {
--- End diff --
should throw AnalysisException ? otherwise it is an incomplete sentence
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26908627
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
--- End diff --
`GROUPING__ID` is the virtual column for GROUPING SETS, actually there is a constant definition at https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala#L231
We'd better use `VirtualColumn.groupingIdName` instead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26912762
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
!o.references.filter(_.name != "grouping__id").subsetOf(o.inputSet) =>
- val missingAttributes = (o.references -- o.inputSet).map(_.prettyString).mkString(",")
- val input = o.inputSet.map(_.prettyString).mkString(",")
+ val missingAttributes = (o.references -- o.inputSet).mkString(",")
--- End diff --
yes,here is a PR to simplify this #5082
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6452] [SQL] Checks for missing attribut...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5129#discussion_r26911770
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala ---
@@ -85,14 +85,19 @@ class CheckAnalysis {
cleaned.foreach(checkValidAggregateExpression)
+ case _ => // Fallbacks to the following checks
+ }
+
+ operator match {
case o if o.children.nonEmpty &&
!o.references.filter(_.name != "grouping__id").subsetOf(o.inputSet) =>
- val missingAttributes = (o.references -- o.inputSet).map(_.prettyString).mkString(",")
- val input = o.inputSet.map(_.prettyString).mkString(",")
+ val missingAttributes = (o.references -- o.inputSet).mkString(",")
--- End diff --
Can we use `o.missingInput` instead of `o.references -- o.inputSet` here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org