-
Notifications
You must be signed in to change notification settings - Fork 292
Fix join bug on csv datasources #13903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -485,6 +485,28 @@ def do_join(spark): | |
| return t0.crossJoin(t1).limit(21) | ||
| assert_gpu_and_cpu_are_equal_collect(do_join) | ||
|
|
||
| @allow_non_gpu('CollectLimitExec') | ||
| def test_empty_right_outer_side_with_limit(std_input_path): | ||
| built_csv_path = std_input_path + '/t1.csv' | ||
| stream_csv_path = std_input_path + '/t0.csv' | ||
|
|
||
| def create_views(spark): | ||
| spark.read.csv(built_csv_path, header=True, inferSchema=True).createOrReplaceTempView("built_table") | ||
| spark.read.csv(stream_csv_path, header=True, inferSchema=True).createOrReplaceTempView("stream_table") | ||
|
|
||
| # create views first on CPU | ||
| with_cpu_session(lambda spark: create_views(spark)) | ||
|
|
||
| # limit to 10 rows to produce `LocalLimitExec` node | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Better add comment on why this limit node is needed. |
||
| def do_join(spark): | ||
| return spark.sql(""" | ||
| SELECT '1', CAST(CAST(stream_table.c0 AS int) as string) | ||
| FROM built_table | ||
| RIGHT OUTER JOIN stream_table | ||
| ON TRUE limit 10 | ||
| """) | ||
| assert_gpu_and_cpu_are_equal_collect(do_join) | ||
|
|
||
| # local sort because of https://github.com/NVIDIA/spark-rapids/issues/84 | ||
| # After 3.1.0 is the min spark version we can drop this | ||
| @ignore_order(local=True) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -643,7 +643,14 @@ abstract class GpuTextBasedPartitionReader[BUFF <: LineBufferer, FACT <: LineBuf | |
| // val cols = (0 until table.getNumberOfColumns).map(i => table.getColumn(i)) | ||
| // Some(new Table(cols: _*)) | ||
| // } | ||
| Some(table) | ||
| 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() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any concerns that this code can throw instead of returning
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The row count is zero, it means there is no GPU memory allocated although table has columns. |
||
| None | ||
| } else { | ||
| Some(table) | ||
| } | ||
| } | ||
|
|
||
| override def next(): Boolean = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Better add comment on why this file will be read as the built batch ?