-
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 1 commit
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 |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| from datetime import datetime, timezone | ||
| from data_gen import * | ||
| from marks import * | ||
| from pyspark.sql import functions as f | ||
| from pyspark.sql.types import * | ||
| from spark_session import * | ||
|
|
||
|
|
@@ -720,3 +721,22 @@ def test_csv_read_gbk_encoded_data(std_input_path): | |
| .schema("name string, age int, city string, job string") | ||
| .csv(std_input_path + "/test_gbk.csv"), | ||
| conf={"spark.sql.legacy.javaCharsets": legacy_charset}) | ||
|
|
||
|
|
||
| @allow_non_gpu('CollectLimitExec') | ||
| def test_csv_stream_table_is_empty_when_join(std_input_path): | ||
| built_csv_path = std_input_path + '/one_row.csv' | ||
| stream_csv_path = std_input_path + '/empty_with_header.csv' | ||
|
|
||
| def _create_view(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 view first on CPU | ||
| with_cpu_session(lambda spark: _create_view(spark)) | ||
|
|
||
| # then do the join on GPU | ||
| with_gpu_session(lambda spark: | ||
|
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. I would prefer a test that verifies we got the right result. Not just one that shows we didn't crash.
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. If using .collect() instead of .show(), the error does not occur.
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. You can do a post project for the
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. done |
||
| spark.table("built_table").join(spark.table("stream_table"), f.lit(True), "right_outer").select( | ||
| f.col("stream_table.c0")).show(), | ||
| conf=_enable_all_types_conf) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| c0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| c0 | ||
| 1 |
| 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.
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.