Skip to content
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

[SPARK-6996][SQL] Support map types in java beans #5578

Closed
wants to merge 3 commits into from
Closed

[SPARK-6996][SQL] Support map types in java beans #5578

wants to merge 3 commits into from

Conversation

punya
Copy link
Contributor

@punya punya commented Apr 19, 2015

@liancheng @mengxr this is similar to #5146.

@SparkQA
Copy link

SparkQA commented Apr 19, 2015

Test build #30552 has finished for PR 5578 at commit 96a73a1.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@punya punya changed the title [WIP] [SPARK-6996][SQL] Support generic collection types in java beans [SPARK-6996][SQL] Support generic collection types in java beans Apr 19, 2015
@SparkQA
Copy link

SparkQA commented Apr 19, 2015

Test build #30553 has finished for PR 5578 at commit 976f3ec.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 19, 2015

Test build #30554 has finished for PR 5578 at commit 0ebda07.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@punya punya changed the title [SPARK-6996][SQL] Support generic collection types in java beans [SPARK-6996][SQL] Support map types in java beans Apr 19, 2015
@SparkQA
Copy link

SparkQA commented Apr 19, 2015

Test build #30555 has finished for PR 5578 at commit ebd3c82.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 19, 2015

Test build #30557 has finished for PR 5578 at commit 469e7b2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

marmbrus
marmbrus reviewed Apr 21, 2015
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala
val (dataType, nullable) = inferDataType(typeToken.getComponentType)
(ArrayType(dataType, nullable), true)

case _ if iterableType.isAssignableFrom(typeToken) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Spark style avoids { } for case statements.

@marmbrus
Copy link
Contributor

Thanks for working on this. I did not know this was possible given erasure! A few minor comments, otherwise LGTM.

@punya
Copy link
Contributor Author

punya commented Apr 21, 2015

@marmbrus relative to my earlier commit, I removed the code to handle Java iterables as ArrayType because I couldn't get it to work at runtime. I think it's Catalyst-related and my knowledge isn't deep enough to figure out what's going on.

@marmbrus
Copy link
Contributor

Have you looked in CatalystConverters? There is handler for JMap, but
maybe not for JIterable?
On Apr 20, 2015 7:34 PM, "Punyashloka Biswal" [email protected]
wrote:

@marmbrus https://github.com/marmbrus relative to my earlier commit, I
removed the code to handle Java iterables as ArrayType because I couldn't
get it to work at runtime. I think it's Catalyst-related and my knowledge
isn't deep enough to figure out what's going on.

—
Reply to this email directly or view it on GitHub
#5578 (comment).

@punya
Copy link
Contributor Author

punya commented Apr 21, 2015

Thanks, CatalystTypeConverters was exactly what I needed to look at!

@SparkQA
Copy link

SparkQA commented Apr 21, 2015

Test build #30622 has finished for PR 5578 at commit 5e00685.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 21, 2015

Test build #30624 has finished for PR 5578 at commit d0d5683.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 21, 2015

Test build #30625 has finished for PR 5578 at commit c7e308b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 21, 2015

Test build #30627 has finished for PR 5578 at commit d56c3e0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@marmbrus
Copy link
Contributor

Thanks! Merged to master.

@asfgit asfgit closed this in 2a24bf9 Apr 21, 2015
@punya punya deleted the feature/SPARK-6996 branch April 22, 2015 01:55
@rayortigas rayortigas mentioned this pull request Apr 27, 2015
Closed
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@nemccarthy
[SPARK-6996][SQL] Support map types in java beans
11a5a9b 
liancheng mengxr this is similar to apache#5146.

Author: Punya Biswal <[email protected]>

Closes apache#5578 from punya/feature/SPARK-6996 and squashes the following commits:

d56c3e0 [Punya Biswal] Fix imports
c7e308b [Punya Biswal] Support java iterable types in POJOs
5e00685 [Punya Biswal] Support map types in java beans
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.
3 participants
@punya @SparkQA @marmbrus