-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29203][SQL][TESTS] Reduce shuffle partitions in SQLQueryTestSuite #25891
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
Changes from 4 commits
3dc0124
6ec9761
55004b9
b4f2d19
ad6bee7
df51b69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,7 +22,7 @@ import java.util.{Locale, TimeZone} | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import scala.util.control.NonFatal | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import org.apache.spark.SparkException | ||||||||||||||||||||||||||||||||
| import org.apache.spark.{SparkConf, SparkException} | ||||||||||||||||||||||||||||||||
| import org.apache.spark.sql.catalyst.planning.PhysicalOperation | ||||||||||||||||||||||||||||||||
| import org.apache.spark.sql.catalyst.plans.logical._ | ||||||||||||||||||||||||||||||||
| import org.apache.spark.sql.catalyst.plans.logical.sql.{DescribeColumnStatement, DescribeTableStatement} | ||||||||||||||||||||||||||||||||
|
|
@@ -133,6 +133,10 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession { | |||||||||||||||||||||||||||||||
| private val notIncludedMsg = "[not included in comparison]" | ||||||||||||||||||||||||||||||||
| private val clsName = this.getClass.getCanonicalName | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| override def sparkConf: SparkConf = super.sparkConf | ||||||||||||||||||||||||||||||||
| // Fewer shuffle partitions to speed up testing. | ||||||||||||||||||||||||||||||||
| .set(SQLConf.SHUFFLE_PARTITIONS, 5) | ||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For Python UDF test, this seems to increase from 4 to 5. Did I understand correctly?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually why don't we try 4 @wangyum?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I'll try to set it to 4. This is because it is set to 5 in two places: spark/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala Lines 58 to 64 in 359375e
spark/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala Lines 613 to 620 in 42b80ae
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add an ORDER BY to make the query output deterministic? |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** List of test cases to ignore, in lower cases. */ | ||||||||||||||||||||||||||||||||
| protected def blackList: Set[String] = Set( | ||||||||||||||||||||||||||||||||
| "blacklist.sql" // Do NOT remove this one. It is here to test the blacklist functionality. | ||||||||||||||||||||||||||||||||
|
|
@@ -291,10 +295,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| testCase match { | ||||||||||||||||||||||||||||||||
| case udfTestCase: UDFTest => | ||||||||||||||||||||||||||||||||
| // In Python UDF tests, the number of shuffle partitions matters considerably in | ||||||||||||||||||||||||||||||||
| // the testing time because it requires to fork and communicate between external | ||||||||||||||||||||||||||||||||
| // processes. | ||||||||||||||||||||||||||||||||
| localSparkSession.conf.set(SQLConf.SHUFFLE_PARTITIONS.key, 4) | ||||||||||||||||||||||||||||||||
| registerTestUDF(udfTestCase.udf, localSparkSession) | ||||||||||||||||||||||||||||||||
| case _ => | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
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.
Ur, do we really need this?
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.
Yes.
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.
Got it. It seems that we are hitting the corner case because the query has a sort on a subset of columns.