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

[chenghao-intel]

No description provided.

[SparkQA]

Test build #22941 has started for PR 3114 at commit f54f369.

• This patch merges cleanly.

[SparkQA]

Test build #22941 has finished for PR 3114 at commit f54f369.

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

[AmplabJenkins]

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

[SparkQA]

Test build #22978 has started for PR 3114 at commit 50a13ba.

• This patch merges cleanly.

[SparkQA]

Test build #22978 has finished for PR 3114 at commit 50a13ba.

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

[AmplabJenkins]

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

[chenghao-intel]

@marmbrus @rxin @liancheng Couple of PRs are (#3109, #2802) blocked by this, can you review this?

[marmbrus]

Nit: why remove this to only have to add a more verbose .asInstanceOf below?

[chenghao-intel]

We don't bind a type here, just to keep consistent with pattern match above, and it is also helpful in debugging, you know, sometimes we just confused what type for catalyst array (Array or Seq), the .asInstanceOf will be a strong check.

[marmbrus]

Hey, thanks for working on this. I'm a little confused about how we are handling nulls though. Am I missing something?

[SparkQA]

Test build #23101 has started for PR 3114 at commit e603bda.

• This patch merges cleanly.

[chenghao-intel]

Thank you @marmbrus for reviewing this.
All of primitive constant null values (Literal(null, _) actually will not find a match currently, and it will resort to a normal Primitive ObjectInspector, this won't bring the incorrect result in most of the cases. But some Hive UDFs require the its arguments are ConstantObjectInspector (e.g. #3109, named_struct), which will cause error.
On the other hand the ConstantObjectInspector is definitely a way for optimization, we can avoid casting a constant catalyst object to Hive one(wrap) when calling the Hive UDFs.
I am planning to do this with couple of follow up PRs for the improvement.

[SparkQA]

Test build #23101 has finished for PR 3114 at commit e603bda.

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

[AmplabJenkins]

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

[liancheng]

@marmbrus The problem @chenghao-intel pointed out can be illustrated by this Scala shell snippet:

scala> def foo(v: Any) = v match {
     |   case _: String => "string"
     |   case _ => "other"
     | }
foo: (v: Any)String

scala> val s: String = null
s: String = null

scala> foo(s)
res0: String = other

That's why type annotations in toInspector are removed.

[marmbrus]

Ah, thanks for the explanation!

[marmbrus]

Thanks! Merged to master and 1.2.

[chenghao-intel]

@marmbrus , but this PR seems still open, can you double-check if it's merged?

[marmbrus]

Github was lagging you can always check here: https://git-wip-us.apache.org/repos/asf?p=spark.git;a=summary