Skip to content

Comments

Add all_match, any_match and none_match functions for arrays#13734

Merged
wenleix merged 3 commits intoprestodb:masterfrom
wenleix:match
Nov 22, 2019
Merged

Add all_match, any_match and none_match functions for arrays#13734
wenleix merged 3 commits intoprestodb:masterfrom
wenleix:match

Conversation

@wenleix
Copy link
Contributor

@wenleix wenleix commented Nov 21, 2019

Backported from trinodb/trino#1045

== RELEASE NOTES ==

General Changes
* Add all_match(), any_match(), and none_match() functions.

wenleix and others added 2 commits November 21, 2019 13:57
Cherry-picked from trinodb/trino#1045

Co-authored-by: Xingyuan Lin <xinlin@linkedin.com>
Cherry-picked from: trinodb/trino#1045

Co-authored-by: Xingyuan Lin <xinlin@linkedin.com>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 21, 2019

CLA Check
The committers are authorized under a signed CLA.

  • ✅ Wenlei Xie (d608223, abbbda1, f8897ba32bfe25ac1fe041dfedf68cc37ce1e278)

@wenleix
Copy link
Contributor Author

wenleix commented Nov 21, 2019

cc @kaikalur

Copy link
Contributor

@rongrong rongrong left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. Only comments are around documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Space after T, before boolean => T, boolean

Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor

Choose a reason for hiding this comment

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

What about empty array? Every predicate would return true? Is that worth documenting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rongrong : It's already discussed:

(a special case is when the array is empty)

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem that the functions are actually listed in alphabetical order so it probably makes more sense to keep these three functions together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rongrong : Looks to me the functions are listed in alphabetical order? Maybe some functions are not following the ordering strictly (although I didn't see it with skim 😄 ) and we should fix them?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's the array_intersect/union/except that's wrong then. Can you add a follow up PR to fix that? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

also add space here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rongrong

Maybe it's the array_intersect/union/except that's wrong then. Can you add a follow up PR to fix that? Thanks!

Yup: #13736 😃

Copy link
Contributor

@rongrong rongrong left a comment

Choose a reason for hiding this comment

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

Please sign the new CLA.

Cherry-picked from: trinodb/trino#1045

Co-authored-by: Xingyuan Lin <xinlin@linkedin.com>
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.

2 participants