Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Aug 4, 2020

What changes were proposed in this pull request?

This PR aims to clean up InMemoryRelation.ser in CachedBatchSerializerSuite.

Why are the changes needed?

SPARK-32274 makes SQL cache serialization pluggable.

[SPARK-32274][SQL] Make SQL cache serialization pluggable

This causes UT failures.

$ build/sbt "sql/testOnly *.CachedBatchSerializerSuite *.CachedTableSuite"
...
[info]   Cause: java.lang.IllegalStateException: This does not work. This is only for testing
[info]   at org.apache.spark.sql.execution.columnar.TestSingleIntColumnarCachedBatchSerializer.convertInternalRowToCachedBatch(CachedBatchSerializerSuite.scala:49)
...
[info] *** 30 TESTS FAILED ***
[error] Failed: Total 51, Failed 30, Errors 0, Passed 21
[error] Failed tests:
[error] 	org.apache.spark.sql.CachedTableSuite
[error] (sql/test:testOnly) sbt.TestsFailedException: Tests unsuccessful

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually.

$ build/sbt "sql/testOnly *.CachedBatchSerializerSuite *.CachedTableSuite"
[info] Tests: succeeded 51, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[info] Passed: Total 51, Failed 0, Errors 0, Passed 51

@dongjoon-hyun
Copy link
Member Author

cc @cloud-fan, @HyukjinKwon , @revans2 , @tgravescs
This is the second approach according to the review comments.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM. I prefer this way but don't feel strongly if other people prefer the other way.

@dongjoon-hyun
Copy link
Member Author

Thank you for reiview and approval, @HyukjinKwon .

@dongjoon-hyun
Copy link
Member Author

Thanks, @cloud-fan . beforeAll is also added.

@SparkQA
Copy link

SparkQA commented Aug 4, 2020

Test build #127030 has finished for PR 29346 at commit a552287.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 4, 2020

Test build #127033 has finished for PR 29346 at commit 71a75a0.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 4, 2020

Test build #127032 has finished for PR 29346 at commit 57c0797.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@HyukjinKwon
Copy link
Member

I am going to merge this since GitHub Actions builds passed.

@SparkQA
Copy link

SparkQA commented Aug 4, 2020

Test build #127044 has finished for PR 29346 at commit 71a75a0.

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

@dongjoon-hyun
Copy link
Member Author

Thank you, @cloud-fan and @HyukjinKwon .

@dongjoon-hyun dongjoon-hyun deleted the SPARK-32524-3 branch August 4, 2020 14:01
@revans2
Copy link
Contributor

revans2 commented Aug 4, 2020

Thanks for finding and fixing this for me.

@dongjoon-hyun
Copy link
Member Author

Thanks, @revans2 ! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants