You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2022/03/07 01:33:02 UTC

[Impala-ASF-CR] IMPALA-11162: generic UDF support

Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18295


Change subject: IMPALA-11162: generic UDF support
......................................................................

IMPALA-11162: generic UDF support

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
XXX: fill in comment
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/java-udf-exception.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
14 files changed, 1,817 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,785 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10388/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 03 Apr 2022 20:38:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10264/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 02:34:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 12: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 17:24:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 14:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/14/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/18295/14/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS14, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java:

http://gerrit.cloudera.org:8080/#/c/18295/14/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@39
PS14, Line 39: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/14/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@32
PS14, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS14, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@27
PS14, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@28
PS14, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@27
PS14, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@28
PS14, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/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/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@32
PS14, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS14, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java:

http://gerrit.cloudera.org:8080/#/c/18295/14/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@27
PS14, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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: Mon, 09 May 2022 17:02:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 14:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10547/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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: Mon, 09 May 2022 17:21:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18295 )

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Reviewed-on: http://gerrit.cloudera.org:8080/18295
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/shaded-deps/hive-exec/pom.xml
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
15 files changed, 1,621 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 14: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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: Tue, 10 May 2022 09:28:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 1:

(39 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/1/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS1, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS1, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@202
PS1, Line 202:   
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@212
PS1, Line 212:   
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@254
PS1, Line 254:       finalBoolean &= currentBool; 
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@255
PS1, Line 255:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@260
PS1, Line 260:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@275
PS1, Line 275:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@290
PS1, Line 290:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@309
PS1, Line 309:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@328
PS1, Line 328:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@333
PS1, Line 333:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@348
PS1, Line 348:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@367
PS1, Line 367:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@386
PS1, Line 386:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@391
PS1, Line 391:     
line has trailing whitespace


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/1/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/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS1, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@217
PS1, Line 217:   
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@232
PS1, Line 232:   
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@289
PS1, Line 289:       finalBoolean &= currentBool; 
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@290
PS1, Line 290:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@295
PS1, Line 295:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@310
PS1, Line 310:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@325
PS1, Line 325:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@344
PS1, Line 344:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@363
PS1, Line 363:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@368
PS1, Line 368:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@383
PS1, Line 383:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@402
PS1, Line 402:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@421
PS1, Line 421:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/18295/1/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@426
PS1, Line 426:     
line has trailing whitespace


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 01:33:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 4:

(35 comments)

Thanks, and sorry for the late review.

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@65
PS4, Line 65: are passed in
How can they not be passed in? If you mean "are not NULL", it would be better to write that in my opinion, because 'not passing in' a parameter suggests to me that there is an overload that doesn't take those parameters.

Also, in checkValidFunction(), the check is always performed (regardless of the types being NULL), isn't it?


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@128
PS4, Line 128: getGenericUDFInstance
'constructGenericUDFInstance' would be better as we already have a 'getGenericUDFInstance' which does a very different thing (is a getter).


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@139
PS4, Line 139: catch (InstantiationException e) {
             :       throw new CatalogException("Unable to call create UDF instance.", e);
             :     } catch (IllegalAccessException e) {
             :       throw new CatalogException("Unable to call create UDF instance.", e);
             :     } catch (InvocationTargetException e) {
             :       throw new CatalogException("Unable to call create UDF instance.", e);
These exceptions are handled identically, so catching them in the same clause could simplify the code (and also generate smaller bytecode):

catch (InstantiationException|IllegalAccessException|CatalogException|CatalogException e) {
  throw new CatalogException("Unable to call create UDF instance.", e);
}

https://docs.oracle.com/javase/8/docs/technotes/guides/language/catch-multiple.html


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@154
PS4, Line 154: !returnOI.getTypeName().equals("void")
Does it mean that if 'genericUDF_.initialize(parameterOIs)' returns 'void' we accept it even if 'retType_' is an int or some other valid type? Shouldn't we only accept 'void' if 'retType_' specifically indicates that we expect a void return type? I know that 'Type' doesn't have a void value, but it could be indicated by 'retType_' being NULL or 'retType_.isInvalid()' being true.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@67
PS4, Line 67: @SuppressWarnings("restriction")
Just curious, why is this warning suppression needed?


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@76
PS4, Line 76:  
Nit: missing 'is'.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@79
PS4, Line 79: deferredObjects_
I think 'parameters_' would be more descriptive name. 'deferredObjects_' actually refers to the type, not what they are used for in the code. If you are worried that it could be confused with 'inputArgs_' in the base class, the name could also be 'deferredParameters_' or something like that.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java@132
PS4, Line 132: " + "
Nit: no need to separate the string literals here.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@50
PS4, Line 50: This class is a copy of the TestGenericUdf class in the FE.
This is actually in the FE. Either this comment should be removed from this file (and only kept in java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java), or if it is important that the two files be exactly the same, the comment should be adjusted so that it is appropriate in both folders.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@59
PS4, Line 59: TestGenericUdf
A class comment should describe what this UDF does for each argument type.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@61
PS4, Line 61: inputTypes
Should have an underscore at the end (and other members too), so inputTypes_, retTypeIO_, retType_.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@85
PS4, Line 85:     retTypeOI = (PrimitiveObjectInspector) arguments[arguments.length - 1];
We do not allow different argument types (this is checked in 'verify*Args()'), so we could take the type of the first argument, which is simpler.
Taking the last argument seems to imply to me that there is a reason for choosing the last one, i.e. there can be different types, which is misleading.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@89
PS4, Line 89: // Ok this is weird.  I tried doing these "if/else" commands as a switch
            :     // statement. WHen I did that, it threw a 'java.lang.NoClassDefFoundError'
            :     // exception with the line number given as the one with the "switch" string.
            :     // This is the same case statement used successfully in the "evaluate" command,
            :     // yet it failed here. Given that this has nothing to do with functionality, I
            :     // decided not to debug this and leave it like this.
This comment should be removed.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@97
PS4, Line 97:       return retTypeOI;
Instead of putting a return statement into each branch, you could put the 'throw' into a general 'else' branch at the end and return after the conditional - if we did not throw by then, we return 'retTypeOI'.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@150
PS4, Line 150: Unsupported return type
This is a bit misleading because the user of this class does not specify the return type directly, but the argument type, so an error message should mention the argument type.

For this UDF the return type is the same as the argument type, so 'retType' could be renamed to something like 'argAndRetType_'.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@159
PS4, Line 159: verifyBooleanArgs
Except for 'byte', these 'verify*Args()' functions are the same, so we could make the expected input type a parameter instead of having a different function for each.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@171
PS4, Line 171:     if (inputTypes.size() > 1) {
Why is 'byte' handled so differently from all the other types?


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@255
PS4, Line 255:       finalBoolean &= currentBool;
For the other types, the behaviour of the UDF is more or less addition (addition for numbers, concatenation for strings). With booleans, addition is usually associated with OR, not AND.
I suggest this is changed to
finalBoolean |= currentBool;


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@276
PS4, Line 276: evaluateShort
Couldn't we unify these 'evaluate*()' implementations, at least the ones for number types, into a generic (template) function? The template parameters would probably be the 'Writable's and (Short, Integer, Long etc.).


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@296
PS4, Line 296: short
Should be 'int', not 'short'.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@384
PS4, Line 384: evaluate
Is it also an evaluation method? Is it used anywhere?


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@475
PS4, Line 475: identity
The multi-argument functions are not identity functions, so this description is misleading.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@485
PS4, Line 485:     TestUdf(null, TestGenericUdf.class, createText("ABCD"), createText("ABCD"));
             :     TestUdf(null, TestGenericUdf.class, createText("ABCD"), createText("ABCD"));
Nit: these two lines are identical, is it intentional?


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

http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@31
PS4, Line 31: GenericImportsNearbyClassesUdf
Could you explain in the class comment what this class is used for?


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

http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@31
PS4, Line 31: GenericReplaceStringUdf
A short comment would be useful about what this UDF does.


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@49
PS4, Line 49:  * This class is a copy of the TestGenericUdf class in the FE. We need this class in a
See the comments for the other copy of this file.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@30
PS4, Line 30: s
Nit: throw.


http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@46
PS4, Line 46: NullPointerException
I'd suggest throwing a general RuntimeException, throwing NullPointerException is misleading as there is no NULL involved anywhere.


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: generic_identity
The multi-argument versions are not identity functions.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@74
PS4, Line 74: BytesWritable, Text,
            : # and String
How are these three types tested? It seems that the type is the same for all.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@88
PS4, Line 88: BytesWritable, Text,
            : # and String
See L74-75.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@170
PS4, Line 170: generic_hive_add
Addition in a boolean context usually means OR, but the current implementation is AND. See the comment in fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java L255.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@272
PS4, Line 272: # Java Generic UDFs for TIMESTAMP are not allowed yet
We could also add tests that cover incorrect return types, for example for org.apache.impala.TestGenericUdf a function that takes two smallints and returns an int - this should also throw an exception, am I right?


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

http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@35
PS4, Line 35: tinyint
It would be more logical if the definition for 'tinyint' was before 'smallint', so the integers would come in increasing byte size.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@51
PS4, Line 51: generic_identity
I wouldn't call these functions '*identity' in the cases where there is more than one argument, maybe not even in the one-argument case.
In the multi-argument cases, they are definitely not identity functions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@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: Tue, 05 Apr 2022 12:24:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 13:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 08 May 2022 19:29:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10417/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@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, 07 Apr 2022 17:03:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@59
PS4, Line 59:  and we can't 
> Could you add a class comment like the one I mentioned? For example
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 16:50:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18295/8/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/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@270
PS8, Line 270: AnalysisException: Type TIMESTAMP is not supported for Java UDFs.
> This one is a bit tricky because the syntax itself isn't supported, so I co
This works for me in impala shell:
create function foo(int...) RETURNS int LOCATION '/test-warehouse/impala-hive-udfs.jar' SYMBOL='org.apache.impala.TestUdf';

I can actually call the function with any number of integer arguments, but it only uses the first one:

Query: select foo(1,3,4,6)
+----------------------------------------+
| default.foo(1, 3, 4, 6) /* java udf */ |
+----------------------------------------+
| 1                                      |
+----------------------------------------+

Meanwhile 
select foo(1,3,4,"a")
returns
ERROR: AnalysisException: No matching function with signature: default.foo(TINYINT, TINYINT, TINYINT, STRING).

I wonder if this is actually a bug, as we should probably return an error in this case, as a legacy Hive UDF cannot  have variable length argument list.

Meanwhile GenericUFSs should support variable length argument list - it is ok to not implement it in this patch, we should have some kind of test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@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, 28 Apr 2022 11:58:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

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

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

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

Change subject: IMPALA-11162: generic UDF support
......................................................................

IMPALA-11162: generic UDF support

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
XXX: fill in comment
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,778 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 13:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/13/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/18295/13/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS13, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java:

http://gerrit.cloudera.org:8080/#/c/18295/13/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@39
PS13, Line 39: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/13/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@32
PS13, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS13, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@27
PS13, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@28
PS13, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@27
PS13, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@28
PS13, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/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/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@32
PS13, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS13, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java:

http://gerrit.cloudera.org:8080/#/c/18295/13/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@27
PS13, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 08 May 2022 19:29:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7909/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 07:12:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@7
PS3, Line 7: Support GenericUDFs for Hive
> Do you plan to merge this commit as it is, or this is a WIP change and you 
The limitations are too big to fix right now.  I'd like to merge it as/is.


http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@24
PS3, Line 24: Functions are not extracted from the jar file. The first generation
            : of Hive UDFs allowed this because the method prototypes are
            : explicitly defined and can be determined at funciton creation time. For
            : GenericUDFs, the return types are determined based on the parameters
            : passed in when running a query.
> Should we extract signatures for generic UDFs? My understanding is that all
My understanding of the current implementation is that a Function object is created which has its return type and parameter types defined as a global object within catalogd.

The problem I am running into here is that it is impossible to create parameter types because of the permutations of all parameters.  For instance (and I realize that it wouldn't be used, but I'm just using an example here): the "+" UDF in Hive supports a Long and an Int as parameters.  The number of permutations we would have with Longs, Ints, SmallInts, TinyInts, etc... would be too many to register at "create function" time.

Of course, we can eventually resolve this limitation, but that would be for a future commit.


http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@30
PS3, Line 30: For the same reason as above, GenericUDFs cannot be made permanent.
            : They will need to be recreated everytime the server is restarted.
            : This is a severe limitation and will be resolved in the near future.
> This is not 100% clear to me.
There is logic in CreateFunctionStatementBase:

    if (getBinaryType() == TFunctionBinaryType.JAVA && hasSignature()) {
      fn_.setIsPersistent(false);
    } else {
      fn_.setIsPersistent(true);
    } 

It doesn't save functions to the database if there is no signature defined at creation time.  There is a "V1" syntax for java functions where parameters are used and a "V2" syntax where one passes in the function name and they are extracted.  As stated in my above comment, there are too many permutations of the V2 function with Generic UDFs for this to make sense. There will have to be another code change to support this.


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@183
PS3, Line 183:       return PrimitiveObjectInspectorFactory.writableIntObjectInspector;
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@189
PS3, Line 189:       return PrimitiveObjectInspectorFactory.writableDoubleObjectInspector;
> nit: indentation
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@73
PS3, Line 73:   private DeferredObject[] deferredObjects_;
            :   private DeferredObject[] runtimeDeferredObjects_;
> Can you add some comments about the roles of these?
Hopefully it's clear with my comment?

A unit test has been added that works with Hive to confirm the behavior mentioned in the comment. I've also tested with a variety of Hive UDFs while implementing.


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

http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@46
PS3, Line 46: /**
> nit: add extra line
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 03 Apr 2022 20:19:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,684 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 14:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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: Mon, 09 May 2022 17:01:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10500/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 18:33:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10499/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 17:11:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,778 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,689 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10456/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 15 Apr 2022 22:29:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 9:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/9/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS9, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/9/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/9/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS9, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/9/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/18295/9/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/9/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS9, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 18:13:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 12:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/12/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/18295/12/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS12, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java:

http://gerrit.cloudera.org:8080/#/c/18295/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@39
PS12, Line 39: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/12/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@32
PS12, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS12, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@27
PS12, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@28
PS12, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@27
PS12, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@28
PS12, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/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/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@32
PS12, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS12, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java:

http://gerrit.cloudera.org:8080/#/c/18295/12/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@27
PS12, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 17:24:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
M java/shaded-deps/hive-exec/pom.xml
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
15 files changed, 1,621 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/18295/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 02:14:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 6:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS6, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/6/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/6/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS6, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/6/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/18295/6/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/6/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS6, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 15 Apr 2022 21:47:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,696 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18295/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18295/8//COMMIT_MSG@26
PS8, Line 26: funciton
typo


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

http://gerrit.cloudera.org:8080/#/c/18295/7/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@72
PS7, Line 72: 
nit: 1 less line


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@49
PS7, Line 49: class is a copy of the TestGenericUdf class in the FE.
This seems quite different at this point, e.g. members have no trailing _. Can you update from the FE version?


http://gerrit.cloudera.org:8080/#/c/18295/8/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/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@254
PS8, Line 254: DATE
Can you also add a test for other unsupported types, e.g. decimal and complex types?


http://gerrit.cloudera.org:8080/#/c/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@270
PS8, Line 270: ====
Do we support variable length argument lists? Can you add a positive or negative test for that?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 17:01:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10545/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 08 May 2022 19:48:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
14 files changed, 1,621 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/18295/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java@65
PS10, Line 65:       Exception e = new CatalogException("tmp");
Is this really needed?


http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java@66
PS10, Line 66: Variable arguments not supporte
Can you add " in Hive UDFs"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 29 Apr 2022 07:09:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 5:

(39 comments)

Thanks, and again sorry for not coming back earlier.

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@65
PS4, Line 65: 
> Removing that comment.  That was changed in one of my iterations.
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@128
PS4, Line 128: xception {
> Changed to "create"
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@139
PS4, Line 139: throw new CatalogException("Unable to call create UDF instance.", e);
             :     }
             :   }
             : 
             :   private void checkValidFunction() throws CatalogException {
             :     try {
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@154
PS4, Line 154: 
> It is important for the caller to match the UDF when it is any specific typ
It's a good point, thanks.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@67
PS4, Line 67: public class HiveUdfExecutorGeneric extends HiveUdfExecutor {
> Cut and paste.  Removing it.
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@76
PS4, Line 76: e
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@79
PS4, Line 79: runtimeDeferredP
> Done
Thanks, but don't forget the underscore at the end, 'deferredParameters_'.


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

http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@79
PS5, Line 79:   private DeferredObject[] runtimeDeferredParameters;
Also here, don't forget the underscore, 'runtimeDeferredParameters_'.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java@132
PS4, Line 132: from 
> Done
Thanks.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@50
PS4, Line 50: Simple Generic UDFs for testing.
> Adjusted comment and code.
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@59
PS4, Line 59: HDFS in testda
> I'm not sure I understand this.
I meant like it computes the sum if the arguments are a number type, concatenates if the arguments are strings etc.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@61
PS4, Line 61: s GenericU
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@85
PS4, Line 85: 
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@89
PS4, Line 89: 
            :     for (ObjectInspector oi : arguments) {
            :       if (!(oi instanceof PrimitiveObjectInspector)) {
            :         throw new UDFArgumentException("Found an input that is not a primitive.");
            :       }
            :       PrimitiveObjectInspector poi = (PrimitiveObjectIns
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@97
PS4, Line 97: 
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@150
PS4, Line 150: 
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@159
PS4, Line 159: SignatureString(a
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@276
PS4, Line 276:  input : inpu
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@296
PS4, Line 296:     r
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@384
PS4, Line 384: 
> Done
Thanks.


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

http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@103
PS5, Line 103:     int numArgsToCheck = (argAndRetType_ == PrimitiveCategory.BYTE) ? 1 : 3;
I don't think there is any point in keeping 'Byte' separate. I know it only has a one-argument version in TestUdf, but I think it is just because nobody bothered to write any other version, I don't think there was any intention to handle it separately.
My opinion is that we should accept any number of arguments for these UDFs.


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@145
PS5, Line 145: int numArgsAllowed
See L103.


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@149
PS5, Line 149: return
It should also be "Unsupported argument type", like on L135.


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@151
PS5, Line 151:     if (inputTypes_.size() > numArgsAllowed) {
See L103.


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@190
PS5, Line 190: Short
Should be 'Byte'.


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@199
PS5, Line 199: 
Nit: unnecessary line.


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@475
PS4, Line 475: GenericU
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@485
PS4, Line 485:     TestUdf(null, TestGenericUdf.class, createText("ABCD"), createText("ABCD"));
             :     TestUdf(null, TestGenericUdf.class, createDouble(3),
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@506
PS4, Line 506:   }
If we choose not to limit the number of UDF arguments like I suggested on L103 in TestGenericUdf.java, no need to remove these tests.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@30
PS4, Line 30:  
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@46
PS4, Line 46: RuntimeException("Te
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: generic_identity
> Done
I think the comment can stay, just we should say "Test GenericUDF functions" or something like that instead.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@74
PS4, Line 74: 
            :        gener
> Done
Do we actually need this test? TestUdf.java has functions for different string-like types (BytesWritable, String, Text) but TestGenericUdf does not.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@170
PS4, Line 170: 
> This test was essentially copied from java-udf.test, so we have a bug there
I would prefer option 2 because the semantics of the functions in the TestGenericUdf class should be consistent. On the other hand I don't think there is any strict requirement that these tests should be the same as the older TestUdf tests.
Also, option 1 is not bad either, it is not uncommon that our commits contain small miscellaneous fixes besides the main issue.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@272
PS4, Line 272: AnalysisException: Type TIMESTAMP is not supported for Java UDFs.
> Not sure how to create a test like that.
Something like
'''
create function generic_add(smallint, smallint) returns int
location '$FILESYSTEM_PREFIX/test-warehouse/impala-hive-udfs.jar'
symbol='org.apache.impala.TestGenericUdf';
---- CATCH
AnalysisException: "SOME_ERROR_MESSAGE"
====
'''


http://gerrit.cloudera.org:8080/#/c/18295/5/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/18295/5/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@85
PS5, Line 85: select length(generic_identity("0123456789")),
I checked IMPALA-1134 and I think we can still reference it here, but we shouldn't say that each call tests a different type as we don't use different string-like types as arguments in TestGenericUdf.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@35
PS4, Line 35: boolean
> Done
Thanks.


http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
File testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test:

http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@27
PS5, Line 27: IDENTITY
Should be lower-case.


http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@51
PS5, Line 51: hive
If we don't use 'hive' in the names of the 'generic_identity' functions and 'generic_throws_exception', 'generic_replace_string', 'generic_import_nearby_classes', we shouldn't use it for the generic add functions either. Or if we do, we should use 'hive' in the other function names as well.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@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, 13 Apr 2022 16:55:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 13: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8082/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@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: Sun, 08 May 2022 23:53:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 5:

(33 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@65
PS4, Line 65: 
> How can they not be passed in? If you mean "are not NULL", it would be bett
Removing that comment.  That was changed in one of my iterations.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@128
PS4, Line 128: xception {
> 'constructGenericUDFInstance' would be better as we already have a 'getGene
Changed to "create"


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@139
PS4, Line 139: throw new CatalogException("Unable to call create UDF instance.", e);
             :     }
             :   }
             : 
             :   private void checkValidFunction() throws CatalogException {
             :     try {
> These exceptions are handled identically, so catching them in the same clau
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@154
PS4, Line 154: 
> Does it mean that if 'genericUDF_.initialize(parameterOIs)' returns 'void' 
It is important for the caller to match the UDF when it is any specific type.  If the caller expects INT, then the UDF had better return an INT.

That doesn't hold true for void.  The caller can send in anything it wants, so it will be a valid function no matter what the UDF returns.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@67
PS4, Line 67: public class HiveUdfExecutorGeneric extends HiveUdfExecutor {
> Just curious, why is this warning suppression needed?
Cut and paste.  Removing it.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@76
PS4, Line 76: e
> Nit: missing 'is'.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@79
PS4, Line 79: runtimeDeferredP
> I think 'parameters_' would be more descriptive name. 'deferredObjects_' ac
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java@132
PS4, Line 132: from 
> Nit: no need to separate the string literals here.
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@50
PS4, Line 50: Simple Generic UDFs for testing.
> This is actually in the FE. Either this comment should be removed from this
Adjusted comment and code.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@59
PS4, Line 59: HDFS in testda
> A class comment should describe what this UDF does for each argument type.
I'm not sure I understand this.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@61
PS4, Line 61: s GenericU
> Should have an underscore at the end (and other members too), so inputTypes
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@85
PS4, Line 85: 
> We do not allow different argument types (this is checked in 'verify*Args()
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@89
PS4, Line 89: 
            :     for (ObjectInspector oi : arguments) {
            :       if (!(oi instanceof PrimitiveObjectInspector)) {
            :         throw new UDFArgumentException("Found an input that is not a primitive.");
            :       }
            :       PrimitiveObjectInspector poi = (PrimitiveObjectIns
> This comment should be removed.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@97
PS4, Line 97: 
> Instead of putting a return statement into each branch, you could put the '
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@150
PS4, Line 150: 
> This is a bit misleading because the user of this class does not specify th
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@159
PS4, Line 159: SignatureString(a
> Except for 'byte', these 'verify*Args()' functions are the same, so we coul
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@171
PS4, Line 171:       if (!(input.get() instanceof BooleanWritable)) {
> Why is 'byte' handled so differently from all the other types?
I tried to grab the same tests from TestUdf.  There was only a one parameter test for Byte, so I just kept it the same.

But I'll keep this consistent


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@255
PS4, Line 255:   public FloatWritable evaluateFloat(DeferredObject[] inputs) throws HiveException {
> For the other types, the behaviour of the UDF is more or less addition (add
This is consistent with the test in TestUdf.  I don't really think it's a huge deal though since this is just a test.


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@276
PS4, Line 276:  input : inpu
> Couldn't we unify these 'evaluate*()' implementations, at least the ones fo
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@296
PS4, Line 296:     r
> Should be 'int', not 'short'.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@384
PS4, Line 384: 
> Is it also an evaluation method? Is it used anywhere?
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@475
PS4, Line 475: GenericU
> The multi-argument functions are not identity functions, so this descriptio
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@485
PS4, Line 485:     TestUdf(null, TestGenericUdf.class, createText("ABCD"), createText("ABCD"));
             :     TestUdf(null, TestGenericUdf.class, createDouble(3),
> Nit: these two lines are identical, is it intentional?
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@49
PS4, Line 49:  * This class is a copy of the TestGenericUdf class in the FE. We need this class in a
> See the comments for the other copy of this file.
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@30
PS4, Line 30:  
> Nit: throw.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@46
PS4, Line 46: RuntimeException("Te
> I'd suggest throwing a general RuntimeException, throwing NullPointerExcept
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: generic_identity
> The multi-argument versions are not identity functions.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@74
PS4, Line 74: 
            :        gener
> How are these three types tested? It seems that the type is the same for al
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@88
PS4, Line 88: 
            : int, int, in
> See L74-75.
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@170
PS4, Line 170: 
> Addition in a boolean context usually means OR, but the current implementat
This test was essentially copied from java-udf.test, so we have a bug there as well.

Three options then here:
1) Fix it in the other place - I don't like this option because it dirties up this commit.
2) Fix it here: I guess we can do this, but then the test is inconsistent with the other one
3) File a Jira and fix both later: I think this is the option I'm gonna go with.  It's just a unit test, so while I think it's a nit, I'd rather leave it as/is for now.


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@272
PS4, Line 272: AnalysisException: Type TIMESTAMP is not supported for Java UDFs.
> We could also add tests that cover incorrect return types, for example for 
Not sure how to create a test like that.


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

http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@35
PS4, Line 35: boolean
> It would be more logical if the definition for 'tinyint' was before 'smalli
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@51
PS4, Line 51: generic_hive_add
> I wouldn't call these functions '*identity' in the cases where there is mor
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@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, 07 Apr 2022 16:41:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/8/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS8, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/8/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/8/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS8, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/8/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/18295/8/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/8/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS8, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 16:51:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

Thanks, +1 from my part.

http://gerrit.cloudera.org:8080/#/c/18295/9/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/18295/9/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@298
PS9, Line 298:  
Nit: extra space.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@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, 28 Apr 2022 09:18:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18295/8/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/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@270
PS8, Line 270: ====
> This works for me in impala shell:
Yeah, heh.  I guess I was using the wrong syntax.  I had a "," before the "..."

I put in some code to disable it for both legacy and generic (easier to put it in one place)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 28 Apr 2022 18:47:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
14 files changed, 1,620 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/18295/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10537/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 17:43:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 7:

(1 comment)

If I'm not mistaken there is only one comment that you didn't address, and the long lines are all in imports, so I can +1 once that comment is addressed.
Let's give others some type to review it in case they also want to.

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

http://gerrit.cloudera.org:8080/#/c/18295/4/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@59
PS4, Line 59: HDFS in testda
> I meant like it computes the sum if the arguments are a number type, concat
Could you add a class comment like the one I mentioned? For example

"The UDF functions in this class take any number of arguments of the same type and implement a generalised addition/summation operation: summation for number types, OR for booleans and concatenation for strings."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@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: Tue, 19 Apr 2022 13:46:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 2:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/2/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS2, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/2/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/2/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS2, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/2/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/18295/2/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/2/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS2, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 02:14:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 3:

(7 comments)

Still processing the code, some high level comments + nits.

http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@7
PS3, Line 7: Support GenericUDFs for Hive
Do you plan to merge this commit as it is, or this is a WIP change and you want to merge after solving some of the limitations?


http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@24
PS3, Line 24: Functions are not extracted from the jar file. The first generation
            : of Hive UDFs allowed this because the method prototypes are
            : explicitly defined and can be determined at funciton creation time. For
            : GenericUDFs, the return types are determined based on the parameters
            : passed in when running a query.
Should we extract signatures for generic UDFs? My understanding is that all generic UDFs will become a single function that accepts arbitrary parameters, and we will check during analyses whether the arguments are valid and what will be the return type. Or Hive behaves differently?


http://gerrit.cloudera.org:8080/#/c/18295/3//COMMIT_MSG@30
PS3, Line 30: For the same reason as above, GenericUDFs cannot be made permanent.
            : They will need to be recreated everytime the server is restarted.
            : This is a severe limitation and will be resolved in the near future.
This is not 100% clear to me.

We load some functions in test_generic_java_udfs - so these functions are not persisted to HMS, only while Impala is running? Which logic decides that the same syntax leads to permanent legacy udf functions, but temporary generic UDFs?


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@183
PS3, Line 183:       return PrimitiveObjectInspectorFactory.writableIntObjectInspector;
nit: indentation


http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@189
PS3, Line 189:       return PrimitiveObjectInspectorFactory.writableDoubleObjectInspector;
nit: indentation


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

http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@73
PS3, Line 73:   private DeferredObject[] deferredObjects_;
            :   private DeferredObject[] runtimeDeferredObjects_;
Can you add some comments about the roles of these?
My understanding is that these two are the same for non-NULL values, while we set runtimeDeferredObjects_ to null if a value is NULL.

Is this how genericUDF expects to get NULL arguments? Shouldn't be the DeferredValue non-null, but return null on get()? Hive doesn't seem to tell this exactly: https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java#L208

deferredObjects_ are backed by HiveUdfExecutor *WriteAble inputObjects_ in HiveUdfExecutor, right?


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

http://gerrit.cloudera.org:8080/#/c/18295/3/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@46
PS3, Line 46: /**
nit: add extra line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@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, 29 Mar 2022 15:37:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 7:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/7/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS7, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/7/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/7/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS7, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS7, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 15 Apr 2022 21:49:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 10:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/10/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/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS10, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@39
PS10, Line 39: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@32
PS10, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS10, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@27
PS10, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@28
PS10, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@27
PS10, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@28
PS10, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/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/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@32
PS10, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS10, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@27
PS10, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@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, 28 Apr 2022 18:47:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 11:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/11/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/18295/11/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS11, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java:

http://gerrit.cloudera.org:8080/#/c/18295/11/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@39
PS11, Line 39: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
File fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/11/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@32
PS11, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS11, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@27
PS11, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java@28
PS11, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
File java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java:

http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@27
PS11, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java@28
PS11, Line 28: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/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/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@32
PS11, Line 32: import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector.PrimitiveCategory;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS11, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java:

http://gerrit.cloudera.org:8080/#/c/18295/11/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java@27
PS11, Line 27: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 16:54:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 12:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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: Sat, 07 May 2022 19:11:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
14 files changed, 1,620 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/18295/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 5:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18295/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/18295/5/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@31
PS5, Line 31: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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

http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.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/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@33
PS5, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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


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


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

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


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


http://gerrit.cloudera.org:8080/#/c/18295/5/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/18295/5/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.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/18295/5/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@33
PS5, Line 33: import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
line too long (95 > 90)


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

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@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, 07 Apr 2022 16:42:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 4:

(15 comments)

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

http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@79
PS5, Line 79:   private DeferredObject[] deferredObjects_;
> Also here, don't forget the underscore, 'runtimeDeferredParameters_'.
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@103
PS5, Line 103:       return retTypeOI;
> I don't think there is any point in keeping 'Byte' separate. I know it only
I prematurely uploaded this.  I first made the change here, then I made the change to Byte to support more arguments.  I meant to come back here and fix this test and then forgot :(


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@145
PS5, Line 145: 
> See L103.
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@149
PS5, Line 149: 
> It should also be "Unsupported argument type", like on L135.
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@151
PS5, Line 151:     }
> See L103.
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@190
PS5, Line 190: 
> Should be 'Byte'.
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java@199
PS5, Line 199:             getSignatureString(retType, inputTypes));
> Nit: unnecessary line.
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java@506
PS4, Line 506:     TestUdf(null, TestGenericUdf.class, createInt(5 + 6 + 7 + 8), createInt(5),
> If we choose not to limit the number of UDF arguments like I suggested on L
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24
PS4, Line 24: generic_identity
> I think the comment can stay, just we should say "Test GenericUDF functions
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@74
PS4, Line 74: BytesWritable, Text,
            : # and String
> Do we actually need this test? TestUdf.java has functions for different str
Done


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@170
PS4, Line 170: generic_hive_add
> I would prefer option 2 because the semantics of the functions in the TestG
Ok, changed it


http://gerrit.cloudera.org:8080/#/c/18295/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@272
PS4, Line 272: # Java Generic UDFs for TIMESTAMP are not allowed yet
> Something like
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/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/18295/5/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@85
PS5, Line 85: 'why hello there','why hello there','why hello there','NULL','NULL','NULL'
> I checked IMPALA-1134 and I think we can still reference it here, but we sh
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
File testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test:

http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@27
PS5, Line 27: identity
> Should be lower-case.
Done


http://gerrit.cloudera.org:8080/#/c/18295/5/testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test@51
PS5, Line 51: iden
> If we don't use 'hive' in the names of the 'generic_identity' functions and
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 15 Apr 2022 21:48:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10455/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 15 Apr 2022 22:28:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: generic UDF support

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

Change subject: IMPALA-11162: generic UDF support
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10263/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 01:53:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10506/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@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, 28 Apr 2022 19:08:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
14 files changed, 1,620 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/18295/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
File fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java:

http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java@65
PS10, Line 65:       Exception e = new CatalogException("tmp");
> Is this really needed?
Accidental push


http://gerrit.cloudera.org:8080/#/c/18295/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java@66
PS10, Line 66: Variable arguments not supporte
> Can you add " in Hive UDFs"?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 16:53:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 12: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8081/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@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: Sat, 07 May 2022 23:51:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 14: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@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: Mon, 09 May 2022 21:30:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10534/ : 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/18295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@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: Fri, 06 May 2022 17:13:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at funciton creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,690 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

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

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

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................

IMPALA-11162: Support GenericUDFs for Hive

Hive has 2 types of UDFs. This commit contains limited
support for the second generation UDFs called GenericUDFs.

The main limitations are as follows:

Decimal types are not supported.  The Impala framework determines
the precision and scale of the decimal return type. However, the
Hive GenericUDFs allow the capability to choose its own return
type based on the parameters. Until this can be resolved, it is
safer to forbid decimals from being used. Note that this
limitation currently exists in the first generation of Hive Java
UDFs.

Complex types are not supported.

Functions are not extracted from the jar file. The first generation
of Hive UDFs allowed this because the method prototypes are
explicitly defined and can be determined at function creation time. For
GenericUDFs, the return types are determined based on the parameters
passed in when running a query.

For the same reason as above, GenericUDFs cannot be made permanent.
They will need to be recreated everytime the server is restarted.
This is a severe limitation and will be resolved in the near future.

Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
---
A fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java
M fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java
A fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
A fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericImportsNearbyClassesUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/GenericReplaceStringUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
A java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfException.java
A testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
A testdata/workloads/functional-query/queries/QueryTest/load-generic-java-udfs.test
M tests/query_test/test_udfs.py
13 files changed, 1,610 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@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-11162: Support GenericUDFs for Hive

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

Change subject: IMPALA-11162: Support GenericUDFs for Hive
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18295/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18295/8//COMMIT_MSG@26
PS8, Line 26: funciton
> typo
Done


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

http://gerrit.cloudera.org:8080/#/c/18295/7/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java@72
PS7, Line 72: 
> nit: 1 less line
Done


http://gerrit.cloudera.org:8080/#/c/18295/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/18295/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@49
PS7, Line 49: class is a copy of the TestGenericUdf class in the FE.
> This seems quite different at this point, e.g. members have no trailing _. 
Done


http://gerrit.cloudera.org:8080/#/c/18295/8/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/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@254
PS8, Line 254: DATE
> Can you also add a test for other unsupported types, e.g. decimal and compl
Done


http://gerrit.cloudera.org:8080/#/c/18295/8/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@270
PS8, Line 270: ====
> Do we support variable length argument lists? Can you add a positive or neg
This one is a bit tricky because the syntax itself isn't supported, so I could literally put in anything and it would give a syntax error.

But I'm assuming the syntax we will use is "...", so I added a case for that.

I suppose I could go back into the code and handle a "..."?  It does seem that printing a syntax error should be a good enough message to deter someone from trying it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6fd09120db413fade94410c83ebe8ff104013cd
Gerrit-Change-Number: 18295
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@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, 27 Apr 2022 18:16:09 +0000
Gerrit-HasComments: Yes