Skip to content

Conversation

@wangyum
Copy link
Member

@wangyum wangyum commented Jul 4, 2017

What changes were proposed in this pull request?

It must override public StructObjectInspector initialize(ObjectInspector[] argOIs) when create a UDTF.

If you override public StructObjectInspector initialize(StructObjectInspector argOIs), IllegalStateException will throw. per: HIVE-12377.

This PR catch IllegalStateException and point user to override public StructObjectInspector initialize(ObjectInspector[] argOIs).

How was this patch tested?

unit tests

Source code and binary jar: SPARK-21101.zip
These two source code copy from : https://github.com/apache/hive/blob/release-2.0.0/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFStack.java

@SparkQA
Copy link

SparkQA commented Jul 4, 2017

Test build #79152 has finished for PR 18527 at commit a3bd52b.

  • This patch fails PySpark pip packaging tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Retest this please

throw new AnalysisException(s"No handler for Hive UDF '${clazz.getCanonicalName}'")
}
} catch {
case ise: IllegalStateException =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @wangyum .
Can we move this after line 112, case NonFatal(e) =>?
IMO, these 8 lines are mostly duplication logic of line 113~116. The only difference is error message.
I guess we can append the new error message more simply.

@SparkQA
Copy link

SparkQA commented Jul 4, 2017

Test build #79160 has finished for PR 18527 at commit a3bd52b.

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

@maropu
Copy link
Member

maropu commented Jul 5, 2017

Can you add a test? I think it's okay to add a source file for UDTF in sql/hive/src/test/java/org/apache/spark/sql/hive/execution or sql/hive/src/test/resources and add the test.

@maropu
Copy link
Member

maropu commented Jul 5, 2017

IMO it'd be better to fix code so we could call both interfaces (deprecated and non-deprecated initialize) instead of throwing the exception. cc: @hvanhovell

@SparkQA
Copy link

SparkQA commented Jul 5, 2017

Test build #79173 has finished for PR 18527 at commit 188ed85.

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

@SparkQA
Copy link

SparkQA commented Jul 5, 2017

Test build #79199 has finished for PR 18527 at commit 479fd44.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wangyum
Copy link
Member Author

wangyum commented Jul 5, 2017

Retest this please

@SparkQA
Copy link

SparkQA commented Jul 5, 2017

Test build #79204 has finished for PR 18527 at commit 479fd44.

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

ghost pushed a commit to dbtsai/spark that referenced this pull request Aug 1, 2017
## What changes were proposed in this pull request?
This pr added documents about unsupported functions in Hive UDF/UDTF/UDAF.
This pr relates to apache#18768 and apache#18527.

## How was this patch tested?
N/A

Author: Takeshi Yamamuro <[email protected]>

Closes apache#18792 from maropu/HOTFIX-20170731.
@gatorsmile
Copy link
Member

ping @wangyum This sounds a reasonable fix. Could you resolve the conflicts?

# Conflicts:
#	sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala
#	sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
@SparkQA
Copy link

SparkQA commented Oct 24, 2017

Test build #83007 has finished for PR 18527 at commit 47071b5.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wangyum
Copy link
Member Author

wangyum commented Oct 24, 2017

Retest this please

@SparkQA
Copy link

SparkQA commented Oct 24, 2017

Test build #83009 has finished for PR 18527 at commit 47071b5.

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

new AnalysisException(s"No handler for UDF/UDAF/UDTF '${clazz.getCanonicalName}': $e" +
s"\nIf you create a UDTF, please make sure your function override " +
s"`public StructObjectInspector initialize(ObjectInspector[] args)`, " +
s"per: SPARK-21101")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

          val noHandlerMsg = s"No handler for UDF/UDAF/UDTF '${clazz.getCanonicalName}': $e"
          val errorMsg =
            if (classOf[GenericUDTF].isAssignableFrom(clazz)) {
              s"$noHandlerMsg\nPlease make sure your function overrides " +
                "`public StructObjectInspector initialize(ObjectInspector[] args)`."
            } else {
              noHandlerMsg
            }
          val analysisException = new AnalysisException(errorMsg)

@SparkQA
Copy link

SparkQA commented Oct 24, 2017

Test build #83021 has finished for PR 18527 at commit f5e895b.

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

@gatorsmile
Copy link
Member

LGTM

@gatorsmile
Copy link
Member

Thanks! Merged to master.

@asfgit asfgit closed this in 524abb9 Oct 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants