Skip to content

Commit 6719168

Browse files
stefankandicMaxGekk
authored andcommitted
[SPARK-47327][SQL] Move sort keys concurrency test to CollationFactorySuite
### What changes were proposed in this pull request? Move concurrency test to the `CollationFactorySuite` ### Why are the changes needed? This is more appropriate location for the test as it directly uses the `CollationFactory`. Also, I just found out that `par` method is highly discouraged and that we should use `ParSeq` instead. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? With existing UTs ### Was this patch authored or co-authored using generative AI tooling? No Closes #45501 from stefankandic/moveTest. Authored-by: Stefan Kandic <[email protected]> Signed-off-by: Max Gekk <[email protected]>
1 parent 653ac5b commit 6719168

File tree

3 files changed

+20
-14
lines changed

3 files changed

+20
-14
lines changed

common/unsafe/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@
4747
<version>${project.version}</version>
4848
</dependency>
4949

50+
<dependency>
51+
<groupId>org.scala-lang.modules</groupId>
52+
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
53+
<scope>test</scope>
54+
</dependency>
55+
5056
<dependency>
5157
<groupId>com.ibm.icu</groupId>
5258
<artifactId>icu4j</artifactId>

common/unsafe/src/test/scala/org/apache/spark/unsafe/types/CollationFactorySuite.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.apache.spark.unsafe.types
1919

20+
import scala.collection.parallel.immutable.ParSeq
2021
import scala.jdk.CollectionConverters.MapHasAsScala
2122

2223
import org.apache.spark.SparkException
@@ -138,4 +139,17 @@ class CollationFactorySuite extends AnyFunSuite with Matchers { // scalastyle:ig
138139
assert(result == testCase.expectedResult)
139140
})
140141
}
142+
143+
test("test concurrently generating collation keys") {
144+
// generating ICU sort keys is not thread-safe by default so this should fail
145+
// if we don't handle the concurrency properly on Collator level
146+
147+
(0 to 10).foreach(_ => {
148+
val collator = fetchCollation("UNICODE").collator
149+
150+
ParSeq(0 to 100).foreach { _ =>
151+
collator.getCollationKey("aaa")
152+
}
153+
})
154+
}
141155
}

sql/core/src/test/scala/org/apache/spark/sql/CollationSuite.scala

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.apache.spark.sql
1919

2020
import scala.collection.immutable.Seq
21-
import scala.collection.parallel.CollectionConverters.ImmutableIterableIsParallelizable
2221
import scala.jdk.CollectionConverters.MapHasAsJava
2322

2423
import org.apache.spark.SparkException
@@ -413,19 +412,6 @@ class CollationSuite extends DatasourceV2SQLBase with AdaptiveSparkPlanHelper {
413412
}
414413
}
415414

416-
test("test concurrently generating collation keys") {
417-
// generating ICU sort keys is not thread-safe by default so this should fail
418-
// if we don't handle the concurrency properly on Collator level
419-
420-
(0 to 10).foreach(_ => {
421-
val collator = CollationFactory.fetchCollation("UNICODE").collator
422-
423-
(0 to 100).par.foreach { _ =>
424-
collator.getCollationKey("aaa")
425-
}
426-
})
427-
}
428-
429415
test("text writing to parquet with collation enclosed with backticks") {
430416
withTempPath{ path =>
431417
sql(s"select 'a' COLLATE `UNICODE`").write.parquet(path.getAbsolutePath)

0 commit comments

Comments
 (0)