Skip to content

Commit

Permalink
[SPARK-25979][SQL] Window function: allow parentheses around window r…
Browse files Browse the repository at this point in the history
…eference

## What changes were proposed in this pull request?

Very minor parser bug, but possibly problematic for code-generated queries:

Consider the following two queries:
```
SELECT avg(k) OVER (w) FROM kv WINDOW w AS (PARTITION BY v ORDER BY w) ORDER BY 1
```
and
```
SELECT avg(k) OVER w FROM kv WINDOW w AS (PARTITION BY v ORDER BY w) ORDER BY 1
```
The former, with parens around the OVER condition, fails to parse while the latter, without parens, succeeds:
```
Error in SQL statement: ParseException:
mismatched input '(' expecting {<EOF>, ',', 'FROM', 'WHERE', 'GROUP', 'ORDER', 'HAVING', 'LIMIT', 'LATERAL', 'WINDOW', 'UNION', 'EXCEPT', 'MINUS', 'INTERSECT', 'SORT', 'CLUSTER', 'DISTRIBUTE'}(line 1, pos 19)

== SQL ==
SELECT avg(k) OVER (w) FROM kv WINDOW w AS (PARTITION BY v ORDER BY w) ORDER BY 1
-------------------^^^
```
This was found when running the cockroach DB tests.

I tried PostgreSQL, The SQL with parentheses  is also workable.

## How was this patch tested?

Unit test

Closes apache#22987 from gengliangwang/windowParentheses.

Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: gatorsmile <[email protected]>
  • Loading branch information
gengliangwang authored and jackylee-ch committed Feb 18, 2019
1 parent 039ea17 commit 2c64e49
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ namedWindow

windowSpec
: name=identifier #windowRef
| '('name=identifier')' #windowRef
| '('
( CLUSTER BY partition+=expression (',' partition+=expression)*
| ((PARTITION | DISTRIBUTE) BY partition+=expression (',' partition+=expression)*)?
Expand Down
6 changes: 6 additions & 0 deletions sql/core/src/test/resources/sql-tests/inputs/window.sql
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,9 @@ last_value(false, false) OVER w AS last_value_contain_null
FROM testData
WINDOW w AS ()
ORDER BY cate, val;

-- parentheses around window reference
SELECT cate, sum(val) OVER (w)
FROM testData
WHERE val is not null
WINDOW w AS (PARTITION BY cate ORDER BY val);
19 changes: 18 additions & 1 deletion sql/core/src/test/resources/sql-tests/results/window.sql.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 22
-- Number of queries: 23


-- !query 0
Expand Down Expand Up @@ -363,3 +363,20 @@ NULL a false true false false true false
1 b false true false false true false
2 b false true false false true false
3 b false true false false true false


-- !query 22
SELECT cate, sum(val) OVER (w)
FROM testData
WHERE val is not null
WINDOW w AS (PARTITION BY cate ORDER BY val)
-- !query 22 schema
struct<cate:string,sum(CAST(val AS BIGINT)) OVER (PARTITION BY cate ORDER BY val ASC NULLS FIRST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW):bigint>
-- !query 22 output
NULL 3
a 2
a 2
a 4
b 1
b 3
b 6

0 comments on commit 2c64e49

Please sign in to comment.