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.

Sign up for GitHub

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

Jump to bottom

Deletes: implement negate for query condition. #3299

Merged
merged 3 commits into from Jul 1, 2022
Merged

Deletes: implement negate for query condition. #3299

merged 3 commits into from Jul 1, 2022

Conversation

KiterLuc
Copy link
Contributor

@KiterLuc KiterLuc commented Jun 20, 2022 •
edited

This adds the ability to negate a query condition for deletes before
serializing to disk.

SC-18803

TYPE: IMPROVEMENT
DESC: Deletes: implement negate for query condition.

@KiterLuc KiterLuc requested a review from ihnorton June 20, 2022 06:59
@KiterLuc KiterLuc requested a review from ihnorton June 30, 2022 14:00
ihnorton
ihnorton approved these changes Jul 1, 2022
View reviewed changes
Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

Minor nit/question inline, and also double-checking: was there a reason to put the constructor implementations in the header?

Otherwise LGTM, the constructor-based implementation looks nice 👍

tiledb/sm/query/ast/query_ast.h Show resolved Hide resolved
@ypatia ypatia self-requested a review July 1, 2022 07:38
ypatia
ypatia requested changes Jul 1, 2022
View reviewed changes
tiledb/sm/query/ast/query_ast.h Outdated Show resolved Hide resolved
@KiterLuc
Deletes: implement negate for query condition.
11aa448 
This adds the ability to negate a query condition for deletes before
serializing to disk.

---
TYPE: IMPROVEMENT
DESC: Deletes: implement negate for query condition.
@KiterLuc
Addressing feedback from @ihnorton.
1f17f32
@KiterLuc KiterLuc force-pushed the lr/qc-negate/sc18803 branch 3 times, most recently from 3494689 to ac475d1 Compare July 1, 2022 08:15
ypatia
ypatia approved these changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@ypatia ypatia left a comment

Choose a reason for hiding this comment

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

Thanks, it now looks great

tiledb/sm/enums/query_condition_combination_op.h Show resolved Hide resolved
tiledb/sm/query/ast/query_ast.h Show resolved Hide resolved
@KiterLuc KiterLuc merged commit e2c11a6 into dev Jul 1, 2022
@KiterLuc KiterLuc deleted the lr/qc-negate/sc18803 branch July 1, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ypatia ypatia ypatia approved these changes

@ihnorton ihnorton ihnorton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@KiterLuc @ypatia @ihnorton