[SPARK-3904] [SQL] add constant objectinspector support for udfs #2762

chenghao-intel · 2014-10-11T06:48:14Z

In HQL, we convert all of the data type into normal ObjectInspectors for UDFs, most of cases it works, however, some of the UDF actually requires its children ObjectInspector to be the ConstantObjectInspector, which will cause exception.
e.g.
select named_struct("x", "str") from src limit 1;

I updated the method wrap by adding the one more parameter ObjectInspector(to describe what it expects to wrap to, for example: java.lang.Integer or IntWritable).

As well as the unwrap method by providing the input ObjectInspector.

AmplabJenkins · 2014-10-11T06:52:05Z

Can one of the admins verify this patch?

chenghao-intel · 2014-10-11T06:53:03Z

test this please.

SparkQA · 2014-10-11T06:55:00Z

QA tests have started for PR 2762 at commit 06581e3.

This patch merges cleanly.

SparkQA · 2014-10-11T07:00:02Z

QA tests have started for PR 2762 at commit 06581e3.

This patch merges cleanly.

tianyi · 2014-10-11T07:06:21Z

...ompatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala

@@ -578,6 +578,7 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
    "multi_join_union",
    "multiMapJoin1",
    "multiMapJoin2",
+    "udf_named_struct",


I think you should put it after "udf_month"

Thanks, moved.

SparkQA · 2014-10-11T07:26:24Z

QA tests have finished for PR 2762 at commit 06581e3.

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

AmplabJenkins · 2014-10-11T07:26:27Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21637/Test FAILed.

SparkQA · 2014-10-11T07:30:52Z

