Skip to content

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
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
@viirya
Copy link
Member Author

viirya commented Oct 21, 2023

Thank you @Dandandan @alamb @comphead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Decimal256 sqllogictests for SUM, MEDIAN and COUNT aggregate expressions

4 participants