Skip to content

Commit 9406d37

Browse files
robert3005claude
andcommitted
Keep float comparisons in Spark and test against Spark 4.2
Vortex compares floats in total order, so -0.0 and 0.0 are not equal, while Spark treats them as equal. Spark cannot bring back rows that Vortex drops, so do not push float and double comparisons (#10064). Run the Spark 4.0 module's compat suite against Spark 4.2.0 as well as 4.1.2, since 4.2 changes CatalogV2Util signatures. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
1 parent 17525a6 commit 9406d37

7 files changed

Lines changed: 39 additions & 19 deletions

File tree

‎java/buildSrc/src/main/kotlin/vortex-spark-module.gradle.kts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,15 @@ val scalaVersion =
2929
"4.0" to "2.13" -> libs.findVersion("scala213Spark40").get().requiredVersion
3030
else -> throw GradleException("Unsupported Spark/Scala variant: $sparkLine/$scalaBinaryVersion")
3131
}
32-
val spark41Version = libs.findVersion("spark41").get().requiredVersion
32+
// Later Spark 4 releases run the Spark 4.0 module's tests against its jar to catch binary breaks.
33+
val sparkCompatVersions =
34+
if (sparkLine == "4.0") {
35+
listOf("4.1" to "spark41", "4.2" to "spark42").map { (line, alias) ->
36+
line to libs.findVersion(alias).get().requiredVersion
37+
}
38+
} else {
39+
emptyList()
40+
}
3341

3442
layout.buildDirectory = layout.projectDirectory.dir("build/${project.name}")
3543

@@ -139,8 +147,8 @@ testing {
139147
targets.all { testTask.configure { enabled = false } }
140148
}
141149

142-
if (sparkLine == "4.0") {
143-
register<JvmTestSuite>("spark41CompatTest") {
150+
sparkCompatVersions.forEach { (line, version) ->
151+
register<JvmTestSuite>("spark${line.replace(".", "")}CompatTest") {
144152
useJUnitJupiter()
145153
sources {
146154
java.setSrcDirs(
@@ -159,8 +167,8 @@ testing {
159167
dependencies {
160168
implementation(project())
161169
implementation(libs.findLibrary("junit-jupiter").get())
162-
implementation("org.apache.spark:spark-core_2.13:$spark41Version")
163-
implementation("org.apache.spark:spark-sql_2.13:$spark41Version")
170+
implementation("org.apache.spark:spark-core_2.13:$version")
171+
implementation("org.apache.spark:spark-sql_2.13:$version")
164172
implementation(libs.findLibrary("s3mock-testcontainers").get())
165173
implementation(libs.findLibrary("testcontainers-juputer").get())
166174
runtimeOnly(libs.findLibrary("slf4j-simple").get())
@@ -170,9 +178,9 @@ testing {
170178
}
171179
}
172180

173-
if (sparkLine == "4.0") {
174-
tasks.named("check") {
175-
dependsOn("spark41CompatTest")
181+
tasks.named("check") {
182+
sparkCompatVersions.forEach { (line, _) ->
183+
dependsOn("spark${line.replace(".", "")}CompatTest")
176184
}
177185
}
178186

‎java/gradle/libs.versions.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ scala213Spark41 = "2.13.17"
2020
spark35 = "3.5.9"
2121
spark40 = "4.0.4"
2222
spark41 = "4.1.2"
23+
spark42 = "4.2.0"
2324
s3mock = "5.0.0"
2425
testcontainers-jupiter = "1.21.4"
2526

‎java/vortex-spark/common/src/main/java/dev/vortex/spark/read/SparkFilterToVortexExpression.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,10 @@ private static Optional<Expression> literal(Object value, DataType dataType) {
265265
if (dataType instanceof LongType) {
266266
return numericLiteral(value, Expression.DType.I64, number -> Expression.literal(number.longValue()));
267267
}
268-
if (dataType instanceof FloatType) {
269-
return numericLiteral(value, Expression.DType.F32, number -> Expression.literal(number.floatValue()));
270-
}
271-
if (dataType instanceof DoubleType) {
272-
return numericLiteral(value, Expression.DType.F64, number -> Expression.literal(number.doubleValue()));
268+
if (dataType instanceof FloatType || dataType instanceof DoubleType) {
269+
// Vortex compares floats in total order, so -0.0 and 0.0 differ, while Spark treats them as equal.
270+
// Spark cannot restore rows that Vortex drops, so keep these comparisons in Spark (see #10064).
271+
return Optional.empty();
273272
}
274273
if (dataType instanceof StringType) {
275274
if (value == null) {

‎java/vortex-spark/common/src/main/scala/dev/vortex/spark/read/VortexScan.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ final class VortexScan(
107107
}
108108
}
109109

110-
/** The listed files that belong to the dataset. The readers skip the others. */
110+
/** The listed files that belong to the dataset. The readers skip the others.
111+
*/
111112
private def vortexFiles =
112113
partitions
113114
.flatMap(_.files)

‎java/vortex-spark/common/src/test/java/dev/vortex/spark/VortexBucketingTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ void bucketByWritesSparkBuckets() throws IOException {
8282
assertFalse(buckets.isEmpty(), "bucketed table should hold bucket files");
8383
assertTrue(buckets.stream().allMatch(id -> id >= 0 && id < 4), "bucket ids: " + buckets);
8484

85-
Dataset<Row> joined = spark.sql(
86-
"SELECT l.id FROM bucketed_left l JOIN bucketed_right r ON l.id = r.id");
85+
Dataset<Row> joined = spark.sql("SELECT l.id FROM bucketed_left l JOIN bucketed_right r ON l.id = r.id");
8786
String plan = joined.queryExecution().executedPlan().toString();
8887
assertFalse(plan.contains("Exchange"), "a bucketed join should not shuffle:\n" + plan);
8988
assertEquals(50, joined.count());
@@ -95,7 +94,8 @@ void bucketTransformCreatesBucketedTable() throws IOException {
9594
spark.sql("CREATE TABLE bucketed_sql (id INT, name STRING) USING vortex PARTITIONED BY (bucket(4, id))");
9695
spark.sql("INSERT INTO bucketed_sql SELECT CAST(id AS INT), CAST(id AS STRING) FROM range(40)");
9796

98-
List<Row> rows = spark.sql("SELECT count(*), count(DISTINCT id) FROM bucketed_sql").collectAsList();
97+
List<Row> rows = spark.sql("SELECT count(*), count(DISTINCT id) FROM bucketed_sql")
98+
.collectAsList();
9999
assertEquals(40L, rows.get(0).getLong(0));
100100
assertEquals(40L, rows.get(0).getLong(1));
101101

‎java/vortex-spark/common/src/test/java/dev/vortex/spark/VortexFileExtensionTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
* Only files that end with {@code .vortex} belong to a Vortex dataset.
2828
*
2929
* <p>Spark's file index keeps {@code _metadata} and {@code _common_metadata}, and it keeps every extension it does not
30-
* recognise, so the connector is the only thing that can decide what belongs to the dataset. It skips every other
31-
* file, as the connector before the file-source rewrite did. These tests hold the paths that see the listing — schema
30+
* recognise, so the connector is the only thing that can decide what belongs to the dataset. It skips every other file,
31+
* as the connector before the file-source rewrite did. These tests hold the paths that see the listing — schema
3232
* inference, scan statistics, {@code COUNT(*)} pushdown, and the V1 and V2 scans — to the same answer.
3333
*/
3434
@TestInstance(TestInstance.Lifecycle.PER_CLASS)

‎java/vortex-spark/common/src/test/java/dev/vortex/spark/read/SparkFilterToVortexExpressionTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
final class SparkFilterToVortexExpressionTest {
2828
private static final StructType SCHEMA = new StructType()
2929
.add("id", DataTypes.IntegerType)
30+
.add("float", DataTypes.FloatType)
31+
.add("double", DataTypes.DoubleType)
3032
.add("date", DataTypes.DateType)
3133
.add("timestamp", DataTypes.TimestampType)
3234
.add("decimal", DataTypes.createDecimalType(10, 2))
@@ -73,6 +75,15 @@ void rejectsDecimalLiteralsThatCannotHoldTheirScale() {
7375
assertPushable(new EqualTo("decimal", new BigDecimal("1.00")));
7476
}
7577

78+
@Test
79+
void rejectsFloatingPointComparisons() {
80+
// Vortex orders -0.0 below 0.0, but Spark treats them as equal.
81+
assertFalse(SparkFilterToVortexExpression.isPushable(new EqualTo("float", 0.0f), SCHEMA));
82+
assertFalse(SparkFilterToVortexExpression.isPushable(new GreaterThan("double", 0.0), SCHEMA));
83+
assertFalse(SparkFilterToVortexExpression.isPushable(new In("double", new Object[] {0.0, 1.0}), SCHEMA));
84+
assertPushable(new IsNull("double"));
85+
}
86+
7687
@Test
7788
void rejectsAPartlyConvertibleConjunctionAndDisjunction() {
7889
assertFalse(SparkFilterToVortexExpression.isPushable(

0 commit comments

Comments
 (0)