Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

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

wants to merge 4 commits into from

Conversation

@liancheng
Copy link
Contributor

@liancheng liancheng commented Nov 9, 2014

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

Review on Reviewable

@SparkQA
Copy link

SparkQA commented Nov 9, 2014

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

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 9, 2014

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
Copy link

AmplabJenkins commented Nov 9, 2014

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
marmbrus reviewed Nov 11, 2014
View reviewed changes
...hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala Outdated
Copy link
Contributor

@marmbrus marmbrus Nov 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@liancheng liancheng Nov 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@liancheng 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
@SparkQA
Copy link

SparkQA commented Nov 14, 2014

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

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 14, 2014

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
Copy link

AmplabJenkins commented Nov 14, 2014

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
marmbrus reviewed Nov 14, 2014
View reviewed changes
...hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala Outdated
Copy link
Contributor

@marmbrus marmbrus Nov 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@marmbrus marmbrus Nov 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@liancheng liancheng Nov 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

SparkQA commented Nov 15, 2014

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

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 15, 2014

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
Copy link

AmplabJenkins commented Nov 15, 2014

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
Copy link
Contributor Author

liancheng commented Nov 15, 2014

@marmbrus This should be good to go now.

@marmbrus
Copy link
Contributor

marmbrus commented Nov 16, 2014

Thanks! Merged to master and 1.2

@JoshRosen
Copy link
Contributor

JoshRosen commented Nov 16, 2014

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
Copy link
Contributor

JoshRosen commented Nov 16, 2014

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

marmbrus
marmbrus reviewed Nov 16, 2014
View reviewed changes
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveContext.scala
Copy link
Contributor

@marmbrus marmbrus Nov 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {

Copy link
Contributor Author

@liancheng liancheng Nov 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

asfgit pushed a commit that referenced this pull request Nov 16, 2014
@liancheng @marmbrus
[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, an…
8b83a34 
…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.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3178)
<!-- Reviewable:end -->

Author: Cheng Lian <[email protected]>

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 <[email protected]>
@asfgit asfgit closed this in cb6bd83 Nov 16, 2014
@liancheng liancheng mentioned this pull request Nov 17, 2014
Closed
@liancheng
Copy link
Contributor Author

liancheng commented Nov 17, 2014

Replaced this one with #3298.

asfgit pushed a commit that referenced this pull request Nov 18, 2014
@liancheng @marmbrus
[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, an…
6b7f2f7 
…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.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3298)
<!-- Reviewable:end -->

Author: Cheng Lian <[email protected]>

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)
asfgit pushed a commit that referenced this pull request Nov 18, 2014
@liancheng @marmbrus
[SPARK-4309][SPARK-4407][SQL] Date type support for Thrift server, an…
ff2fe56 
…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.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/3298)
<!-- Reviewable:end -->

Author: Cheng Lian <[email protected]>

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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@liancheng @SparkQA @AmplabJenkins @marmbrus @JoshRosen