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.

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

[SPARK-17347][SQL][Examples]Encoder in Dataset example has incorrect type #14901

Closed
wants to merge 4 commits into from

Conversation

CodingCat
Copy link
Contributor

What changes were proposed in this pull request?

We propose to fix the Encoder type in the Dataset example

How was this patch tested?

The PR will be tested with the current unit test cases

@CodingCat CodingCat changed the title [SPARK-17347][Examples]Encoder in Dataset example is incorrect on type [SPARK-17347][SQL][Examples]Encoder in Dataset example is incorrect on type Aug 31, 2016
@CodingCat CodingCat changed the title [SPARK-17347][SQL][Examples]Encoder in Dataset example is incorrect on type [SPARK-17347][SQL][Examples]Encoder in Dataset example has incorrect type Aug 31, 2016
@SparkQA
Copy link

SparkQA commented Aug 31, 2016

Test build #64730 has finished for PR 14901 at commit 31a0cd7.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 31, 2016

Test build #64731 has finished for PR 14901 at commit 8ef776d.

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

@@ -203,7 +203,7 @@ object SparkSQLExample {
// No pre-defined encoders for Dataset[Map[K,V]], define explicitly
implicit val mapEncoder = org.apache.spark.sql.Encoders.kryo[Map[String, Any]]
// Primitive types and case classes can be also defined as
implicit val stringIntMapEncoder: Encoder[Map[String, Int]] = ExpressionEncoder()
// implicit val stringIntMapEncoder: Encoder[Map[String, Any]] = ExpressionEncoder()
Copy link
Member

Choose a reason for hiding this comment

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

This compiled before though, right? you're saying it's unnecessary because of the implicit in the line above? that seems fine but let's delete these 2 lines then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am saying that the comments (L205) seem to indicate that, besides L204, the encoder can be defined as L206 as well. Actually, L206 has the incorrect type (you can find that by removing L204 and compile with the original L206)

so, I fixed the type and comment the line for successful compilation

Copy link
Member

Choose a reason for hiding this comment

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

Got it, LGTM

@srowen
Copy link
Member

srowen commented Sep 3, 2016

merged to master/2.0

asfgit pushed a commit that referenced this pull request Sep 3, 2016
… type

## What changes were proposed in this pull request?

We propose to fix the Encoder type in the Dataset example

## How was this patch tested?

The PR will be tested with the current unit test cases

Author: CodingCat <zhunansjtu@gmail.com>

Closes #14901 from CodingCat/SPARK-17347.

(cherry picked from commit 97da410)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@asfgit asfgit closed this in 97da410 Sep 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants