You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2022/12/01 18:05:22 UTC

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19304


Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 670 insertions(+), 183 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 3:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/11937/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:34:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11987/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:51:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@152
PS9, Line 152:     if (returnOi_ != getInspector(retType_, true)
optional nit: Maybe it makes more sense to check if the returnOi_ is valid within initializeWrapper before setting the variable?

It's nice to know that the variable is already valid by the time it is set.



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 17:17:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#6).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 711 insertions(+), 200 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/6
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@152
PS9, Line 152:     if (returnOi_ != getInspector(retType_, true)
> optional nit: Maybe it makes more sense to check if the returnOi_ is valid 
In the state it would be more logical to check it here, but t my other GenericUdf related review changes this are and uses initializeWrapper in different ways:
https://gerrit.cloudera.org/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java

When using GenericUdf in "generic" way the return type is unknown beforehand and we know it only after initialize(). I would prefer to keep it this way in this patch possibly beatifying it after this patch is merged and the other is rebased.



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Dec 2022 12:38:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS4, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS4, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS4, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS4, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS4, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS4, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS4, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:22:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#5).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 709 insertions(+), 198 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/5
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11979/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:50:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11988/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:53:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS6, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS6, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS6, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS6, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS6, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/6/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS6, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/6/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS6, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:30:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS7, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS7, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS7, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS7, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS7, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS7, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS7, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:48:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 6:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG@18
PS4, Line 18: probably inheriting in the opposite
            :   direction would be more logical
> We could also have an abstract base class and a concrete class for both kin
I would prefer to keep is this way in this patch so that the changes in the actual logic behind the udf is more visible.

https://gerrit.cloudera.org/#/c/19177/ also modifies these classes, I can do this refactor after merging this and rebasing that patch.


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@116
PS4, Line 116: 
> We could insert a precondition check for that.
Done


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
File fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@441
PS4, Line 441:     TestHiveUdf(UDFSign.class, createDouble(1), createDouble(3));
> We only have one Bytes test now, is it intentional?
There are not many builtin functions in Hive that use BINARY. Added a tests for UDFBase64 and UDFUnbase64


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@514
PS4, Line 514:     TestGenericUdf(createFloat(1.1f + 1.2f + 1.3f),
> Can we also test a function with more than one BINARY arguments?
Done


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@56
PS4, Line 56: it re
> Now we return the last (see L322).
Rewrote the function to concatenate the arrays.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@57
PS4, Line 57:  *
> Not new in this patch, but we could also mention that if any argument is nu
Oops, forgot this, will do in next patch.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@389
PS4, Line 389:     BytesWritable resultBinary = new BytesWritable();
> Before this change we returned a BytesWritable without setting it, not 'nul
In the original commit evaluateBinary returned null IMO, see
      if (input == null) {
        return null;
      }
The result == null handling was needed for the 0 argument case.


http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: #Test GenericUDF functions
> We could also check some of the multi-argument functions with Java Object r
I am not sure if this useful in this case, as the change is about the return types and does not change argument type handling.



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:40:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#9).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 740 insertions(+), 208 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/9
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11938/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:42:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@57
PS4, Line 57:  *
> Oops, forgot this, will do in next patch.
Done


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@114
PS4, Line 114: ctor
> Shouldn't this be "first"?
Done


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@123
PS4, Line 123: 
> Can't we take PrimitiveObjectInspector here? No need for the cast then.
Done, I think I just wanted to avoid breaking the line by using the shorter name :)


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@42
PS4, Line 42:  * Similarly to TestGenericUdf this class also has copy in the FE.
> If we wrote here both places where this file is, this line wouldn't have to
The file cannot be simply copied because of the different package, see line 18



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:51:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Dec 2022 12:40:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 10: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Dec 2022 17:50:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 8:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS8, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS8, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS8, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS8, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS8, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/8/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS8, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/8/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS8, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:32:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

LGTM

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: #Test GenericUDF functions
> Done. It would be good to have some generic functions where note all argume
Ok, that's fine. Thanks.



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:53:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#7).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 719 insertions(+), 204 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/7
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS5, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS5, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS5, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS5, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS5, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/5/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS5, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/5/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS5, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:27:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS1, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS1, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS1, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS1, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS1, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS1, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS1, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:06:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 10: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Dec 2022 12:41:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11978/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 14:45:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11935/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:25:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#4).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 670 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#3).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 670 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#2).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 670 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11936/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:28:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 4:

(11 comments)

Thanks, looks good.

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG@18
PS4, Line 18: probably inheriting in the opposite
            :   direction would be more logical
We could also have an abstract base class and a concrete class for both kinds of return types.


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@116
PS4, Line 116:       // Only primitive objects are supported currently.
We could insert a precondition check for that.


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
File fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@441
PS4, Line 441:     TestHiveUdf(UDFBin.class, createText("1100100"), createBigInt(100));
We only have one Bytes test now, is it intentional?


http://gerrit.cloudera.org:8080/#/c/19304/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@514
PS4, Line 514:     TestGenericUdf(createBoolean(true),
Can we also test a function with more than one BINARY arguments?


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@56
PS4, Line 56: first
Now we return the last (see L322).
I wonder if concatenation would be possible for BINARY, too?


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@57
PS4, Line 57:  * that argument.
Not new in this patch, but we could also mention that if any argument is null, the result is also null.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@114
PS4, Line 114: last
Shouldn't this be "first"?


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@123
PS4, Line 123: ObjectInspector
Can't we take PrimitiveObjectInspector here? No need for the cast then.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@389
PS4, Line 389:     if (result == null) return null;
Before this change we returned a BytesWritable without setting it, not 'null'.
Also, we null-check 'result' on L391, too, which is unnecessary.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@42
PS4, Line 42:  * Similarly to TestGenericUdf this class also has copy in the FE.
If we wrote here both places where this file is, this line wouldn't have to be different in the the versions (like with TestGenericUdf.java). That's less error prone (though a bit less informative) because a simple copy is enough to sync the files.


http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: #Test GenericUDF functions
We could also check some of the multi-argument functions with Java Object return types.



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Dec 2022 13:45:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11980/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 15:05:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG@18
PS4, Line 18: probably inheriting in the opposite
            :   direction would be more logical
> I would prefer to keep is this way in this patch so that the changes in the
Sounds good.


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@389
PS4, Line 389:     if (result == null) return null;
> In the original commit evaluateBinary returned null IMO, see
Right, thanks.


http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@54
PS7, Line 54: string
Now it is string and binary types, right?


http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@42
PS4, Line 42:  * Similarly to TestGenericUdf this class also has copy in the FE.
> The file cannot be simply copied because of the different package, see line
Ok, thanks.


http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: #Test GenericUDF functions
> I am not sure if this useful in this case, as the change is about the retur
We could still add one or two cases so that we can see that the Java Object return type also works with multi-arg functions.


http://gerrit.cloudera.org:8080/#/c/19304/7/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/7/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@107
PS7, Line 107: strings
binaries



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 15:13:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Reviewed-on: http://gerrit.cloudera.org:8080/19304
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 740 insertions(+), 208 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS2, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS2, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS2, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS2, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS2, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/2/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS2, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/2/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS2, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:08:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS3, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS3, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS3, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS3, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS3, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/3/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS3, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/3/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS3, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 18:21:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19304

to look at the new patch set (#8).

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................

IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Before this patch only the Writable* types were accepted in GenericUdfs
as return types, while some GenericUdfs in the wild return primitive java
types (e.g. Integer instead of IntWritable). For legacy Hive UDFs these
return types were already handled, so the only change needed was to
map the ObjectInspector subclasses (e.g. JavaIntObjectInspector) to the
correct JavaUdfDataType in Impala.

Testing:
- Added a subclass for TestGenericUdf (TestGenericUdfWithJavaReturnTypes)
  that returns primitive java types (probably inheriting in the opposite
  direction would be more logical, but the diff is smaller this way).
- Changed EE tests to also use TestGenericUdfWithJavaReturnTypes.
- Changed FE tests (UdfExecutorTest) to check both
  TestGenericUdfWithJavaReturnTypes and TestGenericUdf.
- Also added a test with BINARY type to UdfExecutorTest as this was
  forgotten during the original BINARY patch.

Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
---
M fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
M fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
M testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
10 files changed, 736 insertions(+), 204 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/19304/8
-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@32
PS9, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java
File fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@25
PS9, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/main/java/org/apache/impala/hive/executor/JavaUdfDataType.java@26
PS9, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveWritableObjectInspector;
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@25
PS9, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdfWithJavaReturnTypes.java@26
PS9, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java:

http://gerrit.cloudera.org:8080/#/c/19304/9/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@25
PS9, Line 25: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/19304/9/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@26
PS9, Line 26: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:34:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@54
PS7, Line 54: string
> Now it is string and binary types, right?
Done


http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: #Test GenericUDF functions
> We could still add one or two cases so that we can see that the Java Object
Done. It would be good to have some generic functions where note all argument types are the same, but I would do this in a different patch (probably https://gerrit.cloudera.org/#/c/19177/ )


http://gerrit.cloudera.org:8080/#/c/19304/7/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test:

http://gerrit.cloudera.org:8080/#/c/19304/7/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@107
PS7, Line 107: binarie
> binaries
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Dec 2022 12:36:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11549: Support Hive GenericUdfs that return primitive java types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )

Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8885/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/19304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63
Gerrit-Change-Number: 19304
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Dec 2022 12:41:12 +0000
Gerrit-HasComments: No