Repository: andialbrecht/sqlparse

Issue:
Incorrect parsing of expressions in SELECT when "<Window Function> OVER ( ... )" is present
When there is a Window Function (e.g., `ROW_NUMBER()`) followed by `OVER ( ... )` in the expressions after the `SELECT` statement, these expressions are not split correctly into individual `Identifier`s. For example, let's have

    query = ("SELECT uid "
             ", ROW_NUMBER() OVER ( PARTITION BY table.col_a ORDER BY table.col_b DESC ) AS row "
             ", full_name AS name "
             ", another_column AS acol "
             ", ROW_NUMBER() OVER ( PARTITION BY table.col_c ORDER BY table.col_d ASC ) AS row_lo "
             ", ROW_NUMBER() OVER ( PARTITION BY table.col_e ORDER BY table.col_f DESC ) AS row_hi "
             ", last_column AS lc "
             "FROM table")

which returns seven columns (`uid`, `row`, `name`, `acol`, `row_lo`, `row_hi`, `lc`). One would expect this to be parsed as (excl. whitespaces and punctuation):

  * `<DML> SELECT`
  * `<IdentifierList>`
    * `<Identifier> uid`
    * `<Identifier> row`
    * `<Identifier> name`
    * `<Identifier> acol`
    * `<Identifier> row_lo`
    * `<Identifier> row_hi`
    * `<Identifier> lc`
  * `<Keyword> FROM`
  * `<Keyword> table`
    
However, `parse(query)` returns the following:

    [<DML 'SELECT' at 0x2269219A1C0>,
     <Whitespace ' ' at 0x2269219A220>,
     <IdentifierList 'uid , ...' at 0x226921CF510>,
     <Whitespace ' ' at 0x2269219A520>,
     <Keyword 'OVER' at 0x2269219A580>,
     <Whitespace ' ' at 0x2269219A5E0>,
     <IdentifierList '( PART...' at 0x226921CF5F0>,
     <Whitespace ' ' at 0x226921CC4C0>,
     <Keyword 'OVER' at 0x226921CC520>,
     <Whitespace ' ' at 0x226921CC580>,
     <IdentifierList '( PART...' at 0x226921CF660>,
     <Whitespace ' ' at 0x226921CB100>,
     <Keyword 'OVER' at 0x226921CB160>,
     <Whitespace ' ' at 0x226921CB1C0>,
     <IdentifierList '( PART...' at 0x226921CF6D0>,
     <Whitespace ' ' at 0x226921CBDC0>,
     <Keyword 'FROM' at 0x226921CBE20>,
     <Whitespace ' ' at 0x226921CBE80>,
     <Keyword 'table' at 0x226921CBEE0>]

where selected tokens from the list above consist of:

  * `<IdentifierList 'uid , ...' at 0x226921CF510>` (value: `uid , ROW_NUMBER()`):

        [<Keyword 'uid' at 0x2269219A280>,
         <Whitespace ' ' at 0x2269219A2E0>,
         <Punctuation ',' at 0x2269219A3A0>,
         <Whitespace ' ' at 0x2269219A340>,
         <Function 'ROW_NU...' at 0x226921BC890>]  # i.e., just "ROW_NUMBER()" without "OVER ( ... )"
    
  * `<IdentifierList '( PART...' at 0x226921CF5F0>` (value: `( PARTITION BY table.col_a ORDER BY table.col_b DESC ) AS row , full_name AS name , another_column AS acol , ROW_NUMBER()`):

        [<Identifier '( PART...' at 0x226921CF040>,  # this belongs to the previous "ROW_NUMBER() ..."
         <Whitespace ' ' at 0x226921A9C40>,
         <Punctuation ',' at 0x226921A9CA0>,
         <Whitespace ' ' at 0x226921A9D00>,
         <Identifier 'full_n...' at 0x226921BCDD0>,  # this should have been a separate Identifier "one level up"
         <Whitespace ' ' at 0x226921A9F40>,
         <Punctuation ',' at 0x226921A9FA0>,
         <Whitespace ' ' at 0x226921CC040>,
         <Identifier 'anothe...' at 0x226921BCF90>,  # this should have been a separate Identifier "one level up"
         <Whitespace ' ' at 0x226921CC280>,
         <Punctuation ',' at 0x226921CC2E0>,
         <Whitespace ' ' at 0x226921CC340>,
         <Function 'ROW_NU...' at 0x226921BC970>]    # again, just "ROW_NUMBER()" without "OVER ( ... )" + should have been "one level up"

  * `<IdentifierList '( PART...' at 0x226921CF660>` (value: `( PARTITION BY table.col_c ORDER BY table.col_d ASC ) AS row_lo , ROW_NUMBER()`)

        [<Identifier '( PART...' at 0x226921CF3C0>,  # this belongs to the previous "ROW_NUMBER() ..."
         <Whitespace ' ' at 0x226921CCE80>,
         <Punctuation ',' at 0x226921CCEE0>,
         <Whitespace ' ' at 0x226921CCF40>,
         <Function 'ROW_NU...' at 0x226921BCAC0>]    # again, just "ROW_NUMBER()" without "OVER ( ... )" + should have been "one level up"

  * `<IdentifierList '( PART...' at 0x226921CF6D0>` (value: `( PARTITION BY table.col_e ORDER BY table.col_f DESC ) AS row_hi , last_column AS lc`)

        [<Identifier '( PART...' at 0x226921CF4A0>,  # this belongs to the previous "ROW_NUMBER() ..."
         <Whitespace ' ' at 0x226921CBAC0>,
         <Punctuation ',' at 0x226921CBB20>,
         <Whitespace ' ' at 0x226921CBB80>,
         <Identifier 'last_c...' at 0x226921CF200>] # this should have been a separate Identifier "one level up"

