Developer1010x opened a new pull request, #25261:
URL: https://github.com/apache/datafusion/pull/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.
   


-- 
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]

Reply via email to