You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "timsaucer (via GitHub)" <gi...@apache.org> on 2024/09/12 12:08:52 UTC
[PR] Feature/aggregates as windows [datafusion-python]
timsaucer opened a new pull request, #871:
URL: https://github.com/apache/datafusion-python/pull/871
*Note* Marking as draft until https://github.com/apache/datafusion-python/pull/867 merges, after which I will rebase and mark read.
# Which issue does this PR close?
Closes #865
# Rationale for this change
This function allows the user to turn any aggregate function into a window function. Previously using aggregates as windows was only expressed through `functions.window()` which took the aggregate's name. This only supports built in window functions or functions that have been registered with the session context. While this worked, it's a bit of an anti-pattern for people who don't need to register functions into the session context.
# What changes are included in this PR?
Adds `.over()` to an expression which indicates it is to be used as a window function.
# Are there any user-facing changes?
During testing I noticed that the default window frames for `f.window()` are set to unbounded preceeding to unbounded following. With `.over()` it uses the default frames, so if ordering is set it goes from unbounded preceeding to current row. This *does* match user expectation.
--
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] Feature/aggregates as windows [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #871:
URL: https://github.com/apache/datafusion-python/pull/871#issuecomment-2359160523
It does! Updated the description
--
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] Feature/aggregates as windows [datafusion-python]
Posted by "Michael-J-Ward (via GitHub)" <gi...@apache.org>.
Michael-J-Ward commented on PR #871:
URL: https://github.com/apache/datafusion-python/pull/871#issuecomment-2359138313
> During testing I noticed that the default window frames for f.window() are set to unbounded preceeding to unbounded following. With .over() it uses the default frames, so if ordering is set it goes from unbounded preceeding to current row. This does match user expectation.
So does this PR also close https://github.com/apache/datafusion-python/issues/688?
--
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] Feature/aggregates as windows [datafusion-python]
Posted by "Michael-J-Ward (via GitHub)" <gi...@apache.org>.
Michael-J-Ward merged PR #871:
URL: https://github.com/apache/datafusion-python/pull/871
--
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] Feature/aggregates as windows [datafusion-python]
Posted by "Michael-J-Ward (via GitHub)" <gi...@apache.org>.
Michael-J-Ward commented on PR #871:
URL: https://github.com/apache/datafusion-python/pull/871#issuecomment-2359195202
I had just verified with a test - if this test is **not** duplicative then maybe we should add it.
```python
def test_window_frame_defaults_match_postgres(ctx):
# ref: https://github.com/apache/datafusion-python/issues/688
# create a RecordBatch and a new DataFrame from it
batch = pa.RecordBatch.from_arrays(
[pa.array([1.0, 10.0, 20.0])],
names=["a"],
)
df = ctx.create_dataframe([[batch]])
window_frame = WindowFrame("rows", None, None)
col_a = column("a")
# Using `f.window` with or without an unbounded window_frame produces the same results
no_frame = f.window("avg", [col_a]).alias('no_frame')
with_frame = f.window("avg", [col_a], window_frame=window_frame).alias('with_frame')
df_1 = df.select(col_a, no_frame, with_frame)
expected = """DataFrame()
+------+--------------------+--------------------+
| a | no_frame | with_frame |
+------+--------------------+--------------------+
| 1.0 | 10.333333333333334 | 10.333333333333334 |
| 10.0 | 10.333333333333334 | 10.333333333333334 |
| 20.0 | 10.333333333333334 | 10.333333333333334 |
+------+--------------------+--------------------+
""".strip()
assert str(df_1) == expected
# When `order_by` is set, the default window should be `unbounded preceding` to `current row`
no_order = f.avg(col_a).over(Window()).alias('over_no_order')
with_order = f.avg(col_a).over(Window(order_by=[col_a])).alias('over_with_order')
df_2 = df.select(col_a, no_order, with_order)
expected = """DataFrame()
+------+--------------------+--------------------+
| a | over_no_order | over_with_order |
+------+--------------------+--------------------+
| 1.0 | 10.333333333333334 | 1.0 |
| 10.0 | 10.333333333333334 | 5.5 |
| 20.0 | 10.333333333333334 | 10.333333333333334 |
+------+--------------------+--------------------+
""".strip()
assert str(df_2) == expected
```
--
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] Feature/aggregates as windows [datafusion-python]
Posted by "Michael-J-Ward (via GitHub)" <gi...@apache.org>.
Michael-J-Ward commented on code in PR #871:
URL: https://github.com/apache/datafusion-python/pull/871#discussion_r1765581243
##########
src/expr.rs:
##########
@@ -558,6 +561,45 @@ impl PyExpr {
pub fn window_frame(&self, window_frame: PyWindowFrame) -> PyExprFuncBuilder {
self.expr.clone().window_frame(window_frame.into()).into()
}
+
+ #[pyo3(signature = (partition_by=None, window_frame=None, order_by=None, null_treatment=None))]
+ pub fn over(
+ &self,
+ partition_by: Option<Vec<PyExpr>>,
+ window_frame: Option<PyWindowFrame>,
+ order_by: Option<Vec<PySortExpr>>,
+ null_treatment: Option<NullTreatment>,
+ ) -> PyResult<PyExpr> {
+ match &self.expr {
+ Expr::AggregateFunction(agg_fn) => {
+ let window_fn = Expr::WindowFunction(WindowFunction::new(
+ WindowFunctionDefinition::AggregateUDF(agg_fn.func.clone()),
+ agg_fn.args.clone(),
+ ));
+
+ add_builder_fns_to_window(
+ window_fn,
+ partition_by,
+ window_frame,
+ order_by,
+ null_treatment,
+ )
+ }
+ Expr::WindowFunction(_) => add_builder_fns_to_window(
+ self.expr.clone(),
+ partition_by,
+ window_frame,
+ order_by,
+ null_treatment,
+ ),
+ _ => Err(
+ DataFusionError::ExecutionError(datafusion::error::DataFusionError::Plan(
+ "Using `over` requires an aggregate function.".to_string(),
Review Comment:
```suggestion
format!("Using {} with `over` is not allowed. Must use an aggregate or window function.", self.expr.variant_name())
```
--
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