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
HIVE-27628 - implement array_remove UDF in Hive #4612
Conversation
// Check if list element and value are of same type | ||
if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) { | ||
throw new UDFArgumentTypeException(VALUE_IDX, | ||
String.format("%s expected at function %s, but %s is found", arrayElementOI.getTypeName(), FUNC_NAME, |
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.
It could be kinder if we add more information. int expected at function array_remove, but double is found
might be confusing because it might not be obvious which array_remove(array<int>, double)
or array_remove(array<double>, int)
is invoked unless checking this file.
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.
Changed message as recommended, now it looks like int type element is expected at function array_remove(array<int>,int), but void is found
DESCRIBE FUNCTION array_remove; | ||
DESCRIBE FUNCTION EXTENDED array_remove; | ||
|
||
-- evaluates function for array of primitives |
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.
It could be nice if we have a test case where the second argument is NULL.
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.
int type element is expected at function array_remove(array<int>,int), but void is found
is thrown if we have null in second argument. so added that test case in Junit file <runAndVerify(asList(i1, i2, i3, i4),null,null);
>
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.
Thanks. Just for your info, I guess array_remove({list}, CAST(null AS {element type})
works.
@@ -0,0 +1,123 @@ | |||
PREHOOK: query: DESCRIBE FUNCTION array_remove |
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 double checked the behavior using Spark 3.4.1
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
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 +1
… Rama Rao Lethavadla, reviewed by Okumin, Sai Hemanth Gantasala)
What changes were proposed in this pull request?
Implement array_remove function in Hive
Why are the changes needed?
This enhancement is already implemented in Spark
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
Created Junit tests as well as qtests as part of this change