[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/18270


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136708766
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -954,6 +954,15 @@ class Analyzer(
 try {
   expr transformUp {
 case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
+case u @ UnresolvedAttribute(nameParts)
+if resolver(u.name, VirtualColumn.hiveGroupingIdName) =>
+  withPosition(u) {
+plan.resolve(nameParts, resolver).getOrElse {
+  Alias(
+catalog.lookupFunction(FunctionIdentifier("grouping_id"), 
Nil),
+VirtualColumn.hiveGroupingIdName)()
+}
+  }
 case u @ UnresolvedAttribute(nameParts) =>
--- End diff --

and here too. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136708762
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -863,6 +854,15 @@ class Analyzer(
   case q: LogicalPlan =>
 logTrace(s"Attempting to resolve ${q.simpleString}")
 q.transformExpressionsUp  {
+  case u @ UnresolvedAttribute(nameParts)
+  if resolver(u.name, VirtualColumn.hiveGroupingIdName) =>
+withPosition(u) {
+  q.resolveChildren(nameParts, resolver).getOrElse {
+Alias(
+  
catalog.lookupFunction(FunctionIdentifier("grouping_id"), Nil),
+  VirtualColumn.hiveGroupingIdName)()
+  }
+}
   case u @ UnresolvedAttribute(nameParts) =>
--- End diff --

just need to add `if !resolver(u.name, VirtualColumn.hiveGroupingIdName)` 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136708657
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -954,6 +954,15 @@ class Analyzer(
 try {
   expr transformUp {
 case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
+case u @ UnresolvedAttribute(nameParts)
--- End diff --

This change looks suspicious. Doesn't `ResolveMissingReferences` resolve 
`grouping_id` used in `order by`?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136708299
  
--- Diff: 
sql/core/src/test/resources/sql-tests/results/group-analytics.sql.out ---
@@ -223,12 +223,19 @@ grouping_id() can only be used with 
GroupingSets/Cube/Rollup;
 
 
 -- !query 16
-SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year)
+SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year) ORDER BY grouping__id, course, year
 -- !query 16 schema
-struct<>
+struct
 -- !query 16 output
-org.apache.spark.sql.AnalysisException
-grouping__id is deprecated; use grouping_id() instead;
+Java20120
--- End diff --

Are you manually editing this `group-analytics.sql.out`? The test failure 
is due to mismatching between spaces and tab.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[cenyuhai]

Github user cenyuhai commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136695110
  
--- Diff: 
sql/core/src/test/resources/sql-tests/results/group-analytics.sql.out ---
@@ -223,12 +223,19 @@ grouping_id() can only be used with 
GroupingSets/Cube/Rollup;
 
 
 -- !query 16
-SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year)
+SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year) ORDER BY grouping__id, course, year
--- End diff --

thanks for your tips


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136694895
  
--- Diff: 
sql/core/src/test/resources/sql-tests/results/group-analytics.sql.out ---
@@ -223,12 +223,19 @@ grouping_id() can only be used with 
GroupingSets/Cube/Rollup;
 
 
 -- !query 16
-SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year)
+SELECT course, year, grouping__id FROM courseSales GROUP BY CUBE(course, 
year) ORDER BY grouping__id, course, year
--- End diff --

Why you only commit `group-analytics.sql.out`? You should also commit 
modified `group-analytics.sql`.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136121931
  
--- Diff: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala 
---
@@ -1414,6 +1414,19 @@ class SQLQuerySuite extends QueryTest with 
SQLTestUtils with TestHiveSingleton {
   ).map(i => Row(i._1, i._2, i._3)))
   }
 
+  test("SPARK-21055 replace grouping__id: Wrong Result for Rollup #1") {
--- End diff --

Instead of adding a new one, you can just make a change on the existing 
one. 
```Scala
  test("SPARK-8976 Wrong Result for Rollup #1") {
Seq("grouping_id()", "grouping__id").foreach { gid =>
  checkAnswer(sql(
s"SELECT count(*) AS cnt, key % 5, $gid FROM src GROUP BY key%5 
WITH ROLLUP"),
Seq(
  (113, 3, 0),
  (91, 0, 0),
  (500, null, 1),
  (84, 1, 0),
  (105, 2, 0),
  (107, 4, 0)
).map(i => Row(i._1, i._2, i._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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[cenyuhai]

GitHub user cenyuhai reopened a pull request:

https://github.com/apache/spark/pull/18270

[SPARK-21055][SQL] replace grouping__id with grouping_id()

## What changes were proposed in this pull request?
spark does not support grouping__id, it has grouping_id() instead.
But it is not convenient for hive user to change to spark-sql
so this pr is to replace grouping__id with grouping_id()
hive user need not to alter their scripts

## How was this patch tested?

test with SQLQuerySuite.scala


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/cenyuhai/spark SPARK-21055

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/18270.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 #18270


commit 36ff72a44ad19efe5bcb2fe461a700d4c54f89ef
Author: CenYuhai 
Date:   2017-08-30T15:22:13Z

eplace grouping__id with 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[cenyuhai]

Github user cenyuhai closed the pull request at:

https://github.com/apache/spark/pull/18270


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[cenyuhai]

Github user cenyuhai commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r136082775
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -954,6 +951,12 @@ class Analyzer(
 try {
   expr transformUp {
 case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
+case u @ UnresolvedAttribute(_) if resolver(u.name, 
VirtualColumn.hiveGroupingIdName) =>
--- End diff --

I don't think I can do it，because ResolveFunctions is behind 
ResolveGroupingAnalytics


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/18270#discussion_r134619232
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -954,6 +951,12 @@ class Analyzer(
 try {
   expr transformUp {
 case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
+case u @ UnresolvedAttribute(_) if resolver(u.name, 
VirtualColumn.hiveGroupingIdName) =>
--- End diff --

Could we do all the changes you made in this file in the rule 
`ResolveFunctions`?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #18270: [SPARK-21055][SQL] replace grouping__id with grou...

[cenyuhai]

GitHub user cenyuhai opened a pull request:

https://github.com/apache/spark/pull/18270

[SPARK-21055][SQL] replace grouping__id with grouping_id()

## What changes were proposed in this pull request?
spark does not support grouping__id, it has grouping_id() instead.
But it is not convenient for hive user to change to spark-sql
so this pr is to replace grouping__id with grouping_id()
hive user need not to alter their scripts

## How was this patch tested?

test with SQLQuerySuite.scala


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/cenyuhai/spark SPARK-21055

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/18270.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 #18270


commit 6fd567ca8d2d9e302612f281f4143033efa2c156
Author: cenyuhai <261810...@qq.com>
Date:   2017-06-11T12:04:05Z

replace grouping__id with 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org