Migrate flaky ITNestedQueryPushDownTest to embedded test #18807
Conversation
…laky IT). The ITNestedQueryPushDownTest has recently been very flaky. So this patch migrates it to the Embedded test framework and hopefully should resolve any flakiness in this IT for good.
|
Thanks a lot for picking this up, @abhishekrb19 ! 🎉 🎉 |
|
Thanks, @kfaraz! I've also added the original native queries for completeness. Please let me know what you think. |
| * Hook for the additional setup that needs to be done before all tests. | ||
| */ | ||
| protected void beforeAll() | ||
| protected void beforeAll() throws IOException |
There was a problem hiding this comment.
Maybe generalize to just have throws Exception.
kfaraz
left a comment
kfaraz
left a comment
There was a problem hiding this comment.
Looks good, left some suggestions.
| ); | ||
|
|
||
| // Nested group by query with force push down and renamed dimensions | ||
| cluster.callApi().verifySqlQuery( |
There was a problem hiding this comment.
Let's put each query into a separate test method of its own, for both native and SQL cases.
|
|
||
| final String sql = StringUtils.format( | ||
| "SET waitUntilSegmentsLoad = TRUE;\n" | ||
| + "REPLACE INTO \"%s\" OVERWRITE ALL\n" |
There was a problem hiding this comment.
Would it be possible to use MoreResources.MSQ.INSERT_TINY_WIKI_JSON here?
There was a problem hiding this comment.
Yeah, I think so. The tiny_wiki* datasets contain a timestamp column, while the original wikiticker dataset used in this test has a time column, so switching between them breaks the test with InsertTimeNull: Null timestamp (__time) encountered during INSERT or REPLACE.. We could separately reconcile this by updating one of the datasets and relevant tests (probably tiny_wiki?) that will let us reuse things more cleanly; for now, I've just left this wikiticker query in.
There was a problem hiding this comment.
Thanks for checking! Yeah, this shouldn't block the PR (this change is much needed! 😄 ).
We can dedupe later.
| broker.bindings() | ||
| .jsonMapper() | ||
| .writeValueAsString(new LocalInputSource(null, null, Collections.singletonList(wikiFile), null)) |
There was a problem hiding this comment.
Can this be simplified in a manner similar to how MoreResources.MSQ.INSERT_TINY_WIKI_JSON does it?
2 participants
Migrate
ITNestedQueryPushDownTestto the embedded test framework.This specific test has particularly been very flaky on PRs often requiring multiple reruns . A few recent failed runs: https://github.com/apache/druid/actions/runs/19917476737/job/57101420884 and https://github.com/apache/druid/actions/runs/19696360296/job/56424984581
Kudos to @kfaraz for leading the efforts on the embedded test framework. The migration itself was quite straightforward. While porting the test, I noticed a SQL bug related to
forcePushDownNestedQuery, please see the related disabled test.This PR has: