Skip to content

Commit 6f42817

Browse files
authored
Fix Spark 4.2 collect_set float buffer conversion for mixed aggs (NVIDIA#15455)
Fixes NVIDIA#15454. ### Description - Convert CollectSet float/double aggregation buffers between GPU logical values and Spark 4.2 CPU normalized bit-pattern keys during mixed hashAgg stages, so NaN and signed-zero uniqueness matches pure CPU. - Add `collectSetCpuBufferElementType` shim (bit-keyed on Spark 4.2, identity earlier) and CollectSet-specific GPU↔CPU buffer converters used by `GpuOverrides`. - Validate with Spark 4.2.0 / Scala 2.13 and `DATAGEN_SEED=1785353212`: `352 passed` including the previously failing `test_hash_groupby_collect_partial_replace_fallback` / `test_hash_groupby_collect_partial_replace_with_distinct_fallback` Float cases; also `mvn -f scala2.13/pom.xml -Dbuildver=420 -Dcuda.version=cuda13 -DskipTests -pl sql-plugin verify`. ### Checklists Documentation - [ ] Updated for new or modified user-facing features or behaviors - [x] No user-facing change Testing - [ ] Added or modified tests to cover new code paths - [x] Covered by existing tests (Please provide the names of the existing tests in the PR description.) - [ ] Not required Performance - [ ] Tests ran and results are added in the PR description - [ ] Issue filed with a link in the PR description - [x] Not required --------- Signed-off-by: Firestarman <firestarmanllc@gmail.com>
1 parent a84225f commit 6f42817

5 files changed

Lines changed: 176 additions & 3 deletions

File tree

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,11 +3859,12 @@ object GpuOverrides extends Logging {
38593859
}
38603860

38613861
override def createCpuToGpuBufferConverter(): CpuToGpuAggregateBufferConverter =
3862-
new CpuToGpuCollectBufferConverter(c.child.dataType,
3862+
new CpuToGpuCollectSetBufferConverter(c.child.dataType,
38633863
!TypeUtilsShims.collectSetIgnoreNulls(c))
38643864

38653865
override def createGpuToCpuBufferConverter(): GpuToCpuAggregateBufferConverter =
3866-
new GpuToCpuCollectBufferConverter()
3866+
new GpuToCpuCollectSetBufferConverter(c.child.dataType,
3867+
!TypeUtilsShims.collectSetIgnoreNulls(c))
38673868

38683869
override val supportBufferConversion: Boolean = true
38693870

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/aggregate/aggregateFunctions.scala

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,6 +2079,163 @@ case class GpuToCpuCollectBufferTransition(
20792079
}
20802080
}
20812081

2082+
/**
2083+
* CollectSet buffer converters.
2084+
*
2085+
* Spark 4.2 changed CollectSet's CPU agg buffer for FloatType/DoubleType to store normalized
2086+
* bit patterns (IntegerType/LongType) so HashSet can treat NaNs and signed zeros as equal.
2087+
* GPU CollectSet still stores the logical float/double values, so mixed CPU/GPU aggregation
2088+
* stages must convert between the two buffer layouts.
2089+
*/
2090+
class CpuToGpuCollectSetBufferConverter(
2091+
elementType: DataType,
2092+
containsNull: Boolean = false) extends CpuToGpuAggregateBufferConverter {
2093+
def createExpression(child: Expression): CpuToGpuBufferTransition = {
2094+
CpuToGpuCollectSetBufferTransition(child, elementType, containsNull)
2095+
}
2096+
}
2097+
2098+
case class CpuToGpuCollectSetBufferTransition(
2099+
override val child: Expression,
2100+
private val elementType: DataType,
2101+
private val containsNull: Boolean) extends CpuToGpuBufferTransition {
2102+
2103+
private lazy val row = new UnsafeRow(1)
2104+
private lazy val cpuElementType: DataType =
2105+
TypeUtilsShims.collectSetCpuBufferElementType(elementType)
2106+
2107+
override def dataType: DataType = ArrayType(elementType, containsNull)
2108+
2109+
override protected def nullSafeEval(input: Any): ArrayData = {
2110+
val bytes = input.asInstanceOf[Array[Byte]]
2111+
row.pointTo(bytes, bytes.length)
2112+
val cpuArray = row.getArray(0)
2113+
if (cpuElementType == elementType) {
2114+
cpuArray.copy()
2115+
} else {
2116+
CollectSetBufferConversions.cpuBitsToGpuValues(cpuArray, elementType, containsNull)
2117+
}
2118+
}
2119+
}
2120+
2121+
class GpuToCpuCollectSetBufferConverter(
2122+
elementType: DataType,
2123+
containsNull: Boolean = false) extends GpuToCpuAggregateBufferConverter {
2124+
def createExpression(child: Expression): GpuToCpuBufferTransition = {
2125+
GpuToCpuCollectSetBufferTransition(child, elementType, containsNull)
2126+
}
2127+
}
2128+
2129+
case class GpuToCpuCollectSetBufferTransition(
2130+
override val child: Expression,
2131+
private val elementType: DataType,
2132+
private val containsNull: Boolean) extends GpuToCpuBufferTransition {
2133+
2134+
private lazy val cpuElementType: DataType =
2135+
TypeUtilsShims.collectSetCpuBufferElementType(elementType)
2136+
private lazy val cpuBufferType: DataType = ArrayType(cpuElementType, containsNull)
2137+
private lazy val projection = UnsafeProjection.create(Array[DataType](cpuBufferType))
2138+
2139+
override protected def nullSafeEval(input: Any): Array[Byte] = {
2140+
val arrayData = input.asInstanceOf[ArrayData]
2141+
val cpuArray = if (cpuElementType == elementType) {
2142+
arrayData
2143+
} else {
2144+
CollectSetBufferConversions.gpuValuesToCpuBits(arrayData, elementType, containsNull)
2145+
}
2146+
projection.apply(InternalRow.apply(cpuArray)).getBytes
2147+
}
2148+
}
2149+
2150+
object CollectSetBufferConversions {
2151+
// Matches Spark's NormalizeFloatingNumbers.FLOAT_NORMALIZER / DOUBLE_NORMALIZER.
2152+
private def normalizeFloat(f: Float): Float = {
2153+
if (f.isNaN) {
2154+
Float.NaN
2155+
} else if (f == -0.0f) {
2156+
0.0f
2157+
} else {
2158+
f
2159+
}
2160+
}
2161+
2162+
private def normalizeDouble(d: Double): Double = {
2163+
if (d.isNaN) {
2164+
Double.NaN
2165+
} else if (d == -0.0d) {
2166+
0.0d
2167+
} else {
2168+
d
2169+
}
2170+
}
2171+
2172+
def gpuValuesToCpuBits(
2173+
arrayData: ArrayData,
2174+
elementType: DataType,
2175+
containsNull: Boolean): ArrayData = {
2176+
val n = arrayData.numElements()
2177+
val out = new Array[Any](n)
2178+
var i = 0
2179+
elementType match {
2180+
case FloatType =>
2181+
while (i < n) {
2182+
if (containsNull && arrayData.isNullAt(i)) {
2183+
out(i) = null
2184+
} else {
2185+
out(i) = java.lang.Float.floatToIntBits(normalizeFloat(arrayData.getFloat(i)))
2186+
}
2187+
i += 1
2188+
}
2189+
case DoubleType =>
2190+
while (i < n) {
2191+
if (containsNull && arrayData.isNullAt(i)) {
2192+
out(i) = null
2193+
} else {
2194+
out(i) = java.lang.Double.doubleToLongBits(normalizeDouble(arrayData.getDouble(i)))
2195+
}
2196+
i += 1
2197+
}
2198+
case other =>
2199+
throw new IllegalStateException(
2200+
s"Unexpected CollectSet GPU-to-CPU buffer conversion for $other")
2201+
}
2202+
new GenericArrayData(out)
2203+
}
2204+
2205+
def cpuBitsToGpuValues(
2206+
arrayData: ArrayData,
2207+
elementType: DataType,
2208+
containsNull: Boolean): ArrayData = {
2209+
val n = arrayData.numElements()
2210+
val out = new Array[Any](n)
2211+
var i = 0
2212+
elementType match {
2213+
case FloatType =>
2214+
while (i < n) {
2215+
if (containsNull && arrayData.isNullAt(i)) {
2216+
out(i) = null
2217+
} else {
2218+
out(i) = java.lang.Float.intBitsToFloat(arrayData.getInt(i))
2219+
}
2220+
i += 1
2221+
}
2222+
case DoubleType =>
2223+
while (i < n) {
2224+
if (containsNull && arrayData.isNullAt(i)) {
2225+
out(i) = null
2226+
} else {
2227+
out(i) = java.lang.Double.longBitsToDouble(arrayData.getLong(i))
2228+
}
2229+
i += 1
2230+
}
2231+
case other =>
2232+
throw new IllegalStateException(
2233+
s"Unexpected CollectSet CPU-to-GPU buffer conversion for $other")
2234+
}
2235+
new GenericArrayData(out)
2236+
}
2237+
}
2238+
20822239
/**
20832240
* Base class for overriding standard deviation and variance aggregations.
20842241
* This is also a GPU-based implementation of 'CentralMomentAgg' aggregation class in Spark with

sql-plugin/src/main/spark330/scala/com/nvidia/spark/rapids/shims/TypeUtilsShims.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import ai.rapids.cudf.NaNEquality
2727

2828
import org.apache.spark.sql.catalyst.expressions.aggregate.{CollectList, CollectSet}
2929
import org.apache.spark.sql.catalyst.util.TypeUtils
30+
import org.apache.spark.sql.types.DataType
3031

3132
/**
3233
* Reimplement the function `checkForNumericExpr` which has been removed since
@@ -37,6 +38,9 @@ object TypeUtilsShims {
3738

3839
val collectSetFloatNanEquality: NaNEquality = NaNEquality.UNEQUAL
3940

41+
// Pre-Spark 4.2 CollectSet stores child values directly in the agg buffer.
42+
def collectSetCpuBufferElementType(childType: DataType): DataType = childType
43+
4044
def collectListIgnoreNulls(_collectList: CollectList): Boolean = true
4145

4246
def collectSetIgnoreNulls(_collectSet: CollectSet): Boolean = true

sql-plugin/src/main/spark330db/scala/com/nvidia/spark/rapids/shims/TypeUtilsShims.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ object TypeUtilsShims {
6767

6868
val collectSetFloatNanEquality: NaNEquality = NaNEquality.UNEQUAL
6969

70+
// Pre-Spark 4.2 CollectSet stores child values directly in the agg buffer.
71+
def collectSetCpuBufferElementType(childType: DataType): DataType = childType
72+
7073
def collectListIgnoreNulls(_collectList: CollectList): Boolean = true
7174

7275
def collectSetIgnoreNulls(_collectSet: CollectSet): Boolean = true

sql-plugin/src/main/spark420/scala/com/nvidia/spark/rapids/shims/TypeUtilsShims.scala

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import ai.rapids.cudf.NaNEquality
2222

2323
import org.apache.spark.sql.catalyst.analysis.TypeCheckResult
2424
import org.apache.spark.sql.catalyst.expressions.aggregate.{CollectList, CollectSet}
25-
import org.apache.spark.sql.types.{DataType, NullType, NumericType}
25+
import org.apache.spark.sql.types.{DataType, DoubleType, FloatType, IntegerType, LongType, NullType,
26+
NumericType}
2627

2728
/**
2829
* Reimplement the function `checkForNumericExpr` which has been removed since
@@ -40,6 +41,13 @@ object TypeUtilsShims {
4041
// Spark 4.2 stores collect_set buffers in a way that treats all NaN values as one set entry.
4142
val collectSetFloatNanEquality: NaNEquality = NaNEquality.ALL_EQUAL
4243

44+
// Spark 4.2 CollectSet keys float/double by normalized bit patterns in the agg buffer.
45+
def collectSetCpuBufferElementType(childType: DataType): DataType = childType match {
46+
case FloatType => IntegerType
47+
case DoubleType => LongType
48+
case other => other
49+
}
50+
4351
def collectListIgnoreNulls(collectList: CollectList): Boolean = collectList.ignoreNulls
4452

4553
def collectSetIgnoreNulls(collectSet: CollectSet): Boolean = collectSet.ignoreNulls

0 commit comments

Comments
 (0)