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

Add Filter::mergeWith() method #119

Closed
wants to merge 1 commit into from

Conversation

mbasmanova
Copy link
Contributor

@mbasmanova mbasmanova commented Aug 26, 2021

Add a way to combine two filters using 'AND' logic.

This commit implements the new method for all filters except BytesRange
and MultiRange. The support for these filters will come in a follow-up PR.

@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 Aug 26, 2021
@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

velox/type/Filter.cpp Show resolved Hide resolved
case FilterKind::kAlwaysFalse:
case FilterKind::kIsNull:
return std::make_unique<AlwaysFalse>();
case FilterKind::kIsNotNull:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe this can be moved up to be next to kAlwaysTrue, so the fall through in both is same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Moved.

velox/type/Filter.cpp Show resolved Hide resolved
@mbasmanova mbasmanova force-pushed the merge-filters branch 2 times, most recently from 28d5a95 to b00b71f Compare August 26, 2021 17:13
@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mbasmanova mbasmanova force-pushed the merge-filters branch 2 times, most recently from 999e61a to ff93f0c Compare August 27, 2021 04:53
@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Add a way to combine two filters using 'AND' logic.

This commit implements the new method for all filters except BytesRange,
BigintMultiRange and MultiRange. The support for these filters will come in a
follow-up PR.
@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mbasmanova merged this pull request in 95911db.

atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 1, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:

- Handling case of merging multiple byte ranges which results in MultiRange as the output
- Merging MultiRange with MultiRange
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 1, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:

- Handling case of merging multiple byte ranges which results in MultiRange as the output
- Merging MultiRange with MultiRange
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 2, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:

- Handling case of merging multiple byte ranges which results in MultiRange as the output
- Merging MultiRange with MultiRange
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 5, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 5, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 5, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 5, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 5, 2021
This commit is a follow up of facebookincubator#119
and introduces mergeWith implmentation for BytesRange and BytesValues.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 6, 2021
This commit is a follow up of facebookincubator#119.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 6, 2021
This commit is a follow up of facebookincubator#119.

TODO:
- (New PR) Handling case of merging multiple byte ranges which results in MultiRange as the output
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 6, 2021
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 6, 2021
atanu1991 added a commit to atanu1991/velox-1 that referenced this pull request Oct 7, 2021
)

Summary:
Implement BytesRange::mergeWith and BytesValues::mergeWith
This commit is a follow up of facebookincubator#119.

Pull Request resolved: facebookincubator#297

Reviewed By: mbasmanova

Differential Revision: D31444325

Pulled By: atanu1991

fbshipit-source-id: 3e8d8bb2645455fcaf24ebd18d6c122613221958
facebook-github-bot pushed a commit that referenced this pull request Oct 7, 2021
Summary:
Implement BytesRange::mergeWith and BytesValues::mergeWith
This commit is a follow up of #119.

Pull Request resolved: #297

Reviewed By: mbasmanova

Differential Revision: D31444325

Pulled By: atanu1991

fbshipit-source-id: 7751c10d71ef4f70aa3ac30ed0ff5f85af346d19
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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants