Skip to content

Conversation

@xy-xin
Copy link

@xy-xin xy-xin commented May 15, 2019

What changes were proposed in this pull request?

RecordBinaryComparator, UnsafeExternalRowSorter and UnsafeKeyValueSorter now locates in catalyst, which should be moved to core, as they're used only in physical plan.

How was this patch tested?

exist tests.

@xy-xin xy-xin changed the title Move org.apache.spark.sql.execution.* in catalyst to core [SPARK-27713][SQL] Move org.apache.spark.sql.execution.* in catalyst to core May 15, 2019
@xy-xin
Copy link
Author

xy-xin commented May 15, 2019

cc @cloud-fan @JoshRosen @gatorsmile

@cloud-fan
Copy link
Contributor

ok to test

@cloud-fan
Copy link
Contributor

makes sense to me

@HyukjinKwon
Copy link
Member

Yup, makes sense to me too

@kiszk
Copy link
Member

kiszk commented May 15, 2019

Looks good

@JoshRosen
Copy link
Contributor

👍 from me, especially since these classes are low-level and internal to Spark (so I think there's no risk of breaking third-party packages).

@SparkQA
Copy link

SparkQA commented May 15, 2019

Test build #105396 has finished for PR 24607 at commit 6909b5a.

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

@viirya
Copy link
Member

viirya commented May 15, 2019

Makes sense. The related test suite RecordBinaryComparatorSuite is in core, too.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in fd9acf2 May 15, 2019
@xy-xin
Copy link
Author

xy-xin commented May 15, 2019

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.

7 participants