Skip to content

Conversation

@gruuya
Copy link
Contributor

@gruuya gruuya commented Apr 17, 2024

Which issue does this PR close?

Closes #10113 .

Rationale for this change

What changes are included in this PR?

Coerce AVG accumulator to designated return type in case of columns with nulls.

Are these changes tested?

Yes, also a test is extended.

Are there any user-facing changes?

The query reported in the issue works now.

@github-actions github-actions bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Apr 17, 2024
12)--------------PlaceholderRowExec

query IIII
query IIII rowsort
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we fixed them on main too with #10122 and #10120

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.

nice catch @gruuya
I'm wondering what datatype it picked up before?

Copy link
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.

Thanks @gruuya

12)--------------PlaceholderRowExec

query IIII
query IIII rowsort
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we fixed them on main too with #10122 and #10120

@alamb alamb merged commit b2f5e39 into apache:main Apr 18, 2024
@gruuya gruuya deleted the avg-groups-accumulator-nulls branch April 29, 2024 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Average groups accumulator doesn't coerce to return type in presence of null values

3 participants