apache / apache/datafusion

Invalid CSV null_regex panics the query task instead of returning an error

Closed
#25,260 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

A malformed `null_regex` panics the query task instead of returning an error.
`CsvFormat::infer_schema_from_stream` compiles the pattern with

```rust
// datafusion/datasource-csv/src/file_format.rs
let regex = Regex::new(null_regex.as_str())
.expect("Unable to parse CSV null regex.");
```

so any pattern the `regex` crate rejects aborts the task rather than surfacing
as a `DataFusionError`. It is reachable straight from SQL, from a `CREATE
EXTERNAL TABLE` that does not spell out its columns, since that is what makes
schema inference run.

### To Reproduce

```sql
CREATE EXTERNAL TABLE bad_regex
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
)"
```

Reproduced on main (`9082d6b`) through the sqllogictest harness. Any invalid
pattern does it; `(` is just the shortest.

### Expected behavior

An invalid `null_regex` is a bad option value, so it should come back as an
error naming the offending pattern — the same way other malformed CSV options
are handled — and leave the session usable.

### Additional context

Noticed while working on #25213 / #25254. That PR changes `CsvSource::builder`
to return `Result` for the same reason on the read side, so it does not add a
second panic; this one is on the inference side and is independent of it.

The regex is also recompiled for every chunk inside the inference loop, so
hoisting the compile out is both the fix and a small saving.

I have a fix ready and will open a PR shortly.

Contributor guide

Open the contributing guide

Research direction

Start in datafusion/datasource-csv/src/file_format.rs at CsvFormat::infer_schema_from_stream and inspect how malformed CSV options are reported elsewhere. Reproduce the issue through the sqllogictest harness using the shown CREATE EXTERNAL TABLE statement. Done means the invalid pattern returns a DataFusionError naming the pattern, does not panic, leaves the session usable, and avoids recompiling it for every chunk.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.