Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Sep 19, 2018

What changes were proposed in this pull request?

This goes to revert SPARK-19355 and SPARK-25352, based on some discussion and comments at #16677 (comment).

How was this patch tested?

Existing tests.

@viirya
Copy link
Member Author

viirya commented Sep 19, 2018

cc @cloud-fan @rxin @hvanhovell

@HyukjinKwon
Copy link
Member

@viirya, not a big deal but mind leaving few explicit discussion links (in the PRs you pointed out) to the current PR description?

@maropu
Copy link
Member

maropu commented Sep 19, 2018

I just put the link here: #16677 (comment)

@viirya
Copy link
Member Author

viirya commented Sep 19, 2018

Thanks @HyukjinKwon @maropu. I've put it in the PR description.

@SparkQA
Copy link

SparkQA commented Sep 19, 2018

Test build #96240 has finished for PR 22464 at commit 8ee721c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • trait BaseLimitExec extends UnaryExecNode with CodegenSupport
  • case class LocalLimitExec(limit: Int, child: SparkPlan) extends BaseLimitExec
  • case class GlobalLimitExec(limit: Int, child: SparkPlan) extends BaseLimitExec

@cloud-fan
Copy link
Contributor

@viirya Thanks for doing it! To ease the review, can you revert these 4 commits sequentially with git revert commit-hash? Thanks!
#22344
#22330
#22239
#16677

@viirya
Copy link
Member Author

viirya commented Sep 20, 2018

@cloud-fan Ok. Let me try it.

@viirya
Copy link
Member Author

viirya commented Sep 20, 2018

@cloud-fan Shall we have 4 PRs to revert these PRs individually? Or just one PR to includes 4 revert commits?

@cloud-fan
Copy link
Contributor

just one PR including 4 commits

@viirya viirya closed this Sep 20, 2018
@viirya viirya deleted the revert-SPARK-19355 branch December 27, 2023 18:35
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.

5 participants