-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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-13898][SQL] Merge DatasetHolder and DataFrameHolder #11737
Conversation
Test build #53211 has finished for PR 11737 at commit
|
Test build #53210 has finished for PR 11737 at commit
|
Test build #53215 has finished for PR 11737 at commit
|
Test build #53272 has finished for PR 11737 at commit
|
Test build #53394 has finished for PR 11737 at commit
|
@jodersky there are still two failures here. I probably won't have time to look at it until next week. If you have some time, please go for it! I think some are legitimate bugs in dataset encoders exposed by this change. |
cc @cloud-fan can you take a look at the failure for failing to serialize? The exception is
After changing the toString of GetStructField to the following:
A different exception appears:
|
@@ -729,7 +729,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton { | |||
} | |||
|
|||
test("SPARK-5203 union with different decimal precision") { | |||
Seq.empty[(Decimal, Decimal)] | |||
Seq.empty[(java.math.BigDecimal, java.math.BigDecimal)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all these decimal related changes because merging DataFrameHolder
and DatasetHolder
breaks compilation, or just because Decimal
is internal type and shouldn't be exposed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decimal's an internal type. (although I think we should expose it in the future -- but we haven't done that yet)
PR #11816, which fixes this exception you hit, has been merged. |
Test build #53687 has finished for PR 11737 at commit
|
Test build #53705 has finished for PR 11737 at commit
|
LGTM |
Merging in master. |
Test build #2655 has finished for PR 11737 at commit
|
## What changes were proposed in this pull request? This patch merges DatasetHolder and DataFrameHolder. This makes more sense because DataFrame/Dataset are now one class. In addition, fixed some minor issues with pull request apache#11732. ## How was this patch tested? Updated existing unit tests that test these implicits. Author: Reynold Xin <rxin@databricks.com> Closes apache#11737 from rxin/SPARK-13898.
What changes were proposed in this pull request?
This patch merges DatasetHolder and DataFrameHolder. This makes more sense because DataFrame/Dataset are now one class.
In addition, fixed some minor issues with pull request #11732.
How was this patch tested?
Updated existing unit tests that test these implicits.