adriangb opened a new pull request, #25139: URL: https://github.com/apache/datafusion/pull/25139
## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/4610 ## Rationale for this change Run these statements in `datafusion-cli`: ```sql CREATE TABLE t(a INT, b INT) AS VALUES (1, 1), (2, 2); SELECT a FROM t GROUP BY a HAVING row_number() OVER () = 1; ``` On `main`, the second statement fails during physical planning. The error is the `Debug` text of the internal `Expr::WindowFunction`. It is one line of 526 characters. It does not give the clause, the function, or the column: ``` This feature is not implemented: Physical plan does not support logical expression WindowFunction(WindowFunction { fun: WindowUDF(WindowUDF { inner: RowNumber { signature: Signature { type_signature: Nullary, volatility: Immutable, parameter_names: None } } }), params: WindowFunctionParams { args: [], partition_by: [], order_by: [], window_frame: WindowFrame { units: Rows, start_bound: Preceding(UInt64(NULL)), end_bound: Following(UInt64(NULL)), is_causal: false }, filter: None, null_treatment: None, distinct: false } }) ``` The query from the issue gives the same type of error. That error is 1,981 characters long. With this PR, the statement fails during logical planning. The error gives the clause and the window function: ``` Error during planning: Window function calls are not allowed in HAVING: 'row_number() ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING' ``` The same applies to `WHERE`: ```sql SELECT a FROM t WHERE row_number() OVER () = 1; ``` ``` Error during planning: Window function calls are not allowed in WHERE: 'row_number() ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING' ``` The error has a `Diagnostic`. The diagnostic points to the window function in the SQL text. Its help message tells the user to use `QUALIFY` or an inner query. These queries are not valid SQL. DataFusion evaluates a filter before it computes window functions. PostgreSQL rejects the same queries with `window functions are not allowed in HAVING`. These queries continue to work: ```sql -- window function in the SELECT list SELECT a, count(*) AS cnt, sum(count(*)) OVER () AS total FROM t GROUP BY a; -- window function in QUALIFY SELECT a, count(*) AS cnt, sum(count(*)) OVER () AS total FROM t GROUP BY a QUALIFY sum(count(*)) OVER () >= 2; -- window function in an inner query SELECT * FROM (SELECT a, count(*) AS cnt, sum(count(*)) OVER () AS total FROM t GROUP BY a) s WHERE total >= 2; -- window function in a subquery of a WHERE predicate SELECT a FROM t WHERE a IN (SELECT a FROM t QUALIFY row_number() OVER (ORDER BY a) = 1); ``` ## What changes are included in this PR? - Add `datafusion_expr::utils::check_no_window_functions(expr, clause)`. It returns a planning error if `expr` contains a window function. The error has a `Diagnostic` with a span and a help message. The function does not look into subqueries. It has the same shape as `check_aggregate_and_window_nesting` from #23813. - `Filter::try_new` calls the function with the clause name `filter predicates`. This applies to the `DataFrame` and `LogicalPlanBuilder` paths. `Filter::new` does not do the check. Its documentation says this. - The SQL planner calls the function for `WHERE` and for `HAVING`. The `HAVING` check runs after alias resolution. Thus `HAVING total >= 10`, where `total` is an alias of a window function, is also rejected. ## What is the testing strategy for this PR? - `datafusion/expr/src/utils.rs`: unit test for `check_no_window_functions`. It checks the accepted expressions, the error message, and the diagnostic. - `datafusion/expr/src/logical_plan/builder.rs`: `LogicalPlanBuilder::filter` and `having` reject a window predicate. `window(...).filter(...)` continues to work. - `datafusion/sql/tests/sql_integration.rs`: the `WHERE` and `HAVING` messages, the alias case, and the `QUALIFY` and subquery forms. - `datafusion/sql/tests/cases/diagnostic.rs`: the diagnostic span points to the window function argument in the SQL text. - `datafusion/sqllogictest/test_files/window.slt`: the reproducer from the issue, the `WHERE` variant, and the four queries that continue to work. ## Are there any user-facing changes? Yes. A query with a window function in `WHERE` or `HAVING` now fails during logical planning. The error is `Error during planning: Window function calls are not allowed in {WHERE|HAVING}: '<window call>'`. Before, the query failed during physical planning with `This feature is not implemented: Physical plan does not support logical expression WindowFunction(...)`. `Filter::try_new`, `LogicalPlanBuilder::filter` and `LogicalPlanBuilder::having` return the same type of error for such a predicate. Before, they returned a plan that could not be executed. New public API: `datafusion_expr::utils::check_no_window_functions`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
