Fix join bug on csv datasources by res-life · Pull Request #13903 · NVIDIA/cudf-spark

res-life · 2025-11-28T04:04:12Z

bug analysis

This is a corner case described in the issue #13873.
If reading other type datasources instead of CSV files, the error does not occur.
If using .collect() instead of .show(), the error does not occur.
If the stream data is not empty, the error does not occur.

CSV file with only header and no data rows, empty table(num_rows = 0) will be returned, then after project, the column number of empty table becomes 0, finally empty table(num_rows=0, num_cols=0) occurs in the iterator.

bug fix

If num_rows of table is 0 when reading csv file, return None instead of empty table, the None indicates the iterator of CSV data is empty.

Signed-off-by: Chong Gao res_life@163.com

Signed-off-by: Chong Gao <res_life@163.com>

res-life · 2025-11-28T04:05:32Z

build

greptile-apps · 2025-11-28T04:07:24Z

Greptile Overview

Greptile Summary

Fixes a corner case where reading an empty CSV file (header only) and joining the result caused an assertion failure in GPU join operations.

The fix adds a check in handleResult() to return None when the decoded table has 0 rows, preventing the creation of a degenerate ColumnarBatch with 0 columns and 0 rows that would later trigger the assertion "data with no columns should have been filtered out already" in JoinGathererImpl.

Key Changes:

When CSV reader decodes a table with 0 rows, it now closes the table and returns None to signal empty data
This is consistent with existing behavior where dataSize == 0 also returns None
Adds regression test using CSV files with headers but no data rows

Technical Context:
The bug occurred when:

CSV file had only a header row (0 data rows)
Query projected away all columns (e.g., SELECT '1' FROM table)
CSV reader created an empty table that became a batch with 0 columns after projection
This degenerate batch reached the join logic which expected filtered out already

The fix prevents step 3 by returning None for empty tables, making the iterator signal "no data" correctly.

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The fix is a targeted, well-understood change that addresses a specific corner case. The logic is sound (returning None for empty tables is consistent with existing behavior), includes proper resource cleanup (table.close()), and adds a regression test. The change only affects the CSV/JSON reader path when tables have 0 rows, which is a rare edge case that was previously causing crashes.
No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTextBasedPartitionReader.scala	5/5	Adds check to return None when CSV has 0 rows, preventing empty batches with 0 columns from reaching join logic
integration_tests/src/main/python/join_test.py	5/5	Adds regression test for right outer join with empty CSV file to verify the fix

Sequence Diagram

sequenceDiagram
    participant CSVFile as CSV File (header only)
    participant Reader as GpuTextBasedPartitionReader
    participant GPU as GPU CSV Decoder
    participant Handler as handleResult()
    participant Batch as readBatch()
    participant Iterator as Iterator (next())
    participant Join as Join Logic

    Note over CSVFile: c0<br/>(0 data rows)
    
    Reader->>GPU: readToTable(isFirstChunk)
    GPU-->>Reader: Table (0 rows, N columns)
    
    alt Before Fix (Bug)
        Reader->>Handler: handleResult(table)
        Handler-->>Reader: Some(table)
        Reader->>Batch: Create ColumnarBatch
        Note over Batch: readDataSchema.isEmpty<br/>→ ColumnarBatch(Array.empty, 0)
        Batch-->>Iterator: Some(batch with 0 cols, 0 rows)
        Iterator->>Join: Pass batch to join
        Join->>Join: JoinGathererImpl assertion
        Note over Join: FAIL: "data with no columns<br/>should have been filtered"
    end
    
    alt After Fix (Correct)
        Reader->>Handler: handleResult(table)
        Note over Handler: Check: table.getRowCount == 0
        Handler->>Handler: table.close()
        Handler-->>Reader: None
        Reader->>Batch: table.map(...) with None
        Batch-->>Iterator: None
        Iterator-->>Join: hasNext = false
        Note over Join: No batch processed,<br/>join succeeds with empty result
    end

greptile-apps

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

firestarman · 2025-11-28T06:03:02Z

Personally it is not a good idea to fix the join error by changing the CSV reader. This error may happen again if the source is other type, e.g. parquet, orc...

