HIVE-27663 - implement array_position UDF in Hive by tarak271 · Pull Request #4653 · apache/hive

tarak271 · 2023-09-01T15:28:08Z

What changes were proposed in this pull request?

Implement array_position 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

okumin · 2023-09-04T12:42:08Z

 public abstract class AbstractGenericUDFArrayBase extends GenericUDF {

    static final int ARRAY_IDX = 0;
+    static final int ELEMENT_IDX = 1;


I guess it is more flexible if checkValueAndListElementTypes accepts an index than we hardcode the index here.
For example, array_insert of Spark SQL receives an element in the 3rd argument, not the 2nd.
https://spark.apache.org/docs/latest/api/sql/index.html#array_insert

Another option is we keep only checkValueAndListElementTypes(arrayElementOI, valueOI), removing checkValueAndListElementTypes(arguments, index?).

Yes, keeping only checkValueAndListElementTypes(arrayElementOI, valueOI, elementIndex) method with index argument

okumin · 2023-09-04T12:51:51Z

+ */
+@Description(name = "array_position", value = "_FUNC_(array, element) - Returns the position of the first occurrence of "
+    + "element in array. Array indexing starts at 1. If the element value is NULL, a NULL is returned.", extended =
+    "Example:\n" + "  > SELECT _FUNC_(array(1, 2, 3,4,2), 2) FROM src;\n" + "  2")


I originally thought arrays of Hive, Spark, MySQL, etc. were 0-based. But I also see array_position of Spark is 1-based...

https://spark.apache.org/docs/latest/api/sql/index.html#array_position

spark-sql (default)> SELECT array(1, 2, 3, 4, 5)[3]; 4 spark-sql (default)> SELECT array_position(array(1, 2, 3, 4, 5), 3); 3

Yes, tried to replicate spark's behaviour

okumin · 2023-09-04T12:54:46Z

+    if (arrayOI.getListLength(array) <= 0 || value == null) {
+      return null;
+    }
+    List<?> resultArray = new ArrayList<>(((ListObjectInspector) argumentOIs[ARRAY_IDX]).getList(array));


We may not need to copy the original object here because we don't return the reference of resultArray.

okumin · 2023-09-04T12:57:04Z

+  public Object evaluate(DeferredObject[] arguments) throws HiveException {
+    Object array = arguments[ARRAY_IDX].get();
+    Object value = arguments[ELEMENT_IDX].get();
+    if (arrayOI.getListLength(array) <= 0 || value == null) {


I guess the first condition should be arrayOI.getListLength(array) < 0.

tarak271 · 2023-09-06T05:14:40Z

@okumin I have updated logic to deal with array of Varchar types. Will create another pull request with these varchar changes to the rest of array udfs

sonarqubecloud · 2023-09-06T07:07:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

No Coverage information
No Duplication 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.
Read more here

tarak271 · 2023-09-29T09:04:48Z

Created https://issues.apache.org/jira/browse/HIVE-27681 to handle varchar data type in all other Array UDFs. @okumin @saihemanth-cloudera need your help in review and merge

okumin · 2023-09-06T13:48:23Z

+        // Check if list element and value are of same type
+        if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) {
+            throw new UDFArgumentTypeException(elementIndex,
+                String.format("%s type element is expected at function array_position(array<%s>,%s), but %s is found",


Sorry, I overlooked that this also contains array_position(array<%s>, %s). Maybe, that part, at least the function name, has to be a placeholder.

My bad, created placefolder for function name as well

okumin · 2023-09-30T14:29:20Z

+    }
+    List<?> resultArray = ((ListObjectInspector) argumentOIs[ARRAY_IDX]).getList(array);
+    // Handling Varchar type this way as Object comparison between string and varchar will not work
+    if ((argumentOIs[ELEMENT_IDX].getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME)


What is examples when we need this one?

The below example is for a different UDF, but applies to this as well

CREATE TABLE temp (top ARRAY<STRING>); insert into temp values(array(cast('true' as VARCHAR(10)))); select array_contains(top,cast(true as VARCHAR(10))) from temp;

okumin · 2023-09-30T14:39:15Z

+        }
+      }
+    }
+    return new IntWritable(resultArray.indexOf(value) + 1);


We might have assumed the returned value of ListObjectInspector#getList can be directly used. But I started feeling it is still an abstract value, and then we need to parse them via the object inspector of the element.
https://github.com/apache/hive/blob/rel/release-4.0.0-beta-1/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFArrayContains.java#L121-L122

This is the example where I compared arrays in a table and arrays or an element as a literal. It could be better to check other similar UDFs that have been merged first.

0: jdbc:hive2://hive-hiveserver2:10000/defaul> create table test as select array('a', 'b', 'c', 'b') as a union all select array('a', 'c', 'd') as a; ... 0: jdbc:hive2://hive-hiveserver2:10000/defaul> select a, array_contains(a, 'b') from test; +--------------------+--------+ | a | _c1 | +--------------------+--------+ | ["a","b","c","b"] | true | | ["a","c","d"] | false | +--------------------+--------+ ... 0: jdbc:hive2://hive-hiveserver2:10000/defaul> select a, array_position(a, 'b') from test; +--------------------+------+ | a | _c1 | +--------------------+------+ | ["a","b","c","b"] | 0 | | ["a","c","d"] | 0 | +--------------------+------+ 0: jdbc:hive2://hive-hiveserver2:10000/defaul> select a, array_except(a, array('a', 'd')) from test; ... +--------------------+----------------+ | a | _c1 | +--------------------+----------------+ | ["a","b","c","b"] | ["a","b","c"] | | ["a","c","d"] | ["a","c","d"] | +--------------------+----------------+

Understood. Changed to use ObjectInspector.compare

Will fix the rest of the array UDFs in https://issues.apache.org/jira/browse/HIVE-27681

tarak271 · 2023-10-23T06:23:17Z

@okumin @saihemanth-cloudera need your help in review and merge

okumin · 2023-10-24T12:54:16Z

+            && poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING)) || (
+            poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING)
+                && poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR))) {
+          return true;


What's the reason why we need this? I feel this is a little risky since it affects several places.

Varchar and string should be convertible and be able to compare. Moving this to base class of Array UDFs, AbstractGenericUDFArrayBase

sonarqubecloud · 2023-10-27T16:09:30Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
8 Code Smells

No Coverage information
No Duplication 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.
Read more here

okumin

I would say the current implementation doesn't have any defects. I'm looking forward to using it

okumin · 2024-02-17T04:59:12Z

@@ -0,0 +1,172 @@
+PREHOOK: query: DESCRIBE FUNCTION array_position


I confirmed all results are consistent with Spark SQL 3.4.1.

sonarqubecloud · 2024-03-05T05:46:07Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

…ka Rama Rao Lethavadla, Reviewed by Okumin, Sai Hemanth Gantasala)

