Skip to content

[SPARK-11727][SQL] Split ExpressionEncoder into FlatEncoder and ProductEncoder #9693

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

Closed
wants to merge 1 commit into from

Conversation

cloud-fan
Copy link
Contributor

also add more tests for encoders, and fix bugs that I found:

  • when convert array to catalyst array, we can only skip element conversion for native types(e.g. int, long, boolean), not AtomicType(String is AtomicType but we need to convert it)
  • we should also handle scala BigDecimal when convert from catalyst Decimal.
  • complex map type should be supported

other issues that still in investigation:

  • encode java BigDecimal and decode it back, seems we will loss precision info.
  • when encode case class that defined inside a object, ClassNotFound exception will be thrown.

I'll remove unused code in a follow-up PR.

@cloud-fan
Copy link
Contributor Author

cc @marmbrus

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45863 has finished for PR 9693 at commit 7e4f7fe.

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

@marmbrus
Copy link
Contributor

when encode case class that defined inside a object, ClassNotFound exception will be thrown.

I have a fix for this, but I'm still working out some repl issues.

asfgit pushed a commit that referenced this pull request Nov 13, 2015
…ctEncoder

also add more tests for encoders, and fix bugs that I found:

* when convert array to catalyst array, we can only skip element conversion for native types(e.g. int, long, boolean), not `AtomicType`(String is AtomicType but we need to convert it)
* we should also handle scala `BigDecimal` when convert from catalyst `Decimal`.
* complex map type should be supported

other issues that still in investigation:

* encode java `BigDecimal` and decode it back, seems we will loss precision info.
* when encode case class that defined inside a object, `ClassNotFound` exception will be thrown.

I'll remove unused code in a follow-up PR.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9693 from cloud-fan/split.

(cherry picked from commit d7b2b97)
Signed-off-by: Michael Armbrust <michael@databricks.com>
@asfgit asfgit closed this in d7b2b97 Nov 13, 2015
@cloud-fan cloud-fan deleted the split branch November 14, 2015 00:04
asfgit pushed a commit that referenced this pull request Nov 19, 2015
After some experiment, I found it's not convenient to have separate encoder builders: `FlatEncoder` and `ProductEncoder`. For example, when create encoders for `ScalaUDF`, we have no idea if the type `T` is flat or not. So I revert the splitting change in #9693, while still keeping the bug fixes and tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9726 from cloud-fan/follow.

(cherry picked from commit 47d1c23)
Signed-off-by: Michael Armbrust <michael@databricks.com>
asfgit pushed a commit that referenced this pull request Nov 19, 2015
After some experiment, I found it's not convenient to have separate encoder builders: `FlatEncoder` and `ProductEncoder`. For example, when create encoders for `ScalaUDF`, we have no idea if the type `T` is flat or not. So I revert the splitting change in #9693, while still keeping the bug fixes and tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9726 from cloud-fan/follow.
kiszk pushed a commit to kiszk/spark-gpu that referenced this pull request Dec 26, 2015
After some experiment, I found it's not convenient to have separate encoder builders: `FlatEncoder` and `ProductEncoder`. For example, when create encoders for `ScalaUDF`, we have no idea if the type `T` is flat or not. So I revert the splitting change in apache/spark#9693, while still keeping the bug fixes and tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9726 from cloud-fan/follow.
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