[SPARK-32668][SQL] HiveGenericUDTF initialize UDTF should use StructObjectInspector method by ulysses-you · Pull Request #29490 · apache/spark

ulysses-you · 2020-08-20T12:41:49Z

What changes were proposed in this pull request?

Use initialize(StructObjectInspector argOIs) instead initialize(ObjectInspector[] args) in HiveGenericUDTF.

Why are the changes needed?

In our case, we implement a Hive GenericUDTF and override initialize(StructObjectInspector argOIs). Then it's ok to execute with Hive, but failed with Spark SQL. Here is the Spark SQL error msg:

No handler for UDF/UDAF/UDTF 'com.xxxx.xxxUDTF': java.lang.IllegalStateException: 
Should not be called directly Please make sure your function overrides 
`public StructObjectInspector initialize(ObjectInspector[] args)`.

The reason is Spark HiveGenericUDTF call initialize(ObjectInspector[] argOIs) to init a UDTF, but it's a Deprecated method.

    public StructObjectInspector initialize(StructObjectInspector argOIs) throws UDFArgumentException {
        List<? extends StructField> inputFields = argOIs.getAllStructFieldRefs();
        ObjectInspector[] udtfInputOIs = new ObjectInspector[inputFields.size()];

        for(int i = 0; i < inputFields.size(); ++i) {
            udtfInputOIs[i] = ((StructField)inputFields.get(i)).getFieldObjectInspector();
        }

        return this.initialize(udtfInputOIs);
    }

    @Deprecated
    public StructObjectInspector initialize(ObjectInspector[] argOIs) throws UDFArgumentException {
        throw new IllegalStateException("Should not be called directly");
    }

We should use initialize(StructObjectInspector argOIs) to do this so that we can be compatible both of the two method. Same as Hive.

Does this PR introduce any user-facing change?

Yes, fix UDTF initialize method.

How was this patch tested?

manual test and passed HiveUDFDynamicLoadSuite

ulysses-you · 2020-08-20T12:50:50Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala

-  protected lazy val inputInspectors = children.map(toInspector)
+  protected lazy val inputInspectors = {
+    val inspectors = children.map(toInspector)
+    val fields = inspectors.indices.map(index => s"col$index").asJava


The field name is not important so use col0 col1 ...

SparkQA · 2020-08-20T14:00:45Z

Test build #127692 has finished for PR 29490 at commit c014b75.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

ulysses-you · 2020-08-20T15:01:50Z

The failed test caused by #18527, I think we should support UTDF UDTFStack2. cc @wangyum

dlutleixin · 2020-09-15T07:27:44Z

yes, I have the same problem while using hive UDTF in spark-sql or spark.sql because of not override
public StructObjectInspector initialize(ObjectInspector[] args)
then I override it according to error msg and it works.

ulysses-you · 2020-11-24T06:10:19Z

Then what do you think about this ? cc @dongjoon-hyun @sunchao

dongjoon-hyun · 2020-11-24T07:37:47Z

Thank you for pinging me, @ulysses-you .
cc @gatorsmile , too.

SparkQA · 2020-11-26T02:45:57Z

Test build #131822 has finished for PR 29490 at commit c014b75.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

sunchao · 2020-12-30T19:40:41Z

oops sorry @ulysses-you just remembered that you pinged me on this PR. This looks mostly good to me exception one question: since the new API is added in 0.13 and Spark still support Hive 0.12, do we need to take care of backward compatibility here?

cc @wangyum

gatorsmile · 2020-12-30T19:46:42Z

cc @somani

ulysses-you · 2020-12-31T01:20:20Z

thanks @sunchao . If don't miss something, Hive1.2 has been removed since SPARK-32981 with branch-3.1. Hive0.12 is so far for master branch so we don't need care about compatible with it. cc @dongjoon-hyun isn't it ?

gatorsmile · 2020-12-31T01:58:17Z

@sunchao , you think this will be affected by the Hive metastore client versions?

sunchao · 2020-12-31T02:13:56Z

@sunchao , you think this will be affected by the Hive metastore client versions?

This is the part I'm not sure, that is, Spark loading permanent UDF classes from a HMS with version 0.12. But apparently Hive 0.12 doesn't support permanent UDF so seems this is not an issue.

sunchao · 2020-12-31T02:16:18Z

BTW #30665 is trying to solve the same issue and we should consolidate on one PR.

wangyum · 2021-01-01T04:08:02Z

retest this please.

SparkQA · 2021-01-01T05:10:32Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38167/

SparkQA · 2021-01-01T05:29:41Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38167/

SparkQA · 2021-01-01T05:40:19Z

Test build #133578 has finished for PR 29490 at commit c014b75.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-01T05:42:04Z

Test build #133580 has finished for PR 29490 at commit 05bd795.

This patch fails Java style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-01T06:08:31Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38169/

SparkQA · 2021-01-01T06:14:06Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38170/

SparkQA · 2021-01-01T06:18:43Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38170/

SparkQA · 2021-01-01T06:37:14Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38169/

SparkQA · 2021-01-01T07:22:11Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38171/

SparkQA · 2021-01-01T07:30:38Z

Test build #133581 has finished for PR 29490 at commit 6286395.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-01T07:57:30Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38171/

SparkQA · 2021-01-01T08:11:29Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38172/

SparkQA · 2021-01-01T08:35:27Z

Test build #133582 has finished for PR 29490 at commit 361f6ac.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-01T08:44:55Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38172/

SparkQA · 2021-01-01T09:25:36Z

Test build #133583 has finished for PR 29490 at commit 1ba03cd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala

sql/hive/src/test/java/org/apache/spark/sql/hive/execution/UDTFStack3.java

HyukjinKwon · 2021-01-03T02:39:08Z

The code and PR description look making sense but I think somebody should take a close look ... I will leave it to @sunchao and @wangyum if you guys don't mind.

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

sunchao · 2021-01-03T03:32:57Z

I think there could be cases where one registers Hive 0.12 UDFs (although not sure how rare this is) in a HMS that Spark talks to, and things may fail when calling the UDFs since they don't have the new initialize method.

ulysses-you · 2021-01-04T02:16:19Z

@sunchao you mean user create a permanent udf which is from Hive0.12 build-in function ? If so I believe it's really rare ..

sunchao · 2021-01-04T02:42:55Z

@ulysses-you nvm please ignore my comment above :) I was thinking the case where Spark somehow loads the Hive 0.12 GenericUDTF class when executing user's custom UDTF but it seems that's not possible.

wangyum · 2021-01-04T02:46:40Z

Do we still need this error message?

spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala

Lines 91 to 93 in dfa6fb4

    
           if (classOf[GenericUDTF].isAssignableFrom(clazz)) { 
        
             s"$noHandlerMsg\nPlease make sure your function overrides " + 
        
               "`public StructObjectInspector initialize(ObjectInspector[] args)`."

This reverts commit 48c03b9.

…2668

SparkQA · 2021-01-05T03:45:04Z

Test build #133632 has finished for PR 29490 at commit b739b1a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-01-05T05:50:35Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38221/

SparkQA · 2021-01-05T06:31:53Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38221/

wangyum · 2021-01-10T05:20:11Z

Merged to master.

ulysses-you · 2021-01-10T11:56:41Z

thanks all!

init

c014b75

probot-autolabeler bot added the SQL label Aug 20, 2020

ulysses-you commented Aug 20, 2020

View reviewed changes

ulysses-you added 3 commits January 1, 2021 13:17

fix test

7bb6cd4

fix

05bd795

line

6286395

style

361f6ac

format

1ba03cd

HyukjinKwon reviewed Jan 3, 2021

View reviewed changes

sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed Jan 3, 2021

View reviewed changes

sql/hive/src/test/java/org/apache/spark/sql/hive/execution/UDTFStack3.java Outdated Show resolved Hide resolved

wangyum reviewed Jan 3, 2021

View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala Outdated Show resolved Hide resolved

review

bf9d0cb

ulysses-you added 4 commits January 4, 2021 15:54

remove udtf error msg

48c03b9

Revert "remove udtf error msg"

0b060fb

This reverts commit 48c03b9.

Merge branch 'master' of https://github.com/apache/spark into SPARK-3…

6b55165

…2668

remove udtf error msg

b739b1a

wangyum approved these changes Jan 7, 2021

View reviewed changes

wangyum closed this in 48b9611 Jan 10, 2021

ulysses-you deleted the SPARK-32668 branch January 11, 2021 11:33

Conversation

ulysses-you commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

ulysses-you Aug 20, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 20, 2020

Uh oh!

ulysses-you commented Aug 20, 2020

Uh oh!

dlutleixin commented Sep 15, 2020

Uh oh!

ulysses-you commented Nov 24, 2020

Uh oh!

dongjoon-hyun commented Nov 24, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

sunchao commented Dec 30, 2020

Uh oh!

gatorsmile commented Dec 30, 2020

Uh oh!

ulysses-you commented Dec 31, 2020

Uh oh!

gatorsmile commented Dec 31, 2020

Uh oh!

sunchao commented Dec 31, 2020

Uh oh!

sunchao commented Dec 31, 2020

Uh oh!

wangyum commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

SparkQA commented Jan 1, 2021

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon commented Jan 3, 2021

Uh oh!

Uh oh!

sunchao commented Jan 3, 2021

Uh oh!

ulysses-you commented Jan 4, 2021

Uh oh!

sunchao commented Jan 4, 2021

Uh oh!

wangyum commented Jan 4, 2021

Uh oh!

ulysses-you commented Aug 20, 2020 •

edited

Loading