-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: Add array_max
function support
#14470
base: main
Are you sure you want to change the base?
feat: Add array_max
function support
#14470
Conversation
array_max
functionarray_max
function support
7d76719
to
0dbdbba
Compare
FYI @findepi I think you mentioned this feature recently |
69a307e
to
222bf3b
Compare
I have the same question for |
222bf3b
to
48ecaa5
Compare
48ecaa5
to
a48dfaa
Compare
Thanks @jayzhan211 for the review. I think we have 2 use cases for |
a48dfaa
to
fbb4afb
Compare
fbb4afb
to
f29c95c
Compare
|
||
match arg1.data_type() { | ||
List(_) | LargeList(_) | FixedSizeList(_, _) => { | ||
let input_array = as_list_array(&arg1)?.value(0); |
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.
as_list_array
works only for List(_)
the LargeList(_) | FixedSizeList(_, _)
codepath doesn't work
AFAIR, it's hard to create instance of FixedSizeList
in SQL, so it will be difficult to test.
i'd suggest to leave it out from the PR for now.
|
||
match arg1.data_type() { | ||
List(_) | LargeList(_) | FixedSizeList(_, _) => { | ||
let input_array = as_list_array(&arg1)?.value(0); |
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.
What is .value(0)
??
## array_max | ||
# array_max scalar function #1 (with positive index) | ||
query I | ||
select array_max(make_array(5, 3, 6, 4)); |
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.
add a test where query process couple arrays in single array_max call
SELECT input, array_max(input)
FROM (
SELECT make_array(d-1, d, d-2)
FROM (VALUES (0), (10), (20), (30), (NULL)) t(d)
)
Sounds like Spark crate is a great place for this #5600 |
Per project guidelines proposal #13706 it feels to me as belong to core. |
Which issue does this PR close?
Closes #14469.
What changes are included in this PR?
Currently, Spark, Snowflake and Presto support
array_max
function. This can also be useful for DataFusion.Spark: https://docs.databricks.com/en/sql/language-manual/functions/array_max.html
Snowflake: https://docs.snowflake.com/en/sql-reference/functions/array_max
Presto: https://prestodb.io/docs/current/functions/array.html#array_max-x-x
All potential use-cases have been covered like different
data_types
,empty array
,NULL
etc.Are these changes tested?
Added new UT cases to verify
array_max
function in terms of different source arrays.Are there any user-facing changes?
Yes, new SQL function is supported and documentation has also be updated.