Skip to content

feat(sql): support ignore_corrupt_files in read_csv#7264

Open
jackylee-ch wants to merge 2 commits into
Eventual-Inc:mainfrom
jackylee-ch:sql-read-csv-ignore-corrupt-files
Open

feat(sql): support ignore_corrupt_files in read_csv#7264
jackylee-ch wants to merge 2 commits into
Eventual-Inc:mainfrom
jackylee-ch:sql-read-csv-ignore-corrupt-files

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Changes Made

Adds the ignore_corrupt_files option to the SQL read_csv table function. This closes the last gap in the ignore_corrupt_files parity matrix — Python read_csv, SQL read_parquet (#7133), and iceberg (#7130) already support it, but SQL read_csv did not.

  • Wire ignore_corrupt_files through CsvScanBuilder (field, setter, and finish()CsvSourceConfig).
  • Parse the named arg in the read_csv table provider and add it to the allowed-args list.
  • Release the GIL around blocking schema inference so a skipped corrupt file's log::warn! (forwarded by pyo3-log) cannot deadlock on GIL re-acquisition. This mirrors the read_parquet fix in feat(sql): support read_parquet ignore_corrupt_files #7133 and the DataFrame scan path in daft-scan/src/python.rs.

Usage:

SELECT * FROM read_csv('/path/to/dir', ignore_corrupt_files => true)

Testing

  • New tests/sql/test_sql_table_functions.py::test_sql_read_csv_ignore_corrupt_files verifies corrupt files error by default and are skipped (with skipped_corrupt_files populated) when the option is set.
  • Existing tests/sql/ and tests/io/test_ignore_corrupt_files.py suites pass; a 40× stress loop over corrupt-file schema inference confirmed no GIL deadlock.

Related Issues

Related to #7133 (SQL read_parquet ignore_corrupt_files + GIL fix).

Adds the `ignore_corrupt_files` option to the SQL `read_csv` table
function, closing the last gap in the ignore_corrupt_files parity matrix
(Python read_csv, SQL read_parquet, and iceberg already support it).

- Wire `ignore_corrupt_files` through `CsvScanBuilder` (field, setter,
  and `finish()` -> `CsvSourceConfig`).
- Parse the named arg in the `read_csv` table provider and add it to the
  allowed-args list.
- Release the GIL around blocking schema inference so a skipped corrupt
  file's `log::warn!` (forwarded by pyo3-log) cannot deadlock on GIL
  re-acquisition, mirroring the read_parquet fix in PR Eventual-Inc#7133.

Co-Authored-By: Claude <noreply@anthropic.com>
@jackylee-ch
jackylee-ch requested a review from a team as a code owner July 14, 2026 05:55
@github-actions github-actions Bot added the feat label Jul 14, 2026
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds corrupt-file skipping to SQL read_csv. The main changes are:

  • Adds ignore_corrupt_files to CsvScanBuilder and CsvSourceConfig wiring.
  • Parses ignore_corrupt_files as a SQL named argument for read_csv.
  • Releases the Python GIL while SQL CSV schema inference blocks.
  • Adds SQL coverage for default errors and skipped corrupt-file metadata.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
src/daft-logical-plan/src/scan_builder.rs Adds the CSV corrupt-file option to the builder, keeps the default false, and passes it into the scan config.
src/daft-sql/src/table_provider/read_csv.rs Accepts the new SQL argument and detaches the Python GIL around blocking CSV schema inference.
tests/sql/test_sql_table_functions.py Adds SQL test coverage for corrupt CSV failure by default and skipping when enabled.

Reviews (1): Last reviewed commit: "feat(sql): support ignore_corrupt_files ..." | Re-trigger Greptile

end-of-file-fixer flagged a duplicate trailing blank line.

Co-Authored-By: Claude <noreply@anthropic.com>

@srilman srilman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM @jackylee-ch, just a couple of nits around generated comments.

)?;

let runtime = common_runtime::get_io_runtime(true);
// Release the GIL while blocking on async schema inference. SQL planning is reached

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reduce or remove this comment, its unnecessarily long for a simple situation.


# With ignore_corrupt_files => true the corrupt file is skipped and the good
# rows are returned. This also exercises the schema-inference glob fallback and
# the GIL release added alongside read_parquet (PR #7133).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GIL part of the comment is not relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants