[SPARK-42746][SQL][FOLLOWUP] Fixing potential flakiness in ListAgg golden files #50357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mihailom-db
changed the title
beliefer
changed the title
beliefer
reviewed
Mar 24, 2025
| @@ -1,13 +1,13 @@ | |||
| -- Test cases with utf8_binary | |||
| SELECT listagg(c1) WITHIN GROUP (ORDER BY c1 COLLATE utf8_binary) FROM (VALUES ('a'), ('A'), ('b'), ('B')) AS t(c1); | |||
| SELECT listagg(DISTINCT c1 COLLATE utf8_binary) FROM (VALUES ('a'), ('A'), ('b'), ('B')) AS t(c1); | |||
| WITH t(c1) AS (SELECT listagg(DISTINCT col1 COLLATE utf8_binary) FROM (VALUES ('a'), ('A'), ('b'), ('B'))) SELECT len(c1), regexp_count(c1, 'a'), regexp_count(c1, 'b'), regexp_count(c1, 'A'), regexp_count(c1, 'B') FROM t; | |||
There was a problem hiding this comment.
Could you explain why you introduce this?
There was a problem hiding this comment.
In tests in general, there is no guarantee that ordering of the column will be the same as the query insertion time, unless we include an order by clause. This is potentially making tests flaky, as golden file tests need to be regenerated, so this PR is only making sure we do not have to rerun CIs for tests potentially flaky tests. Also, it is better to prevent flakiness first, then to leave it for detection later.
There was a problem hiding this comment.
I think we do not need this one if listagg is deterministic.
There was a problem hiding this comment.
Spark aggregate is not deterministic by nature, as the shuffle reader fetches shuffle blocks in a random order, and they can arrive in a random order.
|
thanks, merging to master/4.0! |
cloud-fan
pushed a commit
that referenced
this pull request
Mar 24, 2025
…lden files ### What changes were proposed in this pull request? Original PR (#50338) fixed some of the flakiness, but there were more tests that could potentially be flaky. This PR is fixing these issues. ### Why are the changes needed? We should not rely in golden files tests on buffer ordering. This could lead to flakiness in tests and we need to fix it, so that we do not waste resources. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Test only change. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #50357 from mihailom-db/listaggfollowup2. Authored-by: Mihailo Milosevic <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit e4b6e9a) Signed-off-by: Wenchen Fan <[email protected]>
SauronShepherd
pushed a commit
to SauronShepherd/spark
that referenced
this pull request
Mar 25, 2025
…lden files ### What changes were proposed in this pull request? Original PR (apache#50338) fixed some of the flakiness, but there were more tests that could potentially be flaky. This PR is fixing these issues. ### Why are the changes needed? We should not rely in golden files tests on buffer ordering. This could lead to flakiness in tests and we need to fix it, so that we do not waste resources. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Test only change. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50357 from mihailom-db/listaggfollowup2. Authored-by: Mihailo Milosevic <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Original PR (#50338) fixed some of the flakiness, but there were more tests that could potentially be flaky. This PR is fixing these issues.
Why are the changes needed?
We should not rely in golden files tests on buffer ordering. This could lead to flakiness in tests and we need to fix it, so that we do not waste resources.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Test only change.
Was this patch authored or co-authored using generative AI tooling?
No.