You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xwu0226 <gi...@git.apache.org> on 2016/04/04 08:09:13 UTC
[GitHub] spark pull request: [SPARK-14361][SQL]Window function exclude clau...
GitHub user xwu0226 opened a pull request:
https://github.com/apache/spark/pull/12147
[SPARK-14361][SQL]Window function exclude clause
#### What changes were proposed in this pull request?
The current Spark SQL does not support the `exclusion` clause, which is part of ANSI SQL2003’s `Window` syntax. For example, IBM Netezza fully supports it as shown in the [document web link](https://www.ibm.com/support/knowledgecenter/SSULQD_7.1.0/com.ibm.nz.dbu.doc/c_dbuser_window_aggregation_family_syntax.html). This PR is to fill the gap.
##### Introduction
Below is the ANSI SQL2003’s `Window` syntax:
```
FUNCTION_NAME(expr) OVER {window_name | (window_specification)}
window_specification ::= [window_name] [partitioning] [ordering] [framing]
partitioning ::= PARTITION BY value[, value...] [COLLATE collation_name]
ordering ::= ORDER [SIBLINGS] BY rule[, rule...]
rule ::= {value | position | alias} [ASC | DESC] [NULLS {FIRST | LAST}]
framing ::= {ROWS | RANGE} {start | between} [exclusion]
start ::= {UNBOUNDED PRECEDING | unsigned-integer PRECEDING | CURRENT ROW}
between ::= BETWEEN bound AND bound
bound ::= {start | UNBOUNDED FOLLOWING | unsigned-integer FOLLOWING}
exclusion ::= {EXCLUDE CURRENT ROW | EXCLUDE GROUP | EXCLUDE TIES | EXCLUDE NO OTHERS}
```
Exclusion clause can be used to excluded certain rows from the window framing when calculating window aggregation function (e.g. AVG, SUM, MAX, MIN, COUNT, etc) related to current row. Types of window functions that are not supported are listed below:
1. Offset functions, such as lead(), lag()
2. Ranking functions, such as rank(), dense_rank(), percent_rank(), cume_dist, ntile()
3. Row number function, such as row_number()
##### Definition
Syntax | Description
------------ | -------------
EXCLUDE CURRENT ROW | Specifies excluding the current row.
EXCLUDE GROUP | Specifies excluding the current row and all rows that are tied with it. Ties occur when there is a match on the order column or columns.
EXCLUDE NO OTHERS | Specifies not excluding any rows. This value is the default if you specify no exclusion.
EXCLUDE TIES | Specifies excluding all rows that are tied with the current row (peer rows), but retaining the current row.
##### Use-case Examples:
- Let's say you want to find out for every employee, where is his/her salary at compared to the average salary of those within the same department and whose ages are within 5 years younger and older. The query could be:
```SQL
SELECT NAME, DEPT_ID, SALARY, AGE, AVG(SALARY) AS AVG_WITHIN_5_YEAR
OVER(PARTITION BY DEPT_ID
ORDER BY AGE
RANGE BETWEEN 5 PRECEDING AND 5 FOLLOWING
EXCLUDE CURRENT ROW)
FROM EMPLOYEE
```
- Let's say you want to compare every customer's yearly purchase with other customers' average yearly purchase who are at different age group from the current customer. The query could be:
```SQL
SELECT CUST_NAME, AGE, PROD_CATEGORY, YEARLY_PURCHASE, AVG(YEARLY_PURCHASE)
OVER(PARTITION BY PROD_CATEGORY
ORDER BY AGE
RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUND FOLLOWING
EXCLUDE GROUP)
FROM CUSTOMER_PURCHASE_SUM
```
##### Implementation
**catalyst.parser.SqlBase.g4**
- add excludeClause definition and append to each case in windowFrame
- add excludeType definition for CURRENT ROW,GOUP, TIES, NO OTHERS
**catalyst.parser.AstBuilder.scala**
- parse excludeClause to generate `ExcludeClause `object to set into `SpecifiedWindowFrame`
**catalyst.expressions.windowExpressions.scala**
- add `ExcludeClause `and `ExcludeType`
- add `ExcludeClause `object as a parameter to `SpecifiedWindowFrame`
- validate EXCLUDE GROUP/TIES requires ordered window frame
**catalyst.analysis.Analyzer.scala**
- validate exclude clause does not support `RowNumberLike`, `RankLike `and `OffsetWindowFunction`
**execution.Window.scala**
- implement the exclusion logic in sliding frame, expanding frame, shrinking frame and whole partition frame
**Some Test case examples:**
**DDL and DML:**
sql("create table table1 (col1 int, col2 int, col3 int)")
sql("insert into table1 select 6, 12, 10")
sql("insert into table1 select 6, 11, 4")
sql("insert into table1 select 6, 13, 11")
sql("insert into table1 select 6, 9, 10")
sql("insert into table1 select 6, 15, 8")
sql("insert into table1 select 6, 10, 1")
sql("insert into table1 select 6, 15, 8")
sql("insert into table1 select 6, 7, 4")
sql("insert into table1 select 6, 7, 8")
**Queries:**
```
// sliding frame with exclude current row
select col1, col2, col3, sum(col2)
over (partition by col1
order by col3
rows between 2 preceding and 2 following
exclude current row)
from table1 where col1 = 6
```
##### Results
col1 | col2 | col3 | sum(col2)
----- | ------ | ----- | -------
6 | 10 | 1 | 18
6 | 11 | 4 | 32
**6** | **7** | **4** | **51**
6 | 15 | 8 | 40
6 | 15 | 8 | 40
6 | 7 | 8 | 51
6 | 12 | 10 | 44
6 | 9 | 10 | 32
6 | 6 | 13 | 11 | 21
In this case, for example, if we are looking at the (6, 7, 4) row, the frame around this current row has 5 rows (6, 10, 1), (6,11, 4), **(6, 7, 4)**, (6, 15, 8), (6, 15, 8), including the current row. With the `exclude current row` clause, then the calculation of sum(col2) is on the other 4 rows, which is 10 + 11 + 15 + 15 = 51.
```
//expanding frame with exclude current row
select col1, col2, col3, sum(col2)
over (partition by col1
order by col3
rows between unbounded preceding and current row
exclude group)
from table1 where col1 = 6
```
##### Results
col1 | col2 | col3 | sum(col2)
----- | ------ | ----- | -------
6 | 10 | 1 | null
6 | 11 | 4 | 10
**6** | **7** | **4** | **10**
6 | 15 | 8 | 28
6 | 15 | 8 | 28
6 | 7 | 8 | 28
6 | 12 | 10 | 65
6 | 9 | 10 | 65
6 | 13 | 11 | 86
In this case, for example, if we are looking at row (6, 7, 4), the frame around this current row has 3 rows (6, 10, 1), (6, 11, 4) and (6, 7, 4). Then, with `exclude group` clause, rows (6, 11, 4) and (6, 7, 4) will be excluded because the values of order-by column `col3` on these 2 rows are the same, thus in the same group. Therefore, the sum(col2) at current frame is 10.
For testcases are located in hive/execution/**WindowFunctionExcludeSuite.scala**
#### How was this patch tested?
1. created unit tests to cover all framing cases, each with different exclude types.
2. run regtest over catalyst, sql, and hive tests under SQL component.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xwu0226/spark window_exclude
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12147.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 #12147
----
commit e783b1cc0450b3a3f4b98b30e095130955cd779b
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-08T01:02:20Z
supporting EXCLUDE clause in Window function framing
commit c9e1dc4f5c30ac3051aa2de0a567a39d9dfb6fe3
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-11T00:14:51Z
update
commit f32457de2df539a7e14fb584b4dc589202104548
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-11T04:50:21Z
update testcase
commit fa03d45e11d9f02613c8df0c2b459e8d0b195262
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-18T01:34:05Z
refining testcase for adding exclude clause for window function framing
commit 564642bdfa2be93dde9f5037bc97a394bb3199a8
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-21T17:17:53Z
testcase update
commit 3a7b8f59a9d08de4fe4caf0576a683c49fc0e524
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-22T17:21:16Z
more change
commit 50f2dffbd6c650ba734f9cfb4655bb21e50d30ab
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-28T19:05:10Z
update
commit 82f474cca66ad8d205f69df510171f984372b96b
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-28T23:37:10Z
recover my change lost
commit 2b28c87b3f860a57ad9a3db73a9b4747cba3a619
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-29T01:54:42Z
more
commit a34dbceb0e65820bfe037ca759bb3fd9a04c9b2f
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-30T23:19:21Z
modifed to support antlr4
commit 45adccbe2197b9e326cf319de266101470cd03c9
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-03-31T14:48:34Z
modify testcases
commit 4c14e9b8c09eb44eb62c396fb453552881bb70c0
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T02:31:49Z
modified upon review
commit 5dca37cf6b9ab9848f798412014b9ec59e46b2b3
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T03:42:29Z
modify upon review
commit 1ae0d048e580491d8e7b37b4d61c7ea6ac37f03f
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T05:30:27Z
update upon review
commit 1521ef3ffe6a0085a1eb718c3ab8c7c8c636f633
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T18:11:50Z
move frame validation for exclude clause to analysis from parser
commit b88ff1d9fa0a5d94ac3166beb49aa7a2fbfac231
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T18:23:17Z
add comment
commit a4ca051f2c5e8a03e855a39e31881f63d567d551
Author: xin Wu <xi...@us.ibm.com>
Date: 2016-04-03T20:51:19Z
add testcase for validating that exclude group/ties requires orderby clause in the window specification
----
---
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-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205166586
@xwu0226 this look interesting. I will try to get to this in the next couple of days.
The thing I am really worried about is performance. Could you please add benchmarks, comparing the old and new situations. See https://issues.apache.org/jira/secure/attachment/12745984/perf_test3.scala as a starting point.
---
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-14361][SQL]Window function exclude clau...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205166765
**[Test build #2739 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2739/consoleFull)** for PR 12147 at commit [`a4ca051`](https://github.com/apache/spark/commit/a4ca051f2c5e8a03e855a39e31881f63d567d551).
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by thesuperzapper <gi...@git.apache.org>.
Github user thesuperzapper commented on the issue:
https://github.com/apache/spark/pull/12147
Is there any movement on this?
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121567532
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1825,6 +1847,18 @@ class Analyzer(
}
/**
+ * Exclude clause does not support
+ * RowNumberLike: ROW_NUMER()
+ * RankLike: RANK(), DENSE_RANK(), CUME_DIST(), etc.
+ * OffsetWindowFunctions: Lead(), Lag()
+ * @param wf
--- End diff --
I think `@param` can be removed as there is no doc for it and it's obvious. Also for `@return`.
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the issue:
https://github.com/apache/spark/pull/12147
Since there are a lot of changes by other PRs over the last year, many changes in this PR may not be applicable and I am doing the rebase and necessary rework on this PR. Will update soon.
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 closed the pull request at:
https://github.com/apache/spark/pull/12147
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205341062
@hvanhovell , Thanks for the suggestions! I will look into this today..
---
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-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58442009
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala ---
@@ -262,19 +270,82 @@ object SpecifiedWindowFrame {
*/
def defaultWindowFrame(
hasOrderSpecification: Boolean,
- acceptWindowFrame: Boolean): SpecifiedWindowFrame = {
+ acceptWindowFrame: Boolean,
+ excludeSpec: ExcludeClause = ExcludeClause.defaultExclude): SpecifiedWindowFrame = {
if (hasOrderSpecification && acceptWindowFrame) {
// If order spec is defined and the window function supports user specified window frames,
// the default frame is RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW.
- SpecifiedWindowFrame(RangeFrame, UnboundedPreceding, CurrentRow)
+ SpecifiedWindowFrame(RangeFrame, UnboundedPreceding, CurrentRow, excludeSpec)
} else {
// Otherwise, the default frame is
// ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING.
- SpecifiedWindowFrame(RowFrame, UnboundedPreceding, UnboundedFollowing)
+ SpecifiedWindowFrame(RowFrame, UnboundedPreceding, UnboundedFollowing, excludeSpec)
}
}
}
+/**
+ * The trait used to represent an Exclude type .
+ */
+sealed trait ExcludeType
+
+/**
+ * Represents the type of Excluding Current Row
+ */
+case object ExcludeCurrentRow extends ExcludeType
+
+/**
+ * Specifies excluding the current row and all rows that are tied with it.
+ * Ties occur when there is a match on the order-by column or columns
+ */
+case object ExcludeGroup extends ExcludeType
+
+/**
+ * Specifies excluding all rows that are tied with the current row (peer rows),
+ * but retaining the current row.
+ */
+case object ExcludeTies extends ExcludeType
+
+/**
+ * Specifies not excluding any rows. This value is the default if you specify no exclusion.
+ */
+case object ExcludeNoOthers extends ExcludeType
+
+/**
+ * Exclude clause within window framing clause.
+ *
+ * 'EXCLUDE CURRENT ROW' means the current row for which the window function is calculated
+ * is excluded from the current fame.
+ * 'EXCLUDE GROUP' means that the nearby rows that match the current row with respect to
+ * the orderby expression together with the current row will be excluded from the calculation
+ * 'EXCLUDE TIES' means that the nearby rows that match the current row with respect to
+ * the orderby expression except the current row will be excluded from the calculation
+ * 'EXCLUDE NO OTHERS' means not excluding any rows from the calculation. This is the
+ * default behavior. Exclude types, GROUP and TIES, requires ORDER BY clause in the window
+ * specification clause.
+ * For example, users want to exclude current row from a sliding range framing:
+ * RANGE BETWEEN 2 PRECEDING AND 2 FOLLOWING EXCLUDE CURRENT ROW
+ * @param excludeType The type of exclusion as defined above
+ * @param valueOrdering The ordering operator that does the value comparison between 2 rows
+ * @param toBeCompared The projection of the orderby expression from a row
+ *
+ */
+case class ExcludeClause (
--- End diff --
For excluding, I need an Ordering operator that can compare 2 projections that are derived out of 2 rows based on the orde-by expression. So i was thinking that in execution time, i need to construct the code for Ordering and the projection. So maybe I can put it in execution.Window.scala?
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121592724
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -631,6 +631,17 @@ frameBound
| expression boundType=(PRECEDING | FOLLOWING)
;
+excludeClause
+ : EXCLUDE excludeType=CURRENT ROW
+ | EXCLUDE excludeType=GROUP
+ | EXCLUDE excludeType=TIES
+ | EXCLUDE excludeType=NO OTHERS
+ ;
+
+explainOption
--- End diff --
hmm. I don't think I introduced this change.. maybe I need to rebase to clean up my code before meaningful review. 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-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205492269
@yhuai Thank you very much for your response! I truely understand that you guys are very busy now.. Since Hermant has looked into it and provided some very helpful comments for me. Let me address the comments and you can take a look at it when you are ready.. Thanks again!
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121569249
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala ---
@@ -262,19 +270,55 @@ object SpecifiedWindowFrame {
*/
def defaultWindowFrame(
hasOrderSpecification: Boolean,
- acceptWindowFrame: Boolean): SpecifiedWindowFrame = {
+ acceptWindowFrame: Boolean,
+ excludeType: ExcludeType = ExcludeNoOthers): SpecifiedWindowFrame = {
--- End diff --
The default window frame should be `ExcludeNoOthers`. I think we don't need to add this parameter. We can directly add `ExcludeNoOthers` when constructing `SpecifiedWindowFrame` below.
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121570373
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/expressions/WindowSpec.scala ---
@@ -113,7 +132,25 @@ class WindowSpec private[sql](
between(RangeFrame, start, end)
}
- private def between(typ: FrameType, start: Long, end: Long): WindowSpec = {
+ /**
+ * Defines the frame boundaries, from `start` (inclusive) to `end` (inclusive). And specifies
+ * potential exclude type
+ *
+ * Both `start` and `end` are relative from the current row. For example, "0" means "current row",
+ * while "-1" means one off before the current row, and "5" means the five off after the
+ * current row.
+ *
+ * @param start boundary start, inclusive.
+ * The frame is unbounded if this is the minimum long value.
+ * @param end boundary end, inclusive.
+ * The frame is unbounded if this is the maximum long value.
+ * @since 1.4.0
+ */
+ def rangeBetweenExclude(start: Long, end: Long, exclude: String = ""): WindowSpec = {
--- End diff --
We already have `rowsBetween` which doesn't specify exclude type. So I think we don't need to have default value for `exclude`.
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121570768
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/WindowFunctionExcludeSuite.scala ---
@@ -0,0 +1,1000 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.hive.execution
+
+import org.apache.spark.sql.{AnalysisException, QueryTest, Row}
+import org.apache.spark.sql.hive.test.TestHiveSingleton
+import org.apache.spark.sql.test.SQLTestUtils
+
+class WindowFunctionExcludeSuite extends QueryTest with SQLTestUtils with TestHiveSingleton{
--- End diff --
Any significant difference between this test suite and `window_exclude.sql`? If not, we can just have `window_exclude.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 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-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58429360
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1398,6 +1403,18 @@ class Analyzer(
}
/**
+ * Exclude clause does not support
+ * RowNumberLike: ROW_NUMER()
+ * RankLike: RANK(), DENSE_RANK(), CUME_DIST(), etc.
+ * OffsetWindowFunctions: Lead(), Lag()
+ * @param wf
+ * @return
+ */
+ private def notSupportForExclude(wf: WindowFunction): Boolean =
+ wf.isInstanceOf[RowNumberLike] ||
--- End diff --
Check for `WindowFunction`.
---
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-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205166619
cc @yhuai
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121567336
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -631,6 +631,17 @@ frameBound
| expression boundType=(PRECEDING | FOLLOWING)
;
+excludeClause
+ : EXCLUDE excludeType=CURRENT ROW
+ | EXCLUDE excludeType=GROUP
+ | EXCLUDE excludeType=TIES
+ | EXCLUDE excludeType=NO OTHERS
+ ;
+
+explainOption
--- End diff --
Does this related to this change?
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/12147
Gentle ping @xwu0226, how is the update going?
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121567367
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -324,6 +324,23 @@ class Analyzer(
aggsBuffer += e
e
case e if isPartOfAggregation(e) => e
+ case e: GroupingID =>
--- End diff --
ditto.
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121593031
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala ---
@@ -262,19 +270,55 @@ object SpecifiedWindowFrame {
*/
def defaultWindowFrame(
hasOrderSpecification: Boolean,
- acceptWindowFrame: Boolean): SpecifiedWindowFrame = {
+ acceptWindowFrame: Boolean,
+ excludeType: ExcludeType = ExcludeNoOthers): SpecifiedWindowFrame = {
--- End diff --
Agree.
---
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-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58440553
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -713,6 +719,9 @@ PRECEDING: 'PRECEDING';
FOLLOWING: 'FOLLOWING';
CURRENT: 'CURRENT';
ROW: 'ROW';
+EXCLUDE: 'EXCLUDE';
--- End diff --
They are not reserved. I can add them to nonReserved rule. 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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121592843
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1825,6 +1847,18 @@ class Analyzer(
}
/**
+ * Exclude clause does not support
+ * RowNumberLike: ROW_NUMER()
+ * RankLike: RANK(), DENSE_RANK(), CUME_DIST(), etc.
+ * OffsetWindowFunctions: Lead(), Lag()
+ * @param wf
--- End diff --
Yes. 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-14361][SQL]Window function exclude clau...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205477566
@xwu0226 Thank you for working on this! I probably will not be able to review it before the 2.0 release. How about we revisit it later?
---
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 #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r121570239
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/expressions/WindowSpec.scala ---
@@ -113,7 +132,25 @@ class WindowSpec private[sql](
between(RangeFrame, start, end)
}
- private def between(typ: FrameType, start: Long, end: Long): WindowSpec = {
+ /**
+ * Defines the frame boundaries, from `start` (inclusive) to `end` (inclusive). And specifies
+ * potential exclude type
+ *
+ * Both `start` and `end` are relative from the current row. For example, "0" means "current row",
+ * while "-1" means one off before the current row, and "5" means the five off after the
+ * current row.
+ *
+ * @param start boundary start, inclusive.
+ * The frame is unbounded if this is the minimum long value.
+ * @param end boundary end, inclusive.
+ * The frame is unbounded if this is the maximum long value.
+ * @since 1.4.0
--- End diff --
1.4.0?
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12147
@xwu0226 Maybe close this PR if you do not have time to finish it? Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58429510
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -713,6 +719,9 @@ PRECEDING: 'PRECEDING';
FOLLOWING: 'FOLLOWING';
CURRENT: 'CURRENT';
ROW: 'ROW';
+EXCLUDE: 'EXCLUDE';
--- End diff --
Are these reserved keywords? If they are not please add them to the nonReserved rule.
---
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-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205541295
@hvanhovell Per your request, I did the benchmark between master and spark-14361 branch without the exclude case. The following is the results. I will continue on the exclude cases.
```
+-----+----------------+------+-----------+-----+
| Size| Name|Master|SPARK-14361| Diff|
+-----+----------------+------+-----------+-----+
| 2|Entire Partition| 24093| 19106|0.793|
| 2| Growing| 15166| 15032|0.991|
| 2| Shrinking| 15217| 15687|1.031|
| 2| Sliding| 16191| 16314|1.008|
| 8|Entire Partition| 14599| 14749|1.010|
| 8| Growing| 15382| 14883|0.968|
| 8| Shrinking| 15303| 15655|1.023|
| 8| Sliding| 15232| 15245|1.001|
| 32|Entire Partition| 15361| 14990|0.976|
| 32| Growing| 15922| 15073|0.947|
| 32| Shrinking| 16851| 19129|1.135|
| 32| Sliding| 15589| 15655|1.004|
| 128|Entire Partition| 14959| 14486|0.968|
| 128| Growing| 14659| 15095|1.030|
| 128| Shrinking| 18121| 18319|1.011|
| 128| Sliding| 15495| 15030|0.970|
| 256|Entire Partition| 15373| 14951|0.973|
| 256| Growing| 14857| 14545|0.979|
| 256| Shrinking| 20371| 22640|1.111|
| 256| Sliding| 15228| 14626|0.960|
| 1024|Entire Partition| 14774| 14337|0.970|
| 1024| Growing| 15888| 15413|0.970|
| 1024| Shrinking| 39446| 45678|1.158|
| 1024| Sliding| 15260| 14591|0.956|
| 4096|Entire Partition| 14652| 14839|1.013|
| 4096| Growing| 15064| 15099|1.002|
| 4096| Shrinking|147914| 193215|1.306|
| 4096| Sliding| 15225| 15476|1.016|
|16192|Entire Partition| 14155| 14600|1.031|
|16192| Growing| 14040| 14498|1.033|
|16192| Shrinking|699023| 883307|1.264|
|16192| Sliding| 20765| 17278|0.832|
+-----+----------------+------+-----------+-----+
```
It looks like the Shrinking case gets a bit worse than master while the partition size is larger, while other cases have some minor improvements. I will look at the logic in shrinking case. I may know the answer already.
---
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-14361][SQL]Window function exclude clau...
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58449736
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Window.scala ---
@@ -721,16 +791,37 @@ private[execution] final class SlidingWindowFunctionFrame(
*/
private[execution] final class UnboundedWindowFunctionFrame(
target: MutableRow,
- processor: AggregateProcessor) extends WindowFunctionFrame {
+ processor: AggregateProcessor,
+ excludeSpec: ExcludeClause) extends WindowFunctionFrame {
+
+ // Refer to the comment below in function prepare()
+ private[this] val buffer = new util.ArrayList[InternalRow]()
/** Prepare the frame for calculating a new partition. Process all rows eagerly. */
override def prepare(rows: RowBuffer): Unit = {
val size = rows.size()
processor.initialize(size)
- var i = 0
- while (i < size) {
- processor.update(rows.next())
- i += 1
+ if (excludeSpec.excludeType != ExcludeNoOthers) {
+ // For the exclude cases, the content of the window frame is always changing
+ // along with the current row. For example, a row that was not included before
+ // needs to be included now, or a row that was included before needs to be excluded now.
+ // So intermediate buffer is used for now..
+ // TODO: For potential performance gains, we need to look into the AggregateProcessor
--- End diff --
Thanks for the suggestions! Let me try that.
---
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-14361][SQL]Window function exclude clau...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205149936
Can one of the admins verify this patch?
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the issue:
https://github.com/apache/spark/pull/12147
I will re-open this PR once I resolve the issue. Thanks! @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14361][SQL]Window function exclude clau...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12147#issuecomment-205167086
**[Test build #2739 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2739/consoleFull)** for PR 12147 at commit [`a4ca051`](https://github.com/apache/spark/commit/a4ca051f2c5e8a03e855a39e31881f63d567d551).
* This patch **fails Scala style 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-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58430735
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Window.scala ---
@@ -721,16 +791,37 @@ private[execution] final class SlidingWindowFunctionFrame(
*/
private[execution] final class UnboundedWindowFunctionFrame(
target: MutableRow,
- processor: AggregateProcessor) extends WindowFunctionFrame {
+ processor: AggregateProcessor,
+ excludeSpec: ExcludeClause) extends WindowFunctionFrame {
+
+ // Refer to the comment below in function prepare()
+ private[this] val buffer = new util.ArrayList[InternalRow]()
/** Prepare the frame for calculating a new partition. Process all rows eagerly. */
override def prepare(rows: RowBuffer): Unit = {
val size = rows.size()
processor.initialize(size)
- var i = 0
- while (i < size) {
- processor.update(rows.next())
- i += 1
+ if (excludeSpec.excludeType != ExcludeNoOthers) {
+ // For the exclude cases, the content of the window frame is always changing
+ // along with the current row. For example, a row that was not included before
+ // needs to be included now, or a row that was included before needs to be excluded now.
+ // So intermediate buffer is used for now..
+ // TODO: For potential performance gains, we need to look into the AggregateProcessor
--- End diff --
There is no way this is possible. Think of a MIN aggregate for instance. What you can do is merge multiple buffers.
---
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 issue #12147: [SPARK-14361][SQL]Window function exclude clause
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on the issue:
https://github.com/apache/spark/pull/12147
@HyukjinKwon My rebased branch has broken most of the window exclude test cases. Trying to fix.
---
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-14361][SQL]Window function exclude clau...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12147#discussion_r58429851
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala ---
@@ -262,19 +270,82 @@ object SpecifiedWindowFrame {
*/
def defaultWindowFrame(
hasOrderSpecification: Boolean,
- acceptWindowFrame: Boolean): SpecifiedWindowFrame = {
+ acceptWindowFrame: Boolean,
+ excludeSpec: ExcludeClause = ExcludeClause.defaultExclude): SpecifiedWindowFrame = {
if (hasOrderSpecification && acceptWindowFrame) {
// If order spec is defined and the window function supports user specified window frames,
// the default frame is RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW.
- SpecifiedWindowFrame(RangeFrame, UnboundedPreceding, CurrentRow)
+ SpecifiedWindowFrame(RangeFrame, UnboundedPreceding, CurrentRow, excludeSpec)
} else {
// Otherwise, the default frame is
// ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING.
- SpecifiedWindowFrame(RowFrame, UnboundedPreceding, UnboundedFollowing)
+ SpecifiedWindowFrame(RowFrame, UnboundedPreceding, UnboundedFollowing, excludeSpec)
}
}
}
+/**
+ * The trait used to represent an Exclude type .
+ */
+sealed trait ExcludeType
+
+/**
+ * Represents the type of Excluding Current Row
+ */
+case object ExcludeCurrentRow extends ExcludeType
+
+/**
+ * Specifies excluding the current row and all rows that are tied with it.
+ * Ties occur when there is a match on the order-by column or columns
+ */
+case object ExcludeGroup extends ExcludeType
+
+/**
+ * Specifies excluding all rows that are tied with the current row (peer rows),
+ * but retaining the current row.
+ */
+case object ExcludeTies extends ExcludeType
+
+/**
+ * Specifies not excluding any rows. This value is the default if you specify no exclusion.
+ */
+case object ExcludeNoOthers extends ExcludeType
+
+/**
+ * Exclude clause within window framing clause.
+ *
+ * 'EXCLUDE CURRENT ROW' means the current row for which the window function is calculated
+ * is excluded from the current fame.
+ * 'EXCLUDE GROUP' means that the nearby rows that match the current row with respect to
+ * the orderby expression together with the current row will be excluded from the calculation
+ * 'EXCLUDE TIES' means that the nearby rows that match the current row with respect to
+ * the orderby expression except the current row will be excluded from the calculation
+ * 'EXCLUDE NO OTHERS' means not excluding any rows from the calculation. This is the
+ * default behavior. Exclude types, GROUP and TIES, requires ORDER BY clause in the window
+ * specification clause.
+ * For example, users want to exclude current row from a sliding range framing:
+ * RANGE BETWEEN 2 PRECEDING AND 2 FOLLOWING EXCLUDE CURRENT ROW
+ * @param excludeType The type of exclusion as defined above
+ * @param valueOrdering The ordering operator that does the value comparison between 2 rows
+ * @param toBeCompared The projection of the orderby expression from a row
+ *
+ */
+case class ExcludeClause (
--- End diff --
Do we need this class in this location? It seems like this is bound to its implementation? Just passing ExcludeTypes should be fine for now.
---
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