You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/15 05:24:35 UTC
(datafusion) 01/01: fix: return an error for an invalid CSV null_regex instead of panicking (#25261)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25261-745b1ec218d4836ea936a8a89d0cbcc691bbcc05
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 98d84121cd864e683d24d6405c0b2e95fd7e41a6
Author: S Prajwall Narayana <11...@users.noreply.github.com>
AuthorDate: Tue Sep 15 05:24:13 2026 +0000
fix: return an error for an invalid CSV null_regex instead of panicking (#25261)
## Which issue does this PR close?
- Closes #25260.
## Rationale for this change
A malformed `null_regex` panics the query task instead of returning an
error.
`CsvFormat::infer_schema_from_stream` compiled the pattern with
```rust
let regex = Regex::new(null_regex.as_str())
.expect("Unable to parse CSV null regex.");
```
so any pattern the `regex` crate rejects aborted the task. It is
reachable
straight from SQL, through a `CREATE EXTERNAL TABLE` that leaves its
columns to
schema inference:
```sql
CREATE EXTERNAL TABLE t STORED AS CSV LOCATION 'data.csv'
OPTIONS ('format.has_header' 'true', 'format.null_regex' '(');
```
```
task 9 panicked with message "Unable to parse CSV null regex.: Syntax(
regex parse error:
(
^
error: unclosed group
)"
```
An invalid regex is a bad option value, not an internal invariant, so it
should
come back as an error naming the pattern.
## What changes are included in this PR?
The regex is compiled once, before the per-chunk loop, and a failure is
propagated with `exec_datafusion_err!` rather than panicking. Hoisting
it also
stops the pattern being recompiled for every chunk of the inference
stream.
The error text matches the one used on the read side in #25254, so the
same bad
option reads the same whichever path hits it first.
## What is the testing strategy for this PR?
A `statement error Unable to parse CSV null regex` case in
`datafusion/sqllogictest/test_files/csv_files.slt`.
I checked it is not vacuous: with this change reverted, that case fails
with the
panic quoted above rather than an error, so the test reproduces the bug.
`cargo fmt --check`, `cargo clippy --all-targets` and the crate's unit
tests are
clean.
## Are there any user-facing changes?
An invalid `null_regex` now produces an error and leaves the session
usable,
where it previously panicked the task. No API changes.
Independent of #25254 — that one is the read path in `source.rs`, this
is the
inference path in `file_format.rs` — but they touch the same crate, so
whichever
lands second may want a trivial rebase.
Co-authored-by: Prajwal Narayana <sp...@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
datafusion/datasource-csv/src/file_format.rs | 25 ++++++++++++++++++------
datafusion/sqllogictest/test_files/csv_files.slt | 8 ++++++++
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/datafusion/datasource-csv/src/file_format.rs b/datafusion/datasource-csv/src/file_format.rs
index a8c27369cb..1f7da7e849 100644
--- a/datafusion/datasource-csv/src/file_format.rs
+++ b/datafusion/datasource-csv/src/file_format.rs
@@ -30,8 +30,8 @@ use arrow::error::ArrowError;
use datafusion_common::config::{ConfigField, ConfigFileType, CsvOptions};
use datafusion_common::file_options::csv_writer::CsvWriterOptions;
use datafusion_common::{
- DEFAULT_CSV_EXTENSION, DataFusionError, GetExt, Result, Statistics, exec_err,
- not_impl_err,
+ DEFAULT_CSV_EXTENSION, DataFusionError, GetExt, Result, Statistics,
+ exec_datafusion_err, exec_err, not_impl_err,
};
use datafusion_common_runtime::SpawnedTask;
use datafusion_datasource::TableSchema;
@@ -540,6 +540,21 @@ impl CsvFormat {
let mut record_number = -1;
let initial_records_to_read = records_to_read;
+ // Compile once rather than per chunk, and report a malformed pattern
+ // instead of panicking on it.
+ let null_regex = self
+ .options
+ .null_regex
+ .as_ref()
+ .map(|null_regex| {
+ Regex::new(null_regex).map_err(|e| {
+ exec_datafusion_err!(
+ "Unable to parse CSV null regex '{null_regex}': {e}"
+ )
+ })
+ })
+ .transpose()?;
+
pin_mut!(stream);
while let Some(chunk) = stream.next().await.transpose()? {
@@ -557,10 +572,8 @@ impl CsvFormat {
.with_quote(self.options.quote)
.with_truncated_rows(self.options.truncated_rows.unwrap_or(false));
- if let Some(null_regex) = &self.options.null_regex {
- let regex = Regex::new(null_regex.as_str())
- .expect("Unable to parse CSV null regex.");
- format = format.with_null_regex(regex);
+ if let Some(regex) = &null_regex {
+ format = format.with_null_regex(regex.clone());
}
if let Some(escape) = self.options.escape {
diff --git a/datafusion/sqllogictest/test_files/csv_files.slt b/datafusion/sqllogictest/test_files/csv_files.slt
index b5b6130940..3562383c8b 100644
--- a/datafusion/sqllogictest/test_files/csv_files.slt
+++ b/datafusion/sqllogictest/test_files/csv_files.slt
@@ -22,6 +22,14 @@ STORED AS CSV
LOCATION '../core/tests/data/duplicate_header.csv'
OPTIONS ('format.has_header' 'true');
+# A malformed null_regex is reported as an error rather than panicking the
+# query. Schema inference compiles the pattern, so CREATE is where it surfaces.
+statement error Unable to parse CSV null regex
+CREATE EXTERNAL TABLE csv_with_invalid_null_regex
+STORED AS CSV
+LOCATION '../core/tests/data/quote.csv'
+OPTIONS ('format.has_header' 'true', 'format.null_regex' '(');
+
# create_external_table_with_quote_escape
statement ok
CREATE EXTERNAL TABLE csv_with_quote (
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org