Discussion:

Patch:
diff --git a/sqlparse/engine/grouping.py b/sqlparse/engine/grouping.py
index 9190797..926a3c1 100644
--- a/sqlparse/engine/grouping.py
+++ b/sqlparse/engine/grouping.py
@@ -235,6 +235,16 @@ def group_identifier(tlist):
         tidx, token = tlist.token_next_by(t=ttypes, idx=tidx)
 
 
+@recurse(sql.Over)
+def group_over(tlist):
+    tidx, token = tlist.token_next_by(m=sql.Over.M_OPEN)
+    while token:
+        nidx, next_ = tlist.token_next(tidx)
+        if imt(next_, i=sql.Parenthesis, t=T.Name):
+            tlist.group_tokens(sql.Over, tidx, nidx)
+        tidx, token = tlist.token_next_by(m=sql.Over.M_OPEN, idx=tidx)
+
+
 def group_arrays(tlist):
     sqlcls = sql.SquareBrackets, sql.Identifier, sql.Function
     ttypes = T.Name, T.String.Symbol
@@ -361,7 +371,12 @@ def group_functions(tlist):
     while token:
         nidx, next_ = tlist.token_next(tidx)
         if isinstance(next_, sql.Parenthesis):
-            tlist.group_tokens(sql.Function, tidx, nidx)
+            over_idx, over = tlist.token_next(nidx)
+            if over and isinstance(over, sql.Over):
+                eidx = over_idx
+            else:
+                eidx = nidx
+            tlist.group_tokens(sql.Function, tidx, eidx)
         tidx, token = tlist.token_next_by(t=T.Name, idx=tidx)
 
 
@@ -412,6 +427,7 @@ def group(stmt):
         group_for,
         group_begin,
 
+        group_over,
         group_functions,
         group_where,
         group_period,
diff --git a/sqlparse/sql.py b/sqlparse/sql.py
index 41606dd..05e1774 100644
--- a/sqlparse/sql.py
+++ b/sqlparse/sql.py
@@ -554,6 +554,11 @@ class Where(TokenList):
         'HAVING', 'RETURNING', 'INTO')
 
 
+class Over(TokenList):
+    """An OVER clause."""
+    M_OPEN = T.Keyword, 'OVER'
+
+
 class Having(TokenList):
     """A HAVING clause."""
     M_OPEN = T.Keyword, 'HAVING'
@@ -618,7 +623,7 @@ class Function(NameAliasMixin, TokenList):
 
     def get_parameters(self):
         """Return a list of parameters."""
-        parenthesis = self.tokens[-1]
+        parenthesis = self.token_next_by(i=Parenthesis)[1]
         result = []
         for token in parenthesis.tokens:
             if isinstance(token, IdentifierList):
@@ -628,6 +633,13 @@ class Function(NameAliasMixin, TokenList):
                 result.append(token)
         return result
 
+    def get_window(self):
+        """Return the window if it exists."""
+        over_clause = self.token_next_by(i=Over)
+        if not over_clause:
+            return None
+        return over_clause[1].tokens[-1]
+
 
 class Begin(TokenList):
     """A BEGIN/END block."""


Test patch:
diff --git a/tests/test_grouping.py b/tests/test_grouping.py
index e90243b..b39ff27 100644
--- a/tests/test_grouping.py
+++ b/tests/test_grouping.py
@@ -185,6 +185,20 @@ def test_grouping_identifier_function():
     assert isinstance(p.tokens[0], sql.Identifier)
     assert isinstance(p.tokens[0].tokens[0], sql.Operation)
     assert isinstance(p.tokens[0].tokens[0].tokens[0], sql.Function)
+    p = sqlparse.parse('foo(c1) over win1 as bar')[0]
+    assert isinstance(p.tokens[0], sql.Identifier)
+    assert isinstance(p.tokens[0].tokens[0], sql.Function)
+    assert len(p.tokens[0].tokens[0].tokens) == 4
+    assert isinstance(p.tokens[0].tokens[0].tokens[3], sql.Over)
+    assert isinstance(p.tokens[0].tokens[0].tokens[3].tokens[2],
+                      sql.Identifier)
+    p = sqlparse.parse('foo(c1) over (partition by c2 order by c3) as bar')[0]
+    assert isinstance(p.tokens[0], sql.Identifier)
+    assert isinstance(p.tokens[0].tokens[0], sql.Function)
+    assert len(p.tokens[0].tokens[0].tokens) == 4
+    assert isinstance(p.tokens[0].tokens[0].tokens[3], sql.Over)
+    assert isinstance(p.tokens[0].tokens[0].tokens[3].tokens[2],
+                      sql.Parenthesis)
 
 
 @pytest.mark.parametrize('s', ['foo+100', 'foo + 100', 'foo*100'])
@@ -378,6 +392,14 @@ def test_grouping_function():
     p = sqlparse.parse('foo(null, bar)')[0]
     assert isinstance(p.tokens[0], sql.Function)
     assert len(list(p.tokens[0].get_parameters())) == 2
+    p = sqlparse.parse('foo(5) over win1')[0]
+    assert isinstance(p.tokens[0], sql.Function)
+    assert len(list(p.tokens[0].get_parameters())) == 1
+    assert isinstance(p.tokens[0].get_window(), sql.Identifier)
+    p = sqlparse.parse('foo(5) over (PARTITION BY c1)')[0]
+    assert isinstance(p.tokens[0], sql.Function)
+    assert len(list(p.tokens[0].get_parameters())) == 1
+    assert isinstance(p.tokens[0].get_window(), sql.Parenthesis)
 
 
 def test_grouping_function_not_in():