QA tests have finished for PR 2762 at commit 06581e3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- protected case class Keyword(str: String)
- class SqlLexical(val keywords: Seq[String]) extends StdLexical
- case class FloatLit(chars: String) extends Token
- class SqlParser extends AbstractSparkSQLParser
- case class SetCommand(kv: Option[(String, Option[String])]) extends Command
- case class ShellCommand(cmd: String) extends Command
- case class SourceCommand(filePath: String) extends Command
- case class SetCommand(kv: Option[(String, Option[String])], output: Seq[Attribute])(

AmplabJenkins · 2014-10-11T07:30:55Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21638/Test FAILed.

SparkQA · 2014-10-11T12:34:38Z

QA tests have started for PR 2762 at commit 335a952.

This patch merges cleanly.

SparkQA · 2014-10-11T13:06:20Z

QA tests have finished for PR 2762 at commit 335a952.

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

AmplabJenkins · 2014-10-11T13:06:22Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21645/Test FAILed.

SparkQA · 2014-10-12T06:49:37Z

QA tests have started for PR 2762 at commit cb97576.

This patch merges cleanly.

SparkQA · 2014-10-12T07:34:09Z

QA tests have finished for PR 2762 at commit cb97576.

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

AmplabJenkins · 2014-10-12T07:34:12Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21661/
Test PASSed.

marmbrus · 2014-10-13T20:17:14Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypes.scala

-    val baseValue = child.eval(input).asInstanceOf[Row]
-    if (baseValue == null) null else baseValue(ordinal)
+    val baseValue = child.eval(input)
+    if (baseValue == null) null else baseValue.asInstanceOf[Row](ordinal)


Why this change?

chenghao-intel · 2014-10-14T14:41:15Z

Thanks, I've rebased the latest master, and solved the issues you guys raised.

chenghao-intel · 2014-10-15T01:19:15Z

test this please.

gvramana · 2014-10-15T07:01:16Z

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

+      deferedObjects(i).asInstanceOf[DeferredObjectAdapter].set(
+        () => {
+          children(idx).eval(input)
+        }, argumentInspectors(i))


argumentInspector can be set once during deferedObjects creation, rather than execution per row.

gvramana · 2014-10-15T07:32:21Z

unfortunately :( I also have worked and implemented the same as part of #2802
Anyways I will rework on my pull request for supporting UDAF and GenericUDAF, once your changes are merged.

chenghao-intel · 2014-10-17T07:09:12Z

Thank you @gvramana , I've updated the code as you suggested.

chenghao-intel · 2014-10-17T07:09:28Z

retest this please

SparkQA · 2014-10-20T00:26:04Z

QA tests have started for PR 2762 at commit 49d442b.

This patch merges cleanly.

SparkQA · 2014-10-20T01:16:34Z

QA tests have finished for PR 2762 at commit 49d442b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class DeferredObjectAdapter(oi: ObjectInspector) extends DeferredObject

tianyi · 2014-10-22T01:33:48Z

Is there any admin could review this PR ASAP? We got some PR blocked by this one. like #2542 @marmbrus @rxin

liancheng · 2014-10-28T06:55:07Z

test this please

SparkQA · 2014-10-28T06:57:33Z

Test build #22345 has started for PR 2762 at commit 782722f.

This patch merges cleanly.

SparkQA · 2014-10-28T07:41:57Z

Test build #22345 has finished for PR 2762 at commit 782722f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class DeferredObjectAdapter(oi: ObjectInspector) extends DeferredObject

AmplabJenkins · 2014-10-28T07:42:00Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22345/
Test FAILed.

chenghao-intel · 2014-10-28T08:56:56Z

It passed the unit test in my local with the master rebased. Seems weird.

chenghao-intel · 2014-10-28T08:57:12Z

retest this please. @liancheng :)

SparkQA · 2014-10-28T21:52:36Z

Test build #490 has started for PR 2762 at commit 782722f.

This patch merges cleanly.

marmbrus · 2014-10-28T22:04:55Z

Also, this needs to be updated again (I think it conflicted with your CTAS patch).

SparkQA · 2014-10-28T23:27:26Z

Test build #490 has finished for PR 2762 at commit 782722f.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class DeferredObjectAdapter(oi: ObjectInspector) extends DeferredObject

…the DeferredObject

chenghao-intel · 2014-10-29T00:35:04Z

Rebased!
test this please.

liancheng · 2014-10-29T00:49:18Z

test this please

liancheng · 2014-10-29T00:51:19Z

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

+    oi match {
+      case x: ConstantObjectInspector => x.getWritableConstantValue
+      case x: PrimitiveObjectInspector => a match {
+        // TODO what if x.preferWritable() == true? reuse the writable?


Seems that the Writable related code was reverted after rebase?

Yes, currently the oi is should not be "preferWritable" as toInspector doesn't return that. Even if we return an new instance of Writable here, it's the same as the preferWritable ObjectInspector does internally.
As you suggested we don't want to dynamically check the oi type, I will keep that for future improvement, and to reuse the writable object.

SparkQA · 2014-10-29T00:54:50Z

Test build #22404 has started for PR 2762 at commit bcacfd7.

This patch merges cleanly.

SparkQA · 2014-10-29T01:43:28Z

Test build #22404 has finished for PR 2762 at commit bcacfd7.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class DeferredObjectAdapter(oi: ObjectInspector) extends DeferredObject

AmplabJenkins · 2014-10-29T01:43:30Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22404/
Test PASSed.

marmbrus · 2014-10-29T02:11:19Z

Thanks for working on this. Merged to master.

tianyi reviewed Oct 11, 2014
View reviewed changes

chenghao-intel force-pushed the udf_coi branch from 335a952 to cb97576 Compare October 12, 2014 06:47

marmbrus reviewed Oct 13, 2014
View reviewed changes

chenghao-intel force-pushed the udf_coi branch from cb97576 to bda7b5a Compare October 14, 2014 14:40

gvramana reviewed Oct 15, 2014
View reviewed changes

chenghao-intel added 8 commits October 29, 2014 08:19

Passing ContantObjectInspector(when necessary) for UDF initializing

8814c3a

Support Constant ObjectInspector for Map & List

f6740fe

update HiveGenericUdf for set the ObjectInspector while constructing …

fe591e4

…the DeferredObject

Add golden files

683d3fd

style issue

4e56e1b

add space before while

5793c01

revert to hive 0.12

2416e5d

Shim for both Hive 0.12 & 0.13.1

bcacfd7

chenghao-intel force-pushed the udf_coi branch from 782722f to bcacfd7 Compare October 29, 2014 00:33

liancheng reviewed Oct 29, 2014
View reviewed changes

asfgit closed this in b5e79bf Oct 29, 2014

tianyi mentioned this pull request Oct 30, 2014

[SPARK-3688][SQL]LogicalPlan can't resolve column correctlly #2542

Closed

gvramana mentioned this pull request Nov 3, 2014

[SPARK-3891][SQL] Add array support to percentile, percentile_approx and constant inspectors support #2802

Closed

chenghao-intel deleted the udf_coi branch July 2, 2015 08:40

[SPARK-3904] [SQL] add constant objectinspector support for udfs #2762

[SPARK-3904] [SQL] add constant objectinspector support for udfs #2762

Conversation

chenghao-intel commented Oct 11, 2014

AmplabJenkins commented Oct 11, 2014

chenghao-intel commented Oct 11, 2014

SparkQA commented Oct 11, 2014

SparkQA commented Oct 11, 2014

tianyi Oct 11, 2014

Choose a reason for hiding this comment

chenghao-intel Oct 11, 2014

Choose a reason for hiding this comment

SparkQA commented Oct 11, 2014

AmplabJenkins commented Oct 11, 2014

SparkQA commented Oct 11, 2014

AmplabJenkins commented Oct 11, 2014

SparkQA commented Oct 11, 2014

SparkQA commented Oct 11, 2014

AmplabJenkins commented Oct 11, 2014

SparkQA commented Oct 12, 2014

SparkQA commented Oct 12, 2014

AmplabJenkins commented Oct 12, 2014

marmbrus Oct 13, 2014

Choose a reason for hiding this comment

chenghao-intel commented Oct 14, 2014

chenghao-intel commented Oct 15, 2014

gvramana Oct 15, 2014

Choose a reason for hiding this comment

gvramana commented Oct 15, 2014

chenghao-intel commented Oct 17, 2014

chenghao-intel commented Oct 17, 2014

SparkQA commented Oct 20, 2014

SparkQA commented Oct 20, 2014

tianyi commented Oct 22, 2014

liancheng commented Oct 28, 2014

SparkQA commented Oct 28, 2014

SparkQA commented Oct 28, 2014

AmplabJenkins commented Oct 28, 2014

chenghao-intel commented Oct 28, 2014

chenghao-intel commented Oct 28, 2014

SparkQA commented Oct 28, 2014

marmbrus commented Oct 28, 2014

SparkQA commented Oct 28, 2014

chenghao-intel commented Oct 29, 2014

liancheng commented Oct 29, 2014

liancheng Oct 29, 2014

Choose a reason for hiding this comment

chenghao-intel Oct 29, 2014

Choose a reason for hiding this comment

SparkQA commented Oct 29, 2014

SparkQA commented Oct 29, 2014

AmplabJenkins commented Oct 29, 2014

marmbrus commented Oct 29, 2014