Developer1010x opened a new pull request, #25254:
URL: https://github.com/apache/datafusion/pull/25254
## Which issue does this PR close?
- Closes #25213.
## Rationale for this change
Setting `null_regex` on a CSV source has no effect on the data that comes
back.
A field matching the pattern is read as the literal string, and if it lands
in a
column typed as a number the query fails outright:
```
Arrow error: Parser error: Error while parsing value 'N/A' as type 'Int64'
for column 1 at line 2. Row data: '[2,N/A]'
```
That second case is the one that matters in practice — `N/A`, `NULL` and `-`
placeholders are almost always sitting in columns that are otherwise numeric,
which is the reason to reach for `null_regex` in the first place.
The option was applied to the `arrow::csv::reader::Format` used by
`CsvFormat::infer_schema`, but never to the `ReaderBuilder` that parses the
rows. Inference therefore behaves as though the regex were honored and only
the
data disagrees, which is what makes it easy to miss.
## What changes are included in this PR?
- `CsvSource::null_regex()`, alongside the existing `escape()`, `comment()`
and
`terminator()` accessors.
- `CsvSource::builder()` applies the regex, and now returns `Result` so an
invalid pattern surfaces as a configuration error instead of panicking.
`CsvFormat::infer_schema` currently `.expect()`s on the same regex, so a
malformed pattern is a panic there today; this side no longer adds a second
one.
- Three call sites updated for the new signature: `CsvSource::open`, and the
two
streaming decoder paths in `CsvOpener::open` (the byte-range branch and the
`GetResultPayload::Stream` branch). Those two go through `builder()` as
well,
so they were missing the regex for the same reason.
The fix is four lines; the signature change is what makes it touch more.
## What is the testing strategy for this PR?
There was no coverage of `null_regex` anywhere in the repository before this
change — not in `datafusion/datasource-csv`, not in the sqllogictest files.
Unit tests in `datafusion/datasource-csv/src/source.rs`:
- `null_regex_nulls_matching_string_values`
- `null_regex_nulls_matching_values_in_numeric_columns` — the case that
errors
today rather than returning a wrong value
- `without_null_regex_the_placeholder_is_read_verbatim` — control, pins that
the
unset behavior does not change
- `invalid_null_regex_is_reported_as_an_error`
I checked these are not vacuous: with the four added lines in `builder()`
removed, the three behavioral tests fail and the control still passes.
SQL-level coverage in `datafusion/sqllogictest/test_files/csv_files.slt`,
over a
new `datafusion/core/tests/data/null_regex.csv` fixture, covering a matching
value in both a `VARCHAR` and an `INT` column.
## Are there any user-facing changes?
`null_regex` starts doing what it is documented to do. Anything that set the
option and worked around it being ignored — for example by typing a column as
`VARCHAR` to avoid the parse error, then filtering the placeholder out in
SQL —
will now see NULL where it previously saw the literal string.
No public API changes: `builder()` is private, and the new `null_regex()`
accessor is additive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
