[SPARK-4244] [SQL] Support Hive Generic UDFs with constant object inspector parameters #3109

chenghao-intel · 2014-11-05T08:49:18Z

Query SELECT named_struct(lower("AA"), "12", lower("Bb"), "13") FROM src LIMIT 1 will throw exception, some of the Hive Generic UDF/UDAF requires the input object inspector is ConstantObjectInspector, however, we won't get that before the expression optimization executed. (Constant Folding).

This PR is a work around to fix this. (As ideally, the output of LogicalPlan should be identical before and after Optimization).

SparkQA · 2014-11-05T08:54:57Z

Test build #22926 has started for PR 3109 at commit a790467.

This patch merges cleanly.

SparkQA · 2014-11-05T09:30:53Z

Test build #22926 has finished for PR 3109 at commit a790467.

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

AmplabJenkins · 2014-11-05T09:30:56Z

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

chenghao-intel · 2014-11-05T16:36:28Z

The failure should be fixed once #3114 merged.

SparkQA · 2014-11-05T16:50:04Z

Test build #22942 has started for PR 3109 at commit 7b0a080.

This patch merges cleanly.

SparkQA · 2014-11-05T17:25:52Z

Test build #22942 has finished for PR 3109 at commit 7b0a080.

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

AmplabJenkins · 2014-11-05T17:25:54Z

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

chenghao-intel · 2014-11-11T01:53:30Z

test this please

SparkQA · 2014-11-11T02:00:09Z

Test build #23186 has started for PR 3109 at commit 7b0a080.

This patch merges cleanly.

SparkQA · 2014-11-11T03:15:43Z

Test build #23186 has finished for PR 3109 at commit 7b0a080.

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

AmplabJenkins · 2014-11-11T03:15:46Z

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

marmbrus · 2014-11-14T21:58:57Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

@@ -262,7 +262,7 @@ object NullPropagation extends Rule[LogicalPlan] {
 */
 object ConstantFolding extends Rule[LogicalPlan] {
  def apply(plan: LogicalPlan): LogicalPlan = plan transform {
-    case q: LogicalPlan => q transformExpressionsDown {
+    case q: LogicalPlan => q transformExpressionsUp {


Why this change? It seem like this is strictly less efficient because it will call evaluate for each subtree, constructing literals each time instead of doing the evaluation in a single pass.

Ah, sorry now I read the PR description more carefully and I think I understand. However, this now seems kind of hacky to me. We are now leaving the analysis phase with a tree that isn't actually valid and then hoping that constant folding is going to fix things down the road. This breaks the contract that it should be safe to inspect dataTypes once analysis is done.

Is this query valid in Hive? If so, I'd almost rather add more rules/logic to analysis (maybe we need to have a notion of "resolved" for HiveUDFs and attempt to constant fold them if they aren't resolved? What do you think?

Yes, this is valid in Hive. And you're right, I will try your suggestion, it seems more reasonable.

SparkQA · 2014-11-17T05:45:02Z

Test build #23458 has started for PR 3109 at commit 2710d59.

This patch does not merge cleanly.

SparkQA · 2014-11-17T06:27:33Z

Test build #23460 has started for PR 3109 at commit 487ff79.

This patch merges cleanly.

SparkQA · 2014-11-17T07:03:38Z

Test build #23460 has finished for PR 3109 at commit 487ff79.

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

AmplabJenkins · 2014-11-17T07:03:41Z

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

SparkQA · 2014-11-17T07:09:32Z

Test build #23458 has finished for PR 3109 at commit 2710d59.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-17T07:09:35Z

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

chenghao-intel · 2014-11-17T08:54:52Z

The test failure depends on #3308, should be OK after #3308 merged.

chenghao-intel · 2014-11-18T01:07:02Z

test this please

SparkQA · 2014-11-18T01:10:21Z

Test build #23522 has started for PR 3109 at commit 487ff79.

This patch merges cleanly.

SparkQA · 2014-11-18T02:21:42Z

Test build #23522 has finished for PR 3109 at commit 487ff79.

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

AmplabJenkins · 2014-11-18T02:21:45Z

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

marmbrus · 2014-11-20T23:57:22Z

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

@@ -298,6 +298,8 @@ private[hive] trait HiveInspectors {
        })
        ObjectInspectorFactory.getStandardConstantMapObjectInspector(keyOI, valueOI, map)
      }
+    case Literal(_, dt) => sys.error(s"Hive doesn't support the constant type [$dt].")


Do we really want to throw an error here? Why not just skip creating a constant object inspector?

We should enumerate all of the possible constant data type in this function, this actually gives us a chance to check if we really missed one, just as previously, we did miss all of the constant type by specifying data type in matching (see #3114)

marmbrus · 2014-11-21T00:51:40Z

Thanks for explaining. Merged to master and 1.2.

…pector parameters Query `SELECT named_struct(lower("AA"), "12", lower("Bb"), "13") FROM src LIMIT 1` will throw exception, some of the Hive Generic UDF/UDAF requires the input object inspector is `ConstantObjectInspector`, however, we won't get that before the expression optimization executed. (Constant Folding). This PR is a work around to fix this. (As ideally, the `output` of LogicalPlan should be identical before and after Optimization). Author: Cheng Hao <[email protected]> Closes apache#3109 from chenghao-intel/optimized and squashes the following commits: 487ff79 [Cheng Hao] rebase to the latest master & update the unittest (cherry picked from commit 84d79ee) Signed-off-by: Michael Armbrust <[email protected]>

chenghao-intel · 2014-11-24T00:41:03Z

I am closing it since it's already merged.

chenghao-intel mentioned this pull request Nov 5, 2014

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

Closed

chenghao-intel mentioned this pull request Nov 7, 2014

[SPARK-4250] [SQL] Fix bug of constant null value mapping to ConstantObjectInspector #3114

Closed

marmbrus reviewed Nov 14, 2014
View reviewed changes

rebase to the latest master & update the unittest

487ff79

chenghao-intel force-pushed the optimized branch from 2710d59 to 487ff79 Compare November 17, 2014 06:26

chenghao-intel changed the title ~~[SPARK-4244] [SQL] Workaround for Hive Generic UDFs with constant object inspector parameter~~ [SPARK-4244] [SQL] Support Hive Generic UDFs with constant object inspector parameters Nov 17, 2014

marmbrus reviewed Nov 20, 2014
View reviewed changes

chenghao-intel closed this Nov 24, 2014

[SPARK-4244] [SQL] Support Hive Generic UDFs with constant object inspector parameters #3109

[SPARK-4244] [SQL] Support Hive Generic UDFs with constant object inspector parameters #3109

Uh oh!

Conversation

chenghao-intel commented Nov 5, 2014

Uh oh!

SparkQA commented Nov 5, 2014

Uh oh!

SparkQA commented Nov 5, 2014

Uh oh!

AmplabJenkins commented Nov 5, 2014

Uh oh!

chenghao-intel commented Nov 5, 2014

Uh oh!

SparkQA commented Nov 5, 2014

Uh oh!

SparkQA commented Nov 5, 2014

Uh oh!

AmplabJenkins commented Nov 5, 2014

Uh oh!

chenghao-intel commented Nov 11, 2014

Uh oh!

SparkQA commented Nov 11, 2014

Uh oh!

SparkQA commented Nov 11, 2014

Uh oh!

AmplabJenkins commented Nov 11, 2014

Uh oh!

marmbrus Nov 14, 2014

Choose a reason for hiding this comment

Uh oh!

marmbrus Nov 14, 2014

Choose a reason for hiding this comment

Uh oh!

chenghao-intel Nov 17, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 17, 2014

Uh oh!

SparkQA commented Nov 17, 2014

Uh oh!

SparkQA commented Nov 17, 2014

Uh oh!

AmplabJenkins commented Nov 17, 2014

Uh oh!

SparkQA commented Nov 17, 2014

Uh oh!

AmplabJenkins commented Nov 17, 2014

Uh oh!

chenghao-intel commented Nov 17, 2014

Uh oh!

chenghao-intel commented Nov 18, 2014

Uh oh!

SparkQA commented Nov 18, 2014

Uh oh!

SparkQA commented Nov 18, 2014

Uh oh!

AmplabJenkins commented Nov 18, 2014

Uh oh!

marmbrus Nov 20, 2014

Choose a reason for hiding this comment

Uh oh!

chenghao-intel Nov 21, 2014

Choose a reason for hiding this comment

Uh oh!

marmbrus commented Nov 21, 2014

Uh oh!

chenghao-intel commented Nov 24, 2014

Uh oh!

Uh oh!