Skip to content

Commit ca5688b

Browse files
res-liferevans2
andauthored
Fix a special case in limit where it could return an empty batch with the wrong number of columns (#13938) (#13971)
Picked from main branch(#13938) PR #13903 (target branch 25.12) needs to use the CSV files in #13938 Signed-off-by: Robert (Bobby) Evans <bobby@apache.org> Co-authored-by: Robert (Bobby) Evans <bobby@apache.org>
1 parent 332ba8d commit ca5688b

5 files changed

Lines changed: 24 additions & 8 deletions

File tree

integration_tests/src/main/python/join_test.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,15 @@ def do_join(spark):
476476
conf = {kudo_enabled_conf_key: kudo_enabled}
477477
assert_gpu_and_cpu_are_equal_collect(do_join, conf = conf)
478478

479+
# because this infers the schema for CSV we need to allow some ops to be on the CPU
480+
@allow_non_gpu("CollectLimitExec", "FileSourceScanExec", "DeserializeToObjectExec")
481+
def test_empty_cross_side_with_limit(std_input_path):
482+
def do_join(spark):
483+
t0 = spark.read.csv(std_input_path + '/t0.csv', header=True, inferSchema=True)
484+
t1 = spark.read.csv(std_input_path + '/t1.csv', header=True, inferSchema=True)
485+
return t0.crossJoin(t1).limit(21)
486+
assert_gpu_and_cpu_are_equal_collect(do_join)
487+
479488
# local sort because of https://github.com/NVIDIA/spark-rapids/issues/84
480489
# After 3.1.0 is the min spark version we can drop this
481490
@ignore_order(local=True)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
c0,c1
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
c0
2+
true

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
package com.nvidia.spark.rapids
1818

19-
import scala.collection.mutable.ArrayBuffer
20-
2119
import ai.rapids.cudf.Table
2220
import com.nvidia.spark.rapids.Arm.{closeOnExcept, withResource}
2321
import com.nvidia.spark.rapids.GpuMetric._
@@ -35,12 +33,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{AllTuples, Distribution, Pa
3533
import org.apache.spark.sql.catalyst.util.truncatedString
3634
import org.apache.spark.sql.execution.{CollectLimitExec, LimitExec, SparkPlan, TakeOrderedAndProjectExec}
3735
import org.apache.spark.sql.execution.exchange.ENSURE_REQUIREMENTS
36+
import org.apache.spark.sql.types.{DataType, StructField, StructType}
3837
import org.apache.spark.sql.vectorized.{ColumnarBatch, ColumnVector}
3938

4039
class GpuBaseLimitIterator(
4140
input: Iterator[ColumnarBatch],
4241
limit: Int,
4342
offset: Int,
43+
dataTypes : Array[DataType],
4444
opTime: GpuMetric,
4545
numOutputBatches: GpuMetric,
4646
numOutputRows: GpuMetric) extends Iterator[ColumnarBatch] {
@@ -55,7 +55,6 @@ class GpuBaseLimitIterator(
5555
}
5656

5757
var batch = input.next()
58-
val numCols = batch.numCols()
5958

6059
// In each partition, we need to skip `offset` rows
6160
while (batch != null && remainingOffset >= batch.numRows()) {
@@ -71,7 +70,10 @@ class GpuBaseLimitIterator(
7170
// If the last batch is null, then we have offset >= numRows in this partition.
7271
// In such case, we should return an empty batch
7372
if (batch == null || batch.numRows() == 0) {
74-
return new ColumnarBatch(new ArrayBuffer[GpuColumnVector](numCols).toArray, 0)
73+
val fields = dataTypes.zipWithIndex.map {
74+
case (dt, idx) => StructField(s"_col$idx", dt, nullable = true)
75+
}
76+
return GpuColumnVector.emptyBatch(StructType(fields))
7577
}
7678

7779
// Here 0 <= remainingOffset < batch.numRow(), we need to get batch[remainingOffset:]
@@ -156,7 +158,8 @@ trait GpuBaseLimitExec extends LimitExec with GpuExec with ShimUnaryExecNode {
156158
val numOutputRows = gpuLongMetric(NUM_OUTPUT_ROWS)
157159
val numOutputBatches = gpuLongMetric(NUM_OUTPUT_BATCHES)
158160
rdd.mapPartitions { iter =>
159-
new GpuBaseLimitIterator(iter, limit, offset, opTime, numOutputBatches, numOutputRows)
161+
new GpuBaseLimitIterator(iter, limit, offset, output.map(_.dataType).toArray,
162+
opTime, numOutputBatches, numOutputRows)
160163
}
161164
}
162165

tests/src/test/scala/com/nvidia/spark/rapids/LimitRetrySuite.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023-2024, NVIDIA CORPORATION.
2+
* Copyright (c) 2023-2025, NVIDIA CORPORATION.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,7 +21,7 @@ import com.nvidia.spark.rapids.Arm.withResource
2121
import com.nvidia.spark.rapids.jni.RmmSpark
2222

2323
import org.apache.spark.sql.catalyst.expressions.{Ascending, AttributeReference, ExprId, SortOrder}
24-
import org.apache.spark.sql.types.IntegerType
24+
import org.apache.spark.sql.types.{DataType, IntegerType}
2525
import org.apache.spark.sql.vectorized.ColumnarBatch
2626

2727
class LimitRetrySuite extends RmmSparkRetrySuiteBase {
@@ -79,7 +79,8 @@ class LimitRetrySuite extends RmmSparkRetrySuiteBase {
7979
val limitIter = new GpuBaseLimitIterator(
8080
// 3 batches as input, and each has 8 rows
8181
(0 until totalRows).grouped(8).map(buildBatch(_)).toList.toIterator,
82-
limit, offset, NoopMetric, NoopMetric, NoopMetric)
82+
limit, offset, Array[DataType](IntegerType),
83+
NoopMetric, NoopMetric, NoopMetric)
8384
var leftRows = if (limit > totalRows) totalRows - offset else limit - offset
8485
var curValue = offset
8586
RmmSpark.forceRetryOOM(RmmSpark.getCurrentThreadId, 1,

0 commit comments

Comments
 (0)