asf-ci-hive added tests pending tests passed and removed tests pending labels Sep 1, 2023

okumin reviewed Sep 4, 2023

View reviewed changes

asf-ci-hive added tests pending and removed tests passed labels Sep 6, 2023

asf-ci-hive added tests passed and removed tests pending labels Sep 6, 2023

tarak271 requested a review from okumin September 6, 2023 08:49

okumin reviewed Sep 30, 2023

View reviewed changes

asf-ci-hive added tests pending tests failed tests passed and removed tests passed tests pending tests failed labels Oct 18, 2023

tarak271 requested a review from okumin October 19, 2023 07:31

okumin reviewed Oct 24, 2023

View reviewed changes

asf-ci-hive added tests pending and removed tests passed labels Oct 27, 2023

asf-ci-hive added tests passed and removed tests pending labels Oct 27, 2023

tarak271 requested a review from okumin November 1, 2023 06:06

tarak271 added 3 commits February 16, 2024 16:47

HIVE-27663 - implement array_position UDF in Hive

fdf9196

HIVE-27663 - implement array_position UDF in Hive

ab7d386

HIVE-27663 - implement array_position UDF in Hive

aff09f0

tarak271 force-pushed the tarak-HIVE-27663 branch from 9670184 to aff09f0 Compare February 16, 2024 11:18

asf-ci-hive added tests pending and removed tests failed labels Feb 16, 2024

HIVE-27663 - implement array_position UDF in Hive

4dabfd1

asf-ci-hive added tests failed tests pending tests passed and removed tests pending tests failed labels Feb 16, 2024

okumin approved these changes Feb 17, 2024

View reviewed changes

github-actions Bot closed this Feb 25, 2024

SourabhBadhya reopened this Feb 26, 2024

asf-ci-hive added tests pending tests passed and removed tests passed tests pending labels Feb 26, 2024

github-actions Bot closed this Mar 5, 2024

SourabhBadhya reopened this Mar 5, 2024

asf-ci-hive added tests pending and removed tests passed labels Mar 5, 2024

asf-ci-hive added tests passed and removed tests pending labels Mar 5, 2024

saihemanth-cloudera approved these changes Mar 6, 2024

View reviewed changes

saihemanth-cloudera merged commit f4e4114 into apache:master Mar 6, 2024

dengzhhu653 pushed a commit to dengzhhu653/hive that referenced this pull request Mar 7, 2024

HIVE-27663 - implement array_position UDF in Hive (apache#4653) (Tara…

06696b3

…ka Rama Rao Lethavadla, Reviewed by Okumin, Sai Hemanth Gantasala)

		@@ -0,0 +1,172 @@
		PREHOOK: query: DESCRIBE FUNCTION array_position

Uh oh!

Conversation

tarak271 commented Sep 1, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

Is the change a dependency upgrade?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarak271 Sep 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarak271 commented Sep 6, 2023

Uh oh!

sonarqubecloud Bot commented Sep 6, 2023

Uh oh!

tarak271 commented Sep 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okumin Sep 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarak271 commented Oct 23, 2023

Uh oh!

okumin Oct 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Oct 27, 2023

Uh oh!

okumin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Mar 5, 2024

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tarak271 Sep 6, 2023 •

edited

Loading

okumin Sep 30, 2023 •

edited

Loading

okumin Oct 24, 2023 •

edited

Loading