Skip to content

[native] Handle scan filter with NaN limits during plan conversion#23289

Merged
amitkdutta merged 1 commit intoprestodb:masterfrom
bikramSingh91:handle_NaN_filter
Jul 24, 2024
Merged

[native] Handle scan filter with NaN limits during plan conversion#23289
amitkdutta merged 1 commit intoprestodb:masterfrom
bikramSingh91:handle_NaN_filter

Conversation

@bikramSingh91
Copy link
Contributor

The filter implementation in Velox, which is utilized as a
pushed-down filter for the scan operator, does not accommodate NaN
values as limits. This update ensures that during the Presto to
Velox plan conversion, such filters are accurately transformed into
their Velox equivalents.

The filter implementation in Velox, which is utilized as a
pushed-down filter for the scan operator, does not accommodate NaN
values as limits. This update ensures that during the Presto to
Velox plan conversion, such filters are accurately transformed into
their Velox equivalents.
@bikramSingh91 bikramSingh91 requested a review from a team as a code owner July 24, 2024 18:20
Copy link
Contributor

@Yuhta Yuhta left a comment

Choose a reason for hiding this comment

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

LGTM, you need a committer stamp in order to merge though

Copy link
Contributor

@amitkdutta amitkdutta left a comment

Choose a reason for hiding this comment

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

@bikramSingh91
Copy link
Contributor Author

One of the CI failures is #22406 which is an unrelated flaky test.
The other one is also a flaky test: #22422

@aditi-pandit
Copy link
Contributor

One of the CI failures is #22406 which is an unrelated flaky test. The other one is also a flaky test: #22422

@bikramSingh91 : Started a re-run of the tests. Would be safer to have these complete once successfully.

@amitkdutta amitkdutta merged commit e63a57a into prestodb:master Jul 24, 2024
@tdcmeehan tdcmeehan mentioned this pull request Aug 23, 2024
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants