Skip to content
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

Look into applying DECIMAL128 sum aggregation overflow checking for window aggregations #4723

Open
jlowe opened this issue Feb 8, 2022 · 0 comments
Labels
P1 Nice to have for release performance A performance related task/issue task Work required that improves the product but is not user facing

Comments

@jlowe
Copy link
Member

jlowe commented Feb 8, 2022

#4688 adds a new technique for detecting overflows in DECIMAL128 sum aggregations, and we should investigate whether it makes sense to do something similar for window sum aggregations. Unlike normal sum aggregations, window sum aggregations are on sorted data ,so the use of sort-based aggregations shouldn't be a performance concern when we hint libcudf about the sorted input. However we are still performing special calculations for overflow checking that is different than what we're doing for sum aggregations, and it may make sense to commonize this code.

@jlowe jlowe added ? - Needs Triage Need team to review and classify performance A performance related task/issue task Work required that improves the product but is not user facing labels Feb 8, 2022
@sameerz sameerz removed the ? - Needs Triage Need team to review and classify label Feb 8, 2022
@mattahrens mattahrens added the P1 Nice to have for release label May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Nice to have for release performance A performance related task/issue task Work required that improves the product but is not user facing
Projects
None yet
Development

No branches or pull requests

3 participants