[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/22582 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22582#discussion_r221413500
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -556,7 +556,7 @@ class Analyzer(
// Group-by expressions coming from SQL are implicit and need to
be deduced.
val groupByExprs = groupByExprsOpt.getOrElse {
val pivotColAndAggRefs =
-(pivotColumn.references ++
aggregates.flatMap(_.references)).toSet
+
aggregates.map(_.references).foldLeft(pivotColumn.references)(_ ++ _)
--- End diff --
Good catch!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22582#discussion_r221413483
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -556,7 +556,7 @@ class Analyzer(
// Group-by expressions coming from SQL are implicit and need to
be deduced.
val groupByExprs = groupByExprsOpt.getOrElse {
val pivotColAndAggRefs =
-(pivotColumn.references ++
aggregates.flatMap(_.references)).toSet
+
aggregates.map(_.references).foldLeft(pivotColumn.references)(_ ++ _)
--- End diff --
`pivotColumn.references ++ AttributeSet(aggregates)`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22582#discussion_r221409258
--- Diff: sql/core/src/test/resources/sql-tests/inputs/pivot.sql ---
@@ -297,3 +297,13 @@ PIVOT (
sum(earnings)
FOR course IN ('dotNET', 'Java')
);
+
+-- correctly handle pivot columns with different cases
+SELECT * FROM (
+ SELECT course, earnings, "a" as a, "z" as z, "b" as b, "y" as y, "c" as
c, "x" as x, "d" as d, "w" as w
+ FROM courseSales
+)
+PIVOT (
+ sum(Earnings)
--- End diff --
You can update the comment at line 291, too.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22582#discussion_r221409166
--- Diff: sql/core/src/test/resources/sql-tests/inputs/pivot.sql ---
@@ -297,3 +297,13 @@ PIVOT (
sum(earnings)
FOR course IN ('dotNET', 'Java')
);
+
+-- correctly handle pivot columns with different cases
+SELECT * FROM (
+ SELECT course, earnings, "a" as a, "z" as z, "b" as b, "y" as y, "c" as
c, "x" as x, "d" as d, "w" as w
+ FROM courseSales
+)
+PIVOT (
+ sum(Earnings)
--- End diff --
Nice catch, @mgaido91 .
Instead of adding this new test case, please change `sum(earnings)` to
`sum(Earnings)` in line 297.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22582: [SPARK-25505][SQL][FOLLOWUP] Fix for attributes c...
GitHub user mgaido91 opened a pull request: https://github.com/apache/spark/pull/22582 [SPARK-25505][SQL][FOLLOWUP] Fix for attributes cosmetically different in Pivot clause ## What changes were proposed in this pull request? #22519 introduced a bug when the attributes in the pivot clause are cosmetically different from the output ones (eg. different case). In particular, the problem is that the PR used a `Set[Attribute]` instead of an `AttributeSet`. ## How was this patch tested? added UT You can merge this pull request into a Git repository by running: $ git pull https://github.com/mgaido91/spark SPARK-25505_followup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/22582.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 #22582 commit 2ed74f3752ff178723c260c4d3c8d7d37ae3c99a Author: Marco Gaido Date: 2018-09-28T10:53:40Z [SPARK-25505][SQL][FOLLOWUP] Fix for attributes cosmetically different in Pivot clause --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
