Skip to content

Support Decimal256 on AVG aggregate expression#7853

Merged
Dandandan merged 5 commits intoapache:mainfrom
viirya:decimal256
Oct 19, 2023
Merged

Support Decimal256 on AVG aggregate expression#7853
Dandandan merged 5 commits intoapache:mainfrom
viirya:decimal256

Conversation

@viirya
Copy link
Copy Markdown
Member

@viirya viirya commented Oct 18, 2023

Which issue does this PR close?

Closes #7852.

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 physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Oct 18, 2023
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks very nice to me -- thank you @viirya

impl<T: DecimalType + ArrowNumericType> Debug for DecimalAvgAccumulator<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("DecimalAvgAccumulator")
.field("sum", &self.sum)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this needed because #derive doesn't know how to handle Option<T::Native>?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yea. #derive doesn't work.

@Dandandan Dandandan merged commit 634ed28 into apache:main Oct 19, 2023
@viirya
Copy link
Copy Markdown
Member Author

viirya commented Oct 19, 2023

Thank you @Dandandan @alamb

jayzhan211 pushed a commit to jayzhan211/datafusion that referenced this pull request Oct 21, 2023
* More

* More

* More

* More

* Fix clippy
@andygrove andygrove added the enhancement New feature or request label Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Decimal256 on AVG aggregate expression

4 participants