[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, and fixes for complex types #3178

liancheng · 2014-11-09T16:41:36Z

SPARK-4407 was detected while working on SPARK-4309. Merged these two into a single PR since 1.2.0 RC is approaching.

SparkQA · 2014-11-09T16:44:59Z

Test build #23122 has started for PR 3178 at commit 313248c.

This patch merges cleanly.

SparkQA · 2014-11-09T17:55:38Z

Test build #23122 has finished for PR 3178 at commit 313248c.

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

AmplabJenkins · 2014-11-09T17:55:41Z

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

marmbrus · 2014-11-11T00:58:56Z

...hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala

-        to += from.get(ordinal).asInstanceOf[String]
+        to +=  from.getAs[Timestamp](ordinal)
+      case StringType | BinaryType | _: ArrayType | _: StructType | _: MapType =>
+        to += from.getAs[String](ordinal)


Does this work? Won't this throw a class cast exception for non-string types?

My fault, neither the original nor the modified code work, should notice this before merging #2685. Will provide a fix for this in this PR soon.

SparkQA · 2014-11-14T17:54:56Z

Test build #23376 has started for PR 3178 at commit 7836f88.

This patch merges cleanly.

SparkQA · 2014-11-14T19:12:11Z

Test build #23376 has finished for PR 3178 at commit 7836f88.

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

AmplabJenkins · 2014-11-14T19:12:14Z

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

marmbrus · 2014-11-14T23:26:14Z

...hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala

+          .queryExecution
+          .asInstanceOf[HiveContext#QueryExecution]
+          .toHiveString(from.get(ordinal) -> dataTypes(ordinal))
+        to += hiveString


Constructing a query execution for each column/row seems kind of expensive. Can we instead just pull toHiveString out into a static function?

Also why are we only doing this for Hive 13 and not Hive 12?

We've already done this for Hive 12, however toHiveString wasn't called back in #2685. Making toHiveString static sounds good.

SparkQA · 2014-11-15T04:40:06Z

Test build #23409 has started for PR 3178 at commit 6f71d0b.

This patch merges cleanly.

SparkQA · 2014-11-15T05:55:19Z

Test build #23409 has finished for PR 3178 at commit 6f71d0b.

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

AmplabJenkins · 2014-11-15T05:55:22Z

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

liancheng · 2014-11-15T06:45:03Z

@marmbrus This should be good to go now.

marmbrus · 2014-11-16T22:27:54Z

Thanks! Merged to master and 1.2

JoshRosen · 2014-11-16T22:45:56Z

Looks like this broke one of the Maven builds: https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-SBT/AMPLAB_JENKINS_BUILD_PROFILE=hadoop1.0,label=centos/1061/

JoshRosen · 2014-11-16T22:46:13Z

Err, SBT builds, with the hadoop1.0 profile.

marmbrus · 2014-11-16T22:58:43Z

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

@@ -147,7 +138,7 @@ class HiveContext(sc: SparkContext) extends SQLContext(sc) {
        // countFileSize to count the table size.
        def calculateTableSize(fs: FileSystem, path: Path): Long = {
          val fileStatus = fs.getFileStatus(path)
-          val size = if (fileStatus.isDir) {
+          val size = if (fileStatus.isDirectory) {


Actually we can't do this:

[ERROR] /home/jenkins/workspace/Spark-Master-Maven-pre-YARN/hadoop.version/1.0.4/label/centos/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveContext.scala:141: value isDirectory is not a member of org.apache.hadoop.fs.FileStatus [ERROR] val size = if (fileStatus.isDirectory) {

Ah, sorry for this, will open another PR and revert this change.

…d fixes for complex types SPARK-4407 was detected while working on SPARK-4309. Merged these two into a single PR since 1.2.0 RC is approaching.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3178)  Author: Cheng Lian <lian@databricks.com> Closes #3178 from liancheng/date-for-thriftserver and squashes the following commits: 6f71d0b [Cheng Lian] Makes toHiveString static 26fa955 [Cheng Lian] Fixes complex type support in Hive 0.13.1 shim a92882a [Cheng Lian] Updates HiveShim for 0.13.1 73f442b [Cheng Lian] Adds Date support for HiveThriftServer2 (Hive 0.12.0) (cherry picked from commit cb6bd83) Signed-off-by: Michael Armbrust <michael@databricks.com>

liancheng · 2014-11-17T02:36:20Z

Replaced this one with #3298.

…d fixes for complex types This PR is exactly the same as #3178 except it reverts the `FileStatus.isDir` to `FileStatus.isDirectory` change, since it doesn't compile with Hadoop 1.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3298)  Author: Cheng Lian <lian@databricks.com> Closes #3298 from liancheng/date-for-thriftserver and squashes the following commits: 866037e [Cheng Lian] Revers isDirectory to isDir (it breaks Hadoop 1 profile) 6f71d0b [Cheng Lian] Makes toHiveString static 26fa955 [Cheng Lian] Fixes complex type support in Hive 0.13.1 shim a92882a [Cheng Lian] Updates HiveShim for 0.13.1 73f442b [Cheng Lian] Adds Date support for HiveThriftServer2 (Hive 0.12.0)

…d fixes for complex types This PR is exactly the same as #3178 except it reverts the `FileStatus.isDir` to `FileStatus.isDirectory` change, since it doesn't compile with Hadoop 1.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3298)  Author: Cheng Lian <lian@databricks.com> Closes #3298 from liancheng/date-for-thriftserver and squashes the following commits: 866037e [Cheng Lian] Revers isDirectory to isDir (it breaks Hadoop 1 profile) 6f71d0b [Cheng Lian] Makes toHiveString static 26fa955 [Cheng Lian] Fixes complex type support in Hive 0.13.1 shim a92882a [Cheng Lian] Updates HiveShim for 0.13.1 73f442b [Cheng Lian] Adds Date support for HiveThriftServer2 (Hive 0.12.0) (cherry picked from commit 6b7f2f7) Signed-off-by: Michael Armbrust <michael@databricks.com>

marmbrus reviewed Nov 11, 2014
View reviewed changes

liancheng changed the title ~~[SPARK-4309][SQL] Date type support for Thrift server~~ [SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, and fixes for complex types Nov 14, 2014

marmbrus reviewed Nov 14, 2014
View reviewed changes

liancheng added 4 commits November 15, 2014 08:09

Adds Date support for HiveThriftServer2 (Hive 0.12.0)

73f442b

Updates HiveShim for 0.13.1

a92882a

Fixes complex type support in Hive 0.13.1 shim

26fa955

Makes toHiveString static

6f71d0b

liancheng force-pushed the date-for-thriftserver branch from 7836f88 to 6f71d0b Compare November 15, 2014 04:35

marmbrus reviewed Nov 16, 2014
View reviewed changes

asfgit closed this in cb6bd83 Nov 16, 2014

liancheng mentioned this pull request Nov 17, 2014

[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, and fixes for complex types #3298

Closed

[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, and fixes for complex types #3178

[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, and fixes for complex types #3178

Uh oh!

Conversation

liancheng commented Nov 9, 2014

Uh oh!

SparkQA commented Nov 9, 2014

Uh oh!

SparkQA commented Nov 9, 2014

Uh oh!

AmplabJenkins commented Nov 9, 2014

Uh oh!

marmbrus Nov 11, 2014

Choose a reason for hiding this comment

Uh oh!

liancheng Nov 14, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 14, 2014

Uh oh!

SparkQA commented Nov 14, 2014

Uh oh!

AmplabJenkins commented Nov 14, 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!

liancheng Nov 14, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 15, 2014

Uh oh!

SparkQA commented Nov 15, 2014

Uh oh!

AmplabJenkins commented Nov 15, 2014

Uh oh!

liancheng commented Nov 15, 2014

Uh oh!

marmbrus commented Nov 16, 2014

Uh oh!

JoshRosen commented Nov 16, 2014

Uh oh!

JoshRosen commented Nov 16, 2014

Uh oh!

marmbrus Nov 16, 2014

Choose a reason for hiding this comment

Uh oh!

liancheng Nov 17, 2014

Choose a reason for hiding this comment

Uh oh!

liancheng commented Nov 17, 2014

Uh oh!

Uh oh!