-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Port ArrayRemove, ArrayRemoveN, ArrayRemoveAll to functions-array subcrate
#9656
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
Conversation
…ons-array subcrate
|
Current |
| array_except( | ||
| make_array(vec![lit(1), lit(2), lit(3)]), | ||
| make_array(vec![lit(1), lit(2)]), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you accidentally remove it. Also, add remove's too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, addressed.
It seems like an issue from github |
| /// [[1, 2, 3], [2, 3, 4], [2, 3, 4]], [[1, 2, 3], [2, 3, 4], [3, 4, 5]], 1, false => [true, false, false] | ||
| /// ) | ||
| /// ``` | ||
| pub(crate) fn compare_element_to_list( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three duplicated code including this one, can you remove other two?
jayzhan211
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@erenavsarogullari Let me merge this first, those duplications can be removed later |
|
@jayzhan211 Thanks for the merge. I will address duplication by separate PR. |
Which issue does this PR close?
Closes #9635.
What changes are included in this PR?
This PR aims to do following changes in terms of Epic #9285:
ArrayRemove,ArrayRemoveN,ArrayRemoveAllare ported tofunctions-arraysubcrate.Are these changes tested?
Yes, all
array.sltandunnest.sltbasedArrayRemove,ArrayRemoveN,ArrayRemoveAlltests are passed.Are there any user-facing changes?
No