[SPARK-33704][SQL] Support latest version of initialize() in HiveGenericUDTF by southernriver · Pull Request #30665 · apache/spark

southernriver · 2020-12-08T07:18:33Z

What changes were proposed in this pull request?

For HiveGenericUDTF , there are two initialization methods:

  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] = inputFields.get(i).getFieldObjectInspector();
    }
    return initialize(udtfInputOIs);
  }
  @Deprecated
  public StructObjectInspector initialize(ObjectInspector[] argOIs)
      throws UDFArgumentException {
    throw new IllegalStateException("Should not be called directly");
  }

As https://issues.apache.org/jira/browse/HIVE-5737 mentioned, hive provided StructObjectInspector for UDTFs rather than ObjectInspect[], but Spark SQL still only support deprecated function.

An exception will be reported before fix:

Error in query: No handler for UDF/UDAF/UDTF 'FeatureParseUDTF1': java.lang.IllegalStateException: Should not be called directly
Please make sure your function overrides public StructObjectInspector initialize(ObjectInspector[] args).; line 1 pos 7

This pr will resolve the exception.

Why are the changes needed?

For the migration of Hive to Spark SQL，we face many UDTF functions throw exception because this issue，It’s really a great improvement for attracting users to Spark SQL.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

manual

…or HiveGenericUDTF

HyukjinKwon · 2020-12-09T08:47:27Z

cc @wangyum

HyukjinKwon · 2020-12-09T08:47:45Z

ok to test

SparkQA · 2020-12-09T09:13:17Z

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

SparkQA · 2020-12-09T09:27:54Z

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

SparkQA · 2020-12-09T11:28:48Z

Test build #132480 has finished for PR 30665 at commit 0e3dca1.

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

wangyum · 2020-12-09T13:30:28Z

@southernriver Could you make pr against master branch?

wangyum

Remove build/._scala-2.12.10 and build/._zinc-0.3.15.

wangyum · 2020-12-09T13:33:20Z

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

+            try {
+              udfExpr = Some(HiveGenericUDTF(name, new HiveFunctionWrapper(clazz.getName), input))
+              // Force it to check input data types.
+              udfExpr.get.asInstanceOf[HiveGenericUDTF].elementSchema
+            } catch {
+              case exception: Exception =>
+                logInfo(s"HiveGenericUDTF initialize(ObjectInspector[] args) is deprecated, and" +
+                  s" we will suit the latest version of initialize(StructObjectInspector argOIs).")
+                udfExpr = Some(HiveGenericUDTF(name, new HiveFunctionWrapper(clazz.getName),
+                  input, false))
+                udfExpr.get.asInstanceOf[HiveGenericUDTF].elementSchema


How about?

val funcWrapper = new HiveFunctionWrapper(clazz.getName) try { udfExpr = Some(HiveGenericUDTF(name, funcWrapper, input, true)) // Force it to check data types. udfExpr.get.asInstanceOf[HiveGenericUDTF].elementSchema } catch { case e: IllegalStateException if e.getMessage.equals("Should not be called directly") => logInfo("Fallback to use the non deprecated UDTF constructor.") udfExpr = Some(HiveGenericUDTF(name, funcWrapper, input, false)) // Force it to check data types. udfExpr.get.asInstanceOf[HiveGenericUDTF].elementSchema }

wangyum · 2020-12-09T13:33:59Z

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

    funcWrapper: HiveFunctionWrapper,
-    children: Seq[Expression])
+    children: Seq[Expression],
+    deprecated: Boolean = true)


deprecated: Boolean = true -> isDeprecatedConstructor: Boolean?

wangyum · 2020-12-09T13:34:48Z

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

+  protected lazy val outputInspector =
+    if (deprecated) {
+      function.initialize(inputInspectors.toArray)
+    } else {
+      function.initialize(rowOI)
+    }


How about?

protected lazy val outputInspector = { if (isDeprecatedConstructor) { function.initialize(inputInspectors.toArray) } else { val rowOI = ObjectInspectorFactory.getStandardStructObjectInspector( children.zipWithIndex.map(e => s"_col${e._2}").asJava, inputInspectors.asJava) function.initialize(rowOI) } }

wangyum · 2020-12-09T13:35:30Z

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

+      val num =
+        sql("SELECT udtf_stack2(2, 'A', 10, date '2015-01-01', 'B', 20, date '2016-01-01')").count()
+      assert(num === 2)


How about?

checkAnswer( sql("SELECT udtf_stack2(2, 'A', 10, date '2015-01-01', 'B', 20, date '2016-01-01')"), Seq(Row("A", 10, java.sql.Date.valueOf("2015-01-01")), Row("B", 20, java.sql.Date.valueOf("2016-01-01"))))

Could you update test name?

southernriver · 2020-12-10T02:31:21Z

@wangyum Thanks a lot, that's good catch. I'll do some fix according to your suggestion.

wangyum · 2020-12-30T13:45:47Z

@southernriver Any update?

sunchao

I think this is trying to do the same thing as #29490

sunchao · 2020-12-30T19:32:55Z

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

+              // Force it to check input data types.
+              udfExpr.get.asInstanceOf[HiveGenericUDTF].elementSchema
+            } catch {
+              case exception: Exception =>


why do we need this fallback mechanism? can we just switch to the new API without the deprecated flag?

wangyum · 2021-01-01T04:06:03Z

@southernriver is busy these days. Let's close this.

southernriver and others added 3 commits December 8, 2020 15:02

Suit the latest version of initialize(StructObjectInspector argOIs) f…

e78421d

…or HiveGenericUDTF

Reset IllegalStateException of udtf_stack2 to be normal

ea65856

fix style

0e3dca1

wangyum requested changes Dec 9, 2020

View reviewed changes

wangyum reviewed Dec 9, 2020

View reviewed changes

sunchao reviewed Dec 30, 2020

View reviewed changes

sunchao mentioned this pull request Dec 31, 2020

[SPARK-32668][SQL] HiveGenericUDTF initialize UDTF should use StructObjectInspector method #29490

Closed

wangyum closed this Jan 1, 2021

Conversation

southernriver commented Dec 8, 2020

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!

HyukjinKwon commented Dec 9, 2020

Uh oh!

HyukjinKwon commented Dec 9, 2020

Uh oh!

SparkQA commented Dec 9, 2020

Uh oh!

SparkQA commented Dec 9, 2020

Uh oh!

SparkQA commented Dec 9, 2020

Uh oh!

wangyum commented Dec 9, 2020

Uh oh!

wangyum left a comment

Choose a reason for hiding this comment

Uh oh!

wangyum Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

wangyum Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

wangyum Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

wangyum Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

wangyum Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

southernriver commented Dec 10, 2020

Uh oh!

wangyum commented Dec 30, 2020

Uh oh!

sunchao left a comment

Choose a reason for hiding this comment

Uh oh!

sunchao Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

wangyum commented Jan 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants