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

fix(sql): average function overflow for LONG column type #1824

Merged
merged 14 commits into from
Jan 26, 2022

Conversation

eugenels
Copy link
Contributor

@eugenels eugenels commented Jan 24, 2022

This PR adds __int128 backed implementations for sum/avg methods to Rosti.
For now they are used by AvgLongVectorAggregateFunction only.

Fixes #1803

@eugenels eugenels marked this pull request as draft January 24, 2022 14:07
@eugenels eugenels self-assigned this Jan 25, 2022
@eugenels eugenels marked this pull request as ready for review January 25, 2022 10:14
Copy link
Collaborator

@ideoma ideoma left a comment

Choose a reason for hiding this comment

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

Do you mind fixing AvgIntVectorAggregateFunction too?

@eugenels
Copy link
Contributor Author

eugenels commented Jan 26, 2022

Do you mind fixing AvgIntVectorAggregateFunction too?

AvgInt implementation accumulates sum into long variable.

@ideoma
Copy link
Collaborator

ideoma commented Jan 26, 2022

[PR Coverage check]

😍 pass : 7 / 8 (87.50%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/groupby/vect/AvgLongVectorAggregateFunction.java 7 8 87.50%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Negative LONG average when all values positive
3 participants