You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jinxing64 <gi...@git.apache.org> on 2018/05/15 11:51:37 UTC
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
GitHub user jinxing64 opened a pull request:
https://github.com/apache/spark/pull/21330
[SPARK-22234] Support distinct window functions
## What changes were proposed in this pull request?
This pr proposes to support distinct window functions. After this change, query like below are supported:
```
SELECT val, cate,
sum(val) OVER (PARTITION BY cate) AS sum1,
sum(DISTINCT val) OVER (PARTITION BY cate) AS sum2
FROM testData
```
In this pr:
1. ORDER BY and distinct window function don't work at the same time due to performance concern and implementation requirement.
2. Insert distinct fields into the order by list, thus during counting, we only need to compare the current row against the previous row, i we ignore the current row if they have same projected values.
## How was this patch tested?
Test added and more tests to be added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jinxing64/spark SPARK-22234
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21330.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 #21330
----
commit 65ad0740e21ffb16cfe6d9ee076f36d93a9aaaea
Author: jinxing <ji...@...>
Date: 2018-05-15T10:11:40Z
[SPARK-22234] Support distinct window functions
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r189433323
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1883,7 +1883,19 @@ class Analyzer(
// Second, we group extractedWindowExprBuffer based on their Partition and Order Specs.
val groupedWindowExpressions = extractedWindowExprBuffer.groupBy { expr =>
val distinctWindowSpec = expr.collect {
- case window: WindowExpression => window.windowSpec
+ case window: WindowExpression =>
+ val winExpr = window.windowFunction
+ val distinctOpt = winExpr.find (expr => expr.isInstanceOf[AggregateExpression]
+ && expr.asInstanceOf[AggregateExpression].isDistinct)
+ if (distinctOpt.nonEmpty && window.windowSpec.orderSpec.nonEmpty) {
--- End diff --
So this only works for global window frames? If it does why did you implement distinct processing for growing, sliding and shrinking frames?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90855 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90855/testReport)** for PR 21330 at commit [`d86ba76`](https://github.com/apache/spark/commit/d86ba76e3e55820c6eb4c859b33bdced7d84b595).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90859/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90640 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90640/testReport)** for PR 21330 at commit [`65ad074`](https://github.com/apache/spark/commit/65ad0740e21ffb16cfe6d9ee076f36d93a9aaaea).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/21330
If this feature is interested, could you please help start the review @jiangxb1987
Thanks a lot.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r189456451
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/window/WindowExec.scala ---
@@ -213,10 +218,24 @@ case class WindowExec(
(expressions, schema) =>
newMutableProjection(expressions, schema, subexpressionEliminationEnabled))
+ def createOrdering(): Ordering[InternalRow] = {
+ val exprs = orderSpec.map(_.child)
+ val sortExprs = orderSpec.zipWithIndex.map { case (e, i) =>
+ SortOrder(BoundReference(i, e.dataType, e.nullable), e.direction)
+ }
+ val ordering = newOrdering(sortExprs, Nil)
+ new Ordering[InternalRow] {
+ override def compare(x: InternalRow, y: InternalRow): Int = {
+ ordering.compare(newMutableProjection(exprs, child.output)(x),
--- End diff --
I should refine this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3390/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90640 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90640/testReport)** for PR 21330 at commit [`65ad074`](https://github.com/apache/spark/commit/65ad0740e21ffb16cfe6d9ee076f36d93a9aaaea).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3386/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2460/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r213000357
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1883,7 +1883,19 @@ class Analyzer(
// Second, we group extractedWindowExprBuffer based on their Partition and Order Specs.
val groupedWindowExpressions = extractedWindowExprBuffer.groupBy { expr =>
val distinctWindowSpec = expr.collect {
- case window: WindowExpression => window.windowSpec
+ case window: WindowExpression =>
+ val winExpr = window.windowFunction
+ val distinctOpt = winExpr.find (expr => expr.isInstanceOf[AggregateExpression]
+ && expr.asInstanceOf[AggregateExpression].isDistinct)
+ if (distinctOpt.nonEmpty && window.windowSpec.orderSpec.nonEmpty) {
+ failAnalysis(s"ORDER BY cannot be used with DISTINCT: $window")
--- End diff --
Yes, it's supported and some other engines like ODPS...
Thanks a lot for comment @HyukjinKwon
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3231/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/21330
Jenkins, retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90859 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90859/testReport)** for PR 21330 at commit [`d86ba76`](https://github.com/apache/spark/commit/d86ba76e3e55820c6eb4c859b33bdced7d84b595).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r212800158
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1883,7 +1883,19 @@ class Analyzer(
// Second, we group extractedWindowExprBuffer based on their Partition and Order Specs.
val groupedWindowExpressions = extractedWindowExprBuffer.groupBy { expr =>
val distinctWindowSpec = expr.collect {
- case window: WindowExpression => window.windowSpec
+ case window: WindowExpression =>
+ val winExpr = window.windowFunction
+ val distinctOpt = winExpr.find (expr => expr.isInstanceOf[AggregateExpression]
+ && expr.asInstanceOf[AggregateExpression].isDistinct)
+ if (distinctOpt.nonEmpty && window.windowSpec.orderSpec.nonEmpty) {
+ failAnalysis(s"ORDER BY cannot be used with DISTINCT: $window")
--- End diff --
Just out of curiosity, does hive have the same limitation? If so, the current way, roughly ordered rows and checking previous row for distinct windows makes sense to me.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/21330
@hvanhovell
Thanks for looking into this.
>So this only works for global window frames?
It's for entire-partition/global window frame, growing frame, shrinking frame and moving/sliding frame. And I added some more tests for different kinds of frames in current change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r189433380
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/window/WindowExec.scala ---
@@ -213,10 +218,24 @@ case class WindowExec(
(expressions, schema) =>
newMutableProjection(expressions, schema, subexpressionEliminationEnabled))
+ def createOrdering(): Ordering[InternalRow] = {
+ val exprs = orderSpec.map(_.child)
+ val sortExprs = orderSpec.zipWithIndex.map { case (e, i) =>
+ SortOrder(BoundReference(i, e.dataType, e.nullable), e.direction)
+ }
+ val ordering = newOrdering(sortExprs, Nil)
+ new Ordering[InternalRow] {
+ override def compare(x: InternalRow, y: InternalRow): Int = {
+ ordering.compare(newMutableProjection(exprs, child.output)(x),
--- End diff --
I am pretty sure this creates two new mutable projections for each comparison.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/21330
Also cc @kiszk @HyukjinKwon
Really appreciate if you can leave some comments :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21330
cc @jiangxb1987
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90855 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90855/testReport)** for PR 21330 at commit [`d86ba76`](https://github.com/apache/spark/commit/d86ba76e3e55820c6eb4c859b33bdced7d84b595).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90855/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21330
cc @hvanhovell
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:
https://github.com/apache/spark/pull/21330
@cloud-fan @jiangxb1987 @cenyuhai
Do you think this change makes sense?
I can keep working on this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21330
**[Test build #90859 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90859/testReport)** for PR 21330 at commit [`d86ba76`](https://github.com/apache/spark/commit/d86ba76e3e55820c6eb4c859b33bdced7d84b595).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21330: [SPARK-22234] Support distinct window functions
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90640/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21330: [SPARK-22234] Support distinct window functions
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r213003383
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1883,7 +1883,19 @@ class Analyzer(
// Second, we group extractedWindowExprBuffer based on their Partition and Order Specs.
val groupedWindowExpressions = extractedWindowExprBuffer.groupBy { expr =>
val distinctWindowSpec = expr.collect {
- case window: WindowExpression => window.windowSpec
+ case window: WindowExpression =>
+ val winExpr = window.windowFunction
+ val distinctOpt = winExpr.find (expr => expr.isInstanceOf[AggregateExpression]
+ && expr.asInstanceOf[AggregateExpression].isDistinct)
+ if (distinctOpt.nonEmpty && window.windowSpec.orderSpec.nonEmpty) {
+ failAnalysis(s"ORDER BY cannot be used with DISTINCT: $window")
--- End diff --
Was wondering if they have the same limitation for clarification.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org