-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add array_first and array_last functions
#27295
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
base: master
Are you sure you want to change the base?
Conversation
core/trino-main/src/main/java/io/trino/operator/scalar/ArrayElementLastFunction.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/operator/scalar/ArrayElementFirstFunction.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/operator/scalar/ArrayElementFirstFunction.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/operator/scalar/ArrayElementLastFunction.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/operator/scalar/ArrayElementFirstFunction.java
Outdated
Show resolved
Hide resolved
2cc7cfc to
7e0539a
Compare
raunaqmorarka
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
@martint PTAL
|
Seems reasonable to add these. Let's rename the functions to |
array_first and array_last functions
|
|
||
| assertThat(assertions.function("element_first", "ARRAY[1.9, 2, 2.3]")) | ||
| .isEqualTo(decimal("0000000001.9", createDecimalType(11, 1))); | ||
| } |
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.
Can we add additional test with nested array as well ?
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 added
assertThat(assertions.function("array_first", "ARRAY_FIRST(ARRAY[ARRAY[1, 2], ARRAY[3, 4]]) "))
.isEqualTo(1);
I think that is what you mean
7e4201f to
126092b
Compare
126092b to
894253a
Compare
martint
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.
A couple of minor comments. Otherwise, it looks good. Once you apply the changes, I'll merge it. Thanks!
| assertThat(assertions.function("array_first", "ARRAY[NULL, ARRAY[1, 2], ARRAY[3]]")) | ||
| .isNull(new ArrayType(INTEGER)); | ||
|
|
||
| assertThat(assertions.function("array_first", "ARRAY_FIRST(ARRAY[ARRAY[1, 2], ARRAY[3, 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.
The convention for function names is to use lower case: array_first
| assertThat(assertions.function("array_last", "ARRAY[ARRAY[1, 2], ARRAY[3], NULL]")) | ||
| .isNull(new ArrayType(INTEGER)); | ||
|
|
||
| assertThat(assertions.function("array_last", "ARRAY_LAST(ARRAY[ARRAY[1, 2], ARRAY[3, 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.
same as above
Description
Backstory: reason why
element_atexists:SELECT ARRAY['foo', 'bar'][10]throws array out of bounds exceptionSELECT element_at(ARRAY['foo', 'bar'], 10)returnsNULL, which is convenientIn many queries I use, I need the first element. However, as functions are nested, the arguments are not always very readable:
element_at(element_at(filter(persons, person -> person.age > 18), 1).addresses, 1).streetThis PR introduces
element_firstandelement_last:element_first(element_first(filter(persons, person -> person.age > 18)).addresses).streetThe
element_lastfunction can be used to return the last element.Additional context and related issues
In Java, a List has
getFirstandgetLastas well: https://docs.oracle.com/en/java/javase/25/docs/api//java.base/java/util/List.html#getFirst()Naming considerations
I considered
first()andarray_first(), but I thinkelement_first()is better because it is much closer to the other function that people will use, theelement_at()(also better when the client auto-suggests function names when typingelement_)Release notes