I made a fix (#13817) before for similar issues, you can figure out the join type for this case and add it to the whitelist to allow it go into the degenerate path.

res-life · 2025-12-01T03:21:54Z

This error may happen again if the source is other type, e.g. parquet, orc...

Already tested, other types do not have this error.

you can figure out the join type for this case and add it to the whitelist to allow it go into the degenerate path.

IMO, this bug is not related to join types. When reading CSV file with no data(only one row header), the iterator<ColumnBatch> on this CSV file should have false value for hasNext. This PR is to fix the root cause in iterator<ColumnBatch>.

gerashegalov · 2025-12-01T18:47:08Z

+    if (table.getRowCount == 0) {
+      // CSV reader can return empty table, close it and return None
+      // E.g.: CSV file with only header and no data rows, empty table will be returned
+      table.close()


any concerns that this code can throw instead of returning None even if close fails?

The row count is zero, it means there is no GPU memory allocated although table has columns.
It is not likely to throw exceptions in practice.

firestarman · 2025-12-02T00:04:50Z

This error may happen again if the source is other type, e.g. parquet, orc...

Already tested, other types do not have this error.

you can figure out the join type for this case and add it to the whitelist to allow it go into the degenerate path.

IMO, this bug is not related to join types. When reading CSV file with no data(only one row header), the iterator<ColumnBatch> on this CSV file should have false value for hasNext. This PR is to fix the root cause in iterator<ColumnBatch>.

Yeah, this is a fix.
I mean there are already some similar paths at https://github.com/NVIDIA/spark-rapids/blob/main/sql-plugin/src/main/scala/org/apache/spark/sql/rapids/execution/GpuBroadcastNestedLoopJoinExecBase.scala#L759 for this kind of case with outer joins.
And this issue still exists in the GpuBroadcastNestedLoopJoinExecBase even this PR is merged and we don't have a case to run into it yet.

res-life · 2025-12-05T07:11:07Z

@firestarman I can not construct a right out condition join case as you tried, refer to link.
I can not add similar code as you did in #13817 because I do not know how to test.
Since this PR is a fix we can merge this first.

res-life · 2025-12-05T07:17:03Z

For conditional right outer join, e.g.: left_table right join right_table on c0_column, then the build table is left table and the build batch is not zero column.
For unconditional right outer join, e.g.: left_table right join right_table, this PR fixes the issue.

revans2

I think we have two people trying to fix the same issue. #13938 was my attempt at this for a cross join case, but this is for a regular join.

I think that both fixes are valid and needed. I just want to five a heads up

revans2 · 2025-12-05T16:59:17Z

+
+
+@allow_non_gpu('CollectLimitExec')
+def test_csv_stream_table_is_empty_when_join(std_input_path):


I think we want to coordinate with #13938 as they both are near duplicates and fix the problem is slightly different ways. I don't might having both fixes, but I don't want duplicate files checked in.

revans2 · 2025-12-05T17:00:01Z

+    with_cpu_session(lambda spark: _create_view(spark))
+
+    # then do the join on GPU
+    with_gpu_session(lambda spark:


I would prefer a test that verifies we got the right result. Not just one that shows we didn't crash.

If using .collect() instead of .show(), the error does not occur.

You can do a post project for the collect to simulate the "show" action. Just like the additional cast(c as string) at https://github.com/NVIDIA/spark-rapids/blob/main/integration_tests/src/main/python/join_test.py#L442

… 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>

Signed-off-by: Chong Gao <res_life@163.com>

res-life · 2025-12-09T04:00:06Z

build

greptile-apps

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

firestarman

LGTM, only some NITs.

firestarman · 2025-12-09T05:44:29Z

+    # create views first on CPU
+    with_cpu_session(lambda spark: create_views(spark))
+
+    # limit to 10 rows to produce `LocalLimitExec` node


NIT: Better add comment on why this limit node is needed.

firestarman · 2025-12-09T05:45:43Z


+@allow_non_gpu('CollectLimitExec')
+def test_empty_right_outer_side_with_limit(std_input_path):
+    built_csv_path = std_input_path + '/t1.csv'


NIT: Better add comment on why this file will be read as the built batch ?

Fix join bug on csv datasources

d99fc98

Signed-off-by: Chong Gao <res_life@163.com>

greptile-apps Bot reviewed Nov 28, 2025

View reviewed changes

res-life changed the base branch from main to release/25.12 November 29, 2025 07:51

sameerz added the bug Something isn't working label Nov 30, 2025

gerashegalov reviewed Dec 1, 2025

View reviewed changes

gerashegalov assigned res-life Dec 2, 2025

revans2 reviewed Dec 5, 2025

View reviewed changes

res-life mentioned this pull request Dec 8, 2025

Fix a special case in limit where it could return an empty batch with the wrong number of columns (#13938) #13971

Merged

Chong Gao added 3 commits December 9, 2025 09:07

Merge branch 'release/25.12' into fix-csv-join-bug

2beff61

Update

133c0cd

Signed-off-by: Chong Gao <res_life@163.com>

Update

2b918df

Signed-off-by: Chong Gao <res_life@163.com>

greptile-apps Bot reviewed Dec 9, 2025

View reviewed changes

firestarman approved these changes Dec 9, 2025

View reviewed changes

res-life merged commit 383d692 into NVIDIA:release/25.12 Dec 9, 2025
62 checks passed

res-life deleted the fix-csv-join-bug branch December 9, 2025 07:15



		@allow_non_gpu('CollectLimitExec')
		def test_csv_stream_table_is_empty_when_join(std_input_path):

Uh oh!

Conversation

res-life commented Nov 28, 2025

bug analysis

bug fix

Uh oh!

res-life commented Nov 28, 2025

Uh oh!

greptile-apps Bot commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

firestarman commented Nov 28, 2025

Uh oh!

res-life commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

firestarman commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

res-life commented Dec 5, 2025

Uh oh!

res-life commented Dec 5, 2025

Uh oh!

revans2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

firestarman Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

res-life commented Dec 9, 2025

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

firestarman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

greptile-apps Bot commented Nov 28, 2025 •

edited

Loading

firestarman commented Dec 2, 2025 •

edited

Loading

firestarman Dec 8, 2025 •

edited

Loading