-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add new-streaming first/last aggregations #20716
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20716 +/- ##
==========================================
- Coverage 79.92% 79.83% -0.10%
==========================================
Files 1559 1560 +1
Lines 221170 221448 +278
Branches 2530 2530
==========================================
+ Hits 176764 176786 +22
- Misses 43824 44080 +256
Partials 582 582 ☔ View full report in Codecov by Sentry. |
@@ -66,7 +66,7 @@ def test_streaming_group_by_types() -> None: | |||
pl.col("bool").last().alias("bool_last"), | |||
pl.col("bool").mean().alias("bool_mean"), | |||
pl.col("bool").sum().alias("bool_sum"), | |||
pl.col("date").sum().alias("date_sum"), | |||
# pl.col("date").sum().alias("date_sum"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ritchie46 Yes, the summing dates doesn't really make any sense, and it's not supported on the eager engine either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, shouldn't we error during IR resolving then?
Only contains an
AnyValue
-based general implementation right now, will add more performant options later.