-
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-35385][SQL][TESTS] Skip duplicate queries in the TPCDS-related tests #32520
Conversation
// we skip them in the TPCDS-related tests. | ||
private val excludedTpcdsQueries: Set[String] = Set("q6", "q34", "q64", "q74", "q75", "q78") | ||
|
||
val tpcdsQueries: Seq[String] = tpcdsAllQueries.filterNot(excludedTpcdsQueries.contains) |
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.
another idea:
val tpcdsQueries: Seq[String] = 1.to(99).map("q" + _).flatMap { q =>
if (Seq("q14", "q23", "q24", "q39").contains(q)) Seq(q + "a", q + "b") else Seq(q)
}.filterNot { q =>
// ...
Seq("q6", "q34", "q64", "q74", "q75", "q78").contains(q)
}
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.
I feel the if
part looks a bit tricky, so I prefer to keep it as it is.
Kubernetes integration test starting |
Kubernetes integration test status failure |
@@ -36,6 +36,12 @@ trait TPCDSBase extends SharedSparkSession with TPCDSSchema { | |||
"q81", "q82", "q83", "q84", "q85", "q86", "q87", "q88", "q89", "q90", | |||
"q91", "q92", "q93", "q94", "q95", "q96", "q97", "q98", "q99") | |||
|
|||
// Since `tpcdsQueriesV2_7_0` has almost the same queries with these oens below, | |||
// we skip them in the TPCDS-related tests. |
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.
This hides the previous reasoning. I believe it's worth to keep SPARK-35327 comment explicitly.
SPARK-35327: Filters out the TPC-DS queries that can cause flaky test results
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.
Sure, updated.
@@ -83,12 +83,6 @@ class TPCDSQueryTestSuite extends QueryTest with TPCDSBase with SQLQueryTestHelp | |||
.toFile.getAbsolutePath | |||
} | |||
|
|||
override val tpcdsQueries = { | |||
// SPARK-35327: Filters out the TPC-DS queries that can cause flaky test results |
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.
Please move this to the new location too.
Test build #138454 has finished for PR 32520 at commit
|
The last commit is only for the comment update, so I merged to master. Thank you, @cloud-fan @dongjoon-hyun |
Test build #138479 has finished for PR 32520 at commit
|
What changes were proposed in this pull request?
This PR proposes to skip the "q6", "q34", "q64", "q74", "q75", "q78" queries in the TPCDS-related tests because the TPCDS v2.7 queries have almost the same ones; the only differences in these queries are ORDER BY columns.
Why are the changes needed?
To improve test performance.
Does this PR introduce any user-facing change?
No, dev only.
How was this patch tested?
Existing tests.