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 NaN handling for min/max aggreates pushed down to scan #10583

Closed

Conversation

bikramSingh91
Copy link
Contributor

Summary: This fixes min/max aggregates to handle NaN values correctly when they are pushed down to the scan operator. Specifically, the change ensures that NaN values are considered greater than infinity.

Reviewed By: zacw7

Differential Revision: D60297934

Summary: This fixes min/max aggregates to handle NaN values correctly when they are pushed down to the scan operator. Specifically, the change ensures that NaN values are considered greater than infinity.

Reviewed By: zacw7

Differential Revision: D60297934
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 26, 2024
Copy link

netlify bot commented Jul 26, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 10e80fb
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66a3e1ecf6d15000081bcdf7

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60297934

Copy link
Contributor

@zacw7 zacw7 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in de21ce6.

Copy link

Conbench analyzed the 1 benchmark run on commit de21ce6c.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

weiting-chen pushed a commit to weiting-chen/velox that referenced this pull request Nov 23, 2024
…ncubator#10583)

Summary:
Pull Request resolved: facebookincubator#10583

This fixes min/max aggregates to handle NaN values correctly when they are pushed down to the scan operator. Specifically, the change ensures that NaN values are considered greater than infinity.

Reviewed By: zacw7

Differential Revision: D60297934

fbshipit-source-id: 3398ba24d6fc70bbd0d8583d2949391b0824099c
weiting-chen added a commit to oap-project/velox that referenced this pull request Nov 23, 2024
…ebookincubator#11298 (#505)

* Fix NaN handling for min/max aggreates pushed down to scan (facebookincubator#10583)

Summary:
Pull Request resolved: facebookincubator#10583

This fixes min/max aggregates to handle NaN values correctly when they are pushed down to the scan operator. Specifically, the change ensures that NaN values are considered greater than infinity.

Reviewed By: zacw7

Differential Revision: D60297934

fbshipit-source-id: 3398ba24d6fc70bbd0d8583d2949391b0824099c

* Schema evolution support for reader value hooks (facebookincubator#10755)

Summary:
X-link: facebookincubator/nimble#72

Pull Request resolved: facebookincubator#10755

Currently reader value hook is not considering schema evolution at all, this change fix that.

Reviewed By: kevinwilfong

Differential Revision: D61229494

fbshipit-source-id: 729bb90611fb3164282b524376eda20985a30194

* Disable aggregate pushdown for decimal type (facebookincubator#11298)

Summary:
Currently, `int64_t` enables push-down for decimal type. This PR disables aggregate pushdown for decimal type regardless of c++ type.

Fixes facebookincubator#11290

Pull Request resolved: facebookincubator#11298

Reviewed By: Yuhta

Differential Revision: D65834210

Pulled By: kevinwilfong

fbshipit-source-id: 422f7eda8f4184c6fa83055e7cf430ff5053d387

---------

Co-authored-by: Bikramjeet Vig <[email protected]>
Co-authored-by: Jimmy Lu <[email protected]>
Co-authored-by: NEUpanning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants