Skip to content

Issue #12600: Streaming Positive LEAD #12685

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

Merged
merged 9 commits into from
Jun 29, 2024
Merged

Conversation

hawkfish
Copy link
Contributor

Implement streaming LEAD by maintaining a delay buffer.
The included benchmark shows a 7x performance gain.

fixes: #12600
fixes: duckdblabs/duckdb-internal#2342

Richard Wesley added 2 commits June 24, 2024 15:31
Implement streaming LEAD by maintaining a delay buffer.
The included benchmark shows a 7x performance gain.

fixes: duckdb#12600
fixes: duckdblabs/duckdb-internal#2342
Fix tidy issue with abs.
Fix another tidy issue with abs...
@hawkfish hawkfish requested a review from Mytherin June 25, 2024 04:00
@duckdb-draftbot duckdb-draftbot marked this pull request as draft June 25, 2024 04:00
@hawkfish hawkfish marked this pull request as ready for review June 25, 2024 04:00
Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great! Could we add some tests where the LAG/LEAD are greater than STANDARD_VECTOR_SIZE? Other than that it's good to go from my side.

@hawkfish hawkfish requested a review from lnkuiper June 25, 2024 13:49
@duckdb-draftbot duckdb-draftbot marked this pull request as draft June 25, 2024 13:49
@hawkfish hawkfish marked this pull request as ready for review June 25, 2024 13:49
@cpcloud
Copy link
Contributor

cpcloud commented Jun 26, 2024

Just tried this out, and it is indeed a great improvement!

Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Looks great, great results! Some comments:

Richard Wesley added 3 commits June 27, 2024 12:20
Fix overrun error and make buffering independent of the standard vector size.
PR Feedback:
* Make buffering independent of the standard vector size.
* Don't allow resize during Append because it should already be the correct size.
PR Feedback:
* Test with vector_size=2
@duckdb-draftbot duckdb-draftbot marked this pull request as draft June 27, 2024 22:53
@hawkfish hawkfish requested a review from Mytherin June 27, 2024 22:53
@Mytherin Mytherin marked this pull request as ready for review June 28, 2024 10:46
Richard Wesley added 2 commits June 28, 2024 09:28
PR Feedback:
* Avoid copying

fixes: duckdb#12600
fixes: duckdblabs/duckdb-internal#2342
@duckdb-draftbot duckdb-draftbot marked this pull request as draft June 29, 2024 16:15
@hawkfish hawkfish marked this pull request as ready for review June 29, 2024 16:15
@Mytherin Mytherin merged commit 5ebd8ca into duckdb:main Jun 29, 2024
40 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@hawkfish hawkfish deleted the streaming-lead branch June 29, 2024 21:50
github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Jul 4, 2024
Merge pull request duckdb/duckdb#12685 from hawkfish/streaming-lead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DuckDB lag seems to be much slower than Polars
4 participants