Skip to content

Generalize existing SQL UDF functions to all types#18587

Merged
kaikalur merged 1 commit intoprestodb:masterfrom
sviscaino:feature/extend-sqlinvokedfunctions-types
Oct 31, 2022
Merged

Generalize existing SQL UDF functions to all types#18587
kaikalur merged 1 commit intoprestodb:masterfrom
sviscaino:feature/extend-sqlinvokedfunctions-types

Conversation

@sviscaino
Copy link
Contributor

Following #18581, this extends the following SQL invoked functions to accept any type as input, not just varchar and double:
array_frequency, array_duplicates, array_has_duplicates, array_intersect

Test plan -
mvn test -Dtest=TestArrayIntersectFunction,TestArraySqlFunctions test

== RELEASE NOTES ==

General Changes
* Extend :func:`array_frequency`, :func:`array_duplicates`, :func:`array_has_duplicates`, :func:`array_intersect(array(array(E))` to accept any type as input instead of only varchar/double

@sviscaino sviscaino requested a review from a team as a code owner October 31, 2022 16:01
@sviscaino sviscaino requested a review from presto-oss October 31, 2022 16:01
Copy link
Contributor Author

@sviscaino sviscaino Oct 31, 2022

Choose a reason for hiding this comment

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

A trick with find_first was needed here because we can't do cast(NULL as T) in the function body

@sviscaino sviscaino force-pushed the feature/extend-sqlinvokedfunctions-types branch from 4945cd2 to 3863093 Compare October 31, 2022 16:12
Copy link
Contributor

@kaikalur kaikalur left a comment

Choose a reason for hiding this comment

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

This is great! Can you add a couple of test to show a) any new types that would work now and b) any that would not (like rows with nulls or something that can't be compared) for all the functions that you fixed? I see for some but also add the error cases say something like: ARRAY[ROW(1, null), ROW(null, 2)] etc.

@sviscaino sviscaino force-pushed the feature/extend-sqlinvokedfunctions-types branch from 3863093 to 5630c9c Compare October 31, 2022 18:47
@sviscaino
Copy link
Contributor Author

This is great! Can you add a couple of test to show a) any new types that would work now and b) any that would not (like rows with nulls or something that can't be compared) for all the functions that you fixed? I see for some but also add the error cases say something like: ARRAY[ROW(1, null), ROW(null, 2)] etc.

Added some more unit tests

@sviscaino sviscaino force-pushed the feature/extend-sqlinvokedfunctions-types branch from 5630c9c to 7c2a16e Compare October 31, 2022 18:48
@kaikalur kaikalur requested a review from highker October 31, 2022 19:41
Copy link
Contributor

@kaikalur kaikalur left a comment

Choose a reason for hiding this comment

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

LGTM

@kaikalur kaikalur merged commit e52a7f1 into prestodb:master Oct 31, 2022
@sviscaino sviscaino deleted the feature/extend-sqlinvokedfunctions-types branch November 3, 2022 11:54
@wanglinsong wanglinsong mentioned this pull request Jan 12, 2023
30 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.

3 participants