alamb commented on code in PR #15033:
URL: https://github.com/apache/datafusion/pull/15033#discussion_r1996239262


##########
datafusion/core/tests/sql/sql_api.rs:
##########
@@ -19,6 +19,23 @@ use datafusion::prelude::*;
 
 use tempfile::TempDir;
 
+#[tokio::test]
+async fn test_window_function() {
+    let ctx = SessionContext::new();
+    let df = ctx
+        .sql(
+            r#"SELECT
+        t1.v1,
+        SUM(t1.v1) OVER w + 1
+        FROM
+        generate_series(1, 10000) AS t1(v1)
+        WINDOW

Review Comment:
   I think we should at least have a .slt test that shows this query running 
and producing the same result as postgres, perhaps with a smaller number of 
series:
   
   ```sql
   postgres=# SELECT
     t1.v1,
     SUM(t1.v1) OVER w
   FROM
     generate_series(1, 5) AS t1(v1)
   WINDOW
     w AS (ORDER BY t1.v1 ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW);
    v1 | sum
   ----+-----
     1 |   1
     2 |   3
     3 |   6
     4 |  10
     5 |  15
   (5 rows)
   ```



##########
datafusion/sql/src/select.rs:
##########
@@ -887,29 +888,42 @@ fn match_window_definitions(
     named_windows: &[NamedWindowDefinition],
 ) -> Result<()> {
     for proj in projection.iter_mut() {
-        if let SelectItem::ExprWithAlias {
-            expr: SQLExpr::Function(f),
-            alias: _,
-        }
-        | SelectItem::UnnamedExpr(SQLExpr::Function(f)) = proj
+        if let SelectItem::ExprWithAlias { expr, alias: _ }
+        | SelectItem::UnnamedExpr(expr) = proj
         {
-            for NamedWindowDefinition(window_ident, window_expr) in 
named_windows.iter() {
-                if let Some(WindowType::NamedWindow(ident)) = &f.over {
-                    if ident.eq(window_ident) {
-                        f.over = Some(match window_expr {
-                            NamedWindowExpr::NamedWindow(ident) => {
-                                WindowType::NamedWindow(ident.clone())
-                            }
-                            NamedWindowExpr::WindowSpec(spec) => {
-                                WindowType::WindowSpec(spec.clone())
+            let mut err = None;
+            visit_expressions_mut(expr, |expr| {

Review Comment:
   I am sorry @chenkovsky  and @2010YOUY01 
   
   I don't know what 
   ```sql
   SELECT
     t1.v1,
     SUM(t1.v1) OVER w + 1
   FROM
     generate_series(1, 10) AS t1(v1)
   WINDOW
     w AS (ORDER BY t1.v1);
   
   ```
   
   Is supposed to be computing (what does adding one to a window definition 
like `w +1` represent?)
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to