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.

Sign up for GitHub

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

Jump to bottom

Add Decimal256 sqllogictests for SUM, MEDIAN and COUNT aggregate expressions #7889

Merged
merged 4 commits into from
Oct 21, 2023
Merged

Add Decimal256 sqllogictests for SUM, MEDIAN and COUNT aggregate expressions #7889

merged 4 commits into from
Oct 21, 2023

Conversation

viirya
Copy link
Member

@viirya viirya commented Oct 20, 2023

Which issue does this PR close?

Closes #7888.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Oct 20, 2023
comphead
comphead approved these changes Oct 20, 2023
View reviewed changes
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm
Thanks @viirya

One question Except for newly added Decimal256 support of AVG, MIN/MAX, SUM and MEDIAN already have Decimal256 support but lack of sqllogictests tests.

should we also add tests for AVG, MIN/MAX, COUNT?

@viirya
Copy link
Member Author

viirya commented Oct 20, 2023

should we also add tests for AVG, MIN/MAX, COUNT?

Yea, they are already added.

@viirya viirya changed the title Add Decimal256 sqllogictests for SUM and MEDIAN aggregate expressions Add Decimal256 sqllogictests for SUM, MEDIAN and COUNT aggregate expressions Oct 20, 2023
@viirya
Copy link
Member Author

viirya commented Oct 20, 2023

Oh, count isn't. I just added it.

@Dandandan
Copy link
Contributor

Thanks @viirya

@Dandandan Dandandan merged commit d6d3244 into apache:main Oct 21, 2023
23 checks passed
@viirya
Copy link
Member Author

viirya commented Oct 21, 2023

Thank you @Dandandan @alamb @comphead

@matthewgapp matthewgapp mentioned this pull request Jan 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@comphead comphead comphead approved these changes

@alamb alamb alamb approved these changes

@Dandandan Dandandan Dandandan approved these changes

Assignees
No one assigned
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Decimal256 sqllogictests for SUM, MEDIAN and COUNT aggregate expressions
4 participants
@viirya @Dandandan @alamb @comphead