You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "haohuaijin (via GitHub)" <gi...@apache.org> on 2025/07/27 15:58:19 UTC
[PR] feat: implement QUALIFY clause [datafusion]
haohuaijin opened a new pull request, #16933:
URL: https://github.com/apache/datafusion/pull/16933
## Which issue does this PR close?
Closes #15485
## Rationale for this change
see https://github.com/apache/datafusion/issues/15485, support `qualify` can filter windows function's result without use subquery
before
```sql
with ranked as (
select row_number() over (PARTITION BY region) as rk from t
) select * from ranked where rk > 1;
```
after
```sql
select row_number() over (PARTITION BY region) as rk from t qualify rk > 1;
```
## What changes are included in this PR?
support the `qualify` clause like what we do for `having` clause
## Are these changes tested?
yes, add integration test and sqllogictest test
## Are there any user-facing changes?
It is easier to filter the windows function's result
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3192722107
Thanks again @haohuaijin
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "Vedin (via GitHub)" <gi...@apache.org>.
Vedin commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3184394834
Hi @haohuaijin, I accidentally worked on the same feature. The approach is the same. So, I don't want anyhow conflict with your contribution and will just wait until your PR is merged. I just want to highlight 2 cases that probably are not currently covered in your PR.
1. Aggregate functions with `QUALIFY`
```sql
CREATE TABLE qt (i INT, p VARCHAR, o INT) AS VALUES
(1, 'A', 1),
(2, 'A', 2),
(3, 'B', 1),
(4, 'B', 2);
SELECT p, SUM(o) AS s
FROM qt
GROUP BY p
QUALIFY RANK() OVER (ORDER BY s DESC) = 1
ORDER BY p;
```
2. Constant filter + QUALIFY
```sql
CREATE TABLE web_base_events_this_run (
domain_sessionid VARCHAR,
app_id VARCHAR,
page_view_id VARCHAR,
derived_tstamp TIMESTAMP,
dvce_created_tstamp TIMESTAMP,
event_id VARCHAR
) AS SELECT * FROM VALUES
('ds1', 'appA', NULL, '2025-01-01 10:00:00'::timestamp, '2025-01-01 10:05:00'::timestamp, 'e1'),
('ds1', 'appA', NULL, '2025-01-01 11:00:00'::timestamp, '2025-01-01 11:00:00'::timestamp, 'e2'),
('ds1', 'appA', 'pv', '2025-01-01 12:00:00'::timestamp, '2025-01-01 12:00:00'::timestamp, 'e3'),
('ds2', 'appB', NULL, '2025-01-01 09:00:00'::timestamp, '2025-01-01 09:10:00'::timestamp, 'e4'),
('ds2', 'appB', NULL, '2025-01-01 09:05:00'::timestamp, '2025-01-01 09:09:00'::timestamp, 'e5');
SELECT domain_sessionid, app_id
FROM web_base_events_this_run
WHERE page_view_id IS NULL
QUALIFY ROW_NUMBER() OVER (
PARTITION BY domain_sessionid
ORDER BY derived_tstamp, dvce_created_tstamp, event_id
) = 1
ORDER BY domain_sessionid;
```
I covered the first one by adding `qualify` expressions to `aggr_expr_haystack`:
```rust
let aggr_expr_haystack = select_exprs
.iter()
.chain(having_expr_opt.iter())
.chain(qualify_expr_opt_pre_aggr.iter());
```
The second one required me to change the logic in `common_subexpr_eliminate.rs`. You can check it out here (https://github.com/Embucket/datafusion/pull/34/). Maybe you'll come up with a better solution.
Hope this helps.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3189170759
> @alamb @jayzhan211 , could you please take a look at this PR? If everything looks good, perhaps we can merge this as the initial version of qualify, and then @Vedin can follow up with the two additional cases in a separate PR.
I think this sounds like a good plan. Once we merge this PR we can then file some tickets to track the other features.
Thanks @haohuaijin and @Vedin
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3192721835
> Hi @Vedin , thanks for bringing up these two use cases for `qualify`. I wasn’t aware of them before. However, I’ve been quite busy lately and won’t be able to include them in this PR.
>
> @alamb @jayzhan211 , could you please take a look at this PR? If everything looks good, perhaps we can merge this as the initial version of qualify, and then @Vedin can follow up with the two additional cases in a separate PR.
- i filed https://github.com/apache/datafusion/issues/17210 to track these improvements
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "haohuaijin (via GitHub)" <gi...@apache.org>.
haohuaijin commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3190663166
Thanks @alamb @jonahgao for reviews, i add the document like `HAVING` clause.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #16933:
URL: https://github.com/apache/datafusion/pull/16933
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#discussion_r2279861071
##########
docs/source/user-guide/sql/select.md:
##########
@@ -261,6 +262,14 @@ Example:
SELECT a, b, MAX(c) FROM table GROUP BY a, b HAVING MAX(c) > 10
```
+## QUALIFY clause
+
Review Comment:
It would be nice if we could provide a little context here (like explaining that the QUALIFY statement can refer to the output of window functions)
However, I think this is consistent with the sparse docs for HAVING
Maybe we can make a follow on PR to improve the documentation with a sentence or two for each clause
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#discussion_r2277174123
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4186,6 +4187,47 @@ fn test_select_distinct_order_by() {
);
}
+#[test]
+fn test_select_qualify_basic() {
+ let sql = "SELECT person.id, ROW_NUMBER() OVER (PARTITION BY person.age ORDER BY person.id) as rn FROM person QUALIFY rn = 1";
+ let plan = logical_plan(sql).unwrap();
+ assert_snapshot!(
+ plan,
+ @r#"
+Projection: person.id, row_number() PARTITION BY [person.age] ORDER BY [person.id ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW AS rn
+ Filter: row_number() PARTITION BY [person.age] ORDER BY [person.id ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW = Int64(1)
+ WindowAggr: windowExpr=[[row_number() PARTITION BY [person.age] ORDER BY [person.id ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW]]
+ TableScan: person
+"#
+ );
+}
+
+#[test]
+fn test_select_qualify_without_window_function() {
+ let sql = "SELECT person.id FROM person QUALIFY person.id > 1";
+ let err = logical_plan(sql).unwrap_err();
+ assert_eq!(
+ err.strip_backtrace(),
+ "Error during planning: QUALIFY clause requires window functions in the SELECT list or QUALIFY clause"
Review Comment:
I verified that this is consistent with the DuckDB behavior:
```sql
D SELECT person.id FROM person QUALIFY person.id > 1;
Binder Error:
at least one window function must appear in the SELECT column or QUALIFY clause
```
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: implement QUALIFY clause [datafusion]
Posted by "haohuaijin (via GitHub)" <gi...@apache.org>.
haohuaijin commented on PR #16933:
URL: https://github.com/apache/datafusion/pull/16933#issuecomment-3186329138
Hi @Vedin , thanks for bringing up these two use cases for `qualify`. I wasn’t aware of them before. However, I’ve been quite busy lately and won’t be able to include them in this PR.
@alamb , could you please take a look at this PR? If everything looks good, perhaps we can merge this as the initial version of qualify, and then @Vedin can follow up with the two additional cases in a separate PR.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org