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-20125][SQL] Dataset of type option of map does not work #17454

Closed
wants to merge 1 commit into from

Conversation

cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

When we build the deserializer expression for map type, we will use StaticInvoke to call ArrayBasedMapData.toScalaMap, and declare the return type as scala.collection.immutable.Map. If the map is inside an Option, we will wrap this StaticInvoke with WrapOption, which requires the input to be scala.collect.Map. Ideally this should be fine, as scala.collection.immutable.Map extends scala.collect.Map, but our ObjectType is too strict about this, this PR fixes it.

How was this patch tested?

new regression test

@cloud-fan
Copy link
Contributor Author

cc @liancheng

@SparkQA
Copy link

SparkQA commented Mar 28, 2017

Test build #75307 has finished for PR 17454 at commit 29a0143.

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

@liancheng
Copy link
Contributor

LGTM, merging to master and branch-2.1. Thanks!

@asfgit asfgit closed this in d4fac41 Mar 28, 2017
@liancheng
Copy link
Contributor

This PR conflicts with branch-2.1, trying to resolve manually while merging.

asfgit pushed a commit that referenced this pull request Mar 28, 2017
When we build the deserializer expression for map type, we will use `StaticInvoke` to call `ArrayBasedMapData.toScalaMap`, and declare the return type as `scala.collection.immutable.Map`. If the map is inside an Option, we will wrap this `StaticInvoke` with `WrapOption`, which requires the input to be `scala.collect.Map`. Ideally this should be fine, as `scala.collection.immutable.Map` extends `scala.collect.Map`, but our `ObjectType` is too strict about this, this PR fixes it.

new regression test

Author: Wenchen Fan <wenchen@databricks.com>

Closes #17454 from cloud-fan/map.

(cherry picked from commit d4fac41)
Signed-off-by: Cheng Lian <lian@databricks.com>
@liancheng
Copy link
Contributor

OK, resolved the conflict manually and merged to branch-2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants