Skip to content

Adding support for NO_KEYS_MATCH udf in presto#19838

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
jainavi17:none_key_match
Jun 26, 2023
Merged

Adding support for NO_KEYS_MATCH udf in presto#19838
rschlussel merged 1 commit intoprestodb:masterfrom
jainavi17:none_key_match

Conversation

@jainavi17
Copy link
Contributor

@jainavi17 jainavi17 commented Jun 8, 2023

Test plan

Added unit tests.
Build successfully using the following terminal command

  • ./mvnw clean install -Dtest=TestNoKeysMatchFunction -fn -pl presto-main
== RELEASE NOTES ==

General Changes
- Add function :func:`no_keys_match`

@jainavi17 jainavi17 requested a review from a team as a code owner June 8, 2023 19:51
@jainavi17 jainavi17 requested a review from presto-oss June 8, 2023 19:51
@feilong-liu
Copy link
Contributor

Refer to #19853 (review) for similar comments

@pranjalssh
Copy link
Contributor

Please address Rebecca's comments from other PR and do same for all PRs. Also make sure all required jobs are passing

@jainavi17
Copy link
Contributor Author

Please address Rebecca's comments from other PR and do same for all PRs. Also make sure all required jobs are passing

Hi @pranjalssh thanks for your comment. Its address now. Please have a look.

Copy link
Contributor

@rschlussel rschlussel left a comment

Choose a reason for hiding this comment

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

can you rename the function no_keys_match (or alternatively no_key_matches is fine too)?

@kaikalur
Copy link
Contributor

can you rename the function no_keys_match (or alternatively no_key_matches is fine too)?

I think keeping them consistent with the array ones (NONE_MATCH etc) is ok. But I'm ok with rename as well.

@rschlussel
Copy link
Contributor

can you rename the function no_keys_match (or alternatively no_key_matches is fine too)?

I think keeping them consistent with the array ones (NONE_MATCH etc) is ok. But I'm ok with rename as well.

I'd rather change, as NONE_KEY_MATCH is harder to parse/understand. We can keep internally for these functions though, so:
all_keys_match
no_keys_match
any_keys_match

@jainavi17
Copy link
Contributor Author

can you rename the function no_keys_match (or alternatively no_key_matches is fine too)?

I'm trying to keep the name same as array function so that its easier to find. We have NONE_MATCH in array. Let me know your thoughts.

@jainavi17 jainavi17 force-pushed the none_key_match branch 2 times, most recently from 9f1ab95 to d18fd8c Compare June 24, 2023 04:38
@jainavi17 jainavi17 changed the title Adding support for NONE_KEY_MATCH udf in presto Adding support for NO_KEYS_MATCH udf in presto Jun 24, 2023
UDF to check if none of the keys present in the map matches the given predicate
@rschlussel rschlussel merged commit c5db905 into prestodb:master Jun 26, 2023
@wanglinsong wanglinsong mentioned this pull request Jul 27, 2023
28 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.

5 participants