You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/08/24 16:25:17 UTC

[I] CsvSource::with_terminator is silently dropped when a physical plan is serialized [datafusion]

adriangb opened a new issue, #24609:
URL: https://github.com/apache/datafusion/issues/24609

   ### Describe the bug
   
   `CsvScanExecNode` has no field for the CSV line terminator, so the `terminator` set via `CsvSource::with_terminator` (or `CsvOptions::terminator`) is silently dropped when a physical plan is serialized with `datafusion-proto`. The decoded `CsvSource` comes back with `terminator: None` and parses with the default line terminator.
   
   This is the same failure mode as `HashJoinExec::fetch` in #24165: `CsvSource::try_to_proto` reads plan state through getters, so a field nobody wired into the encoder is simply never written and nothing fails at compile time. `Debug`-comparing round-trip tests do not catch it either.
   
   Impact: a CSV source that needs a non-default terminator — e.g. `OPTIONS ('format.terminator' E'\r')`, which DataFusion supports and tests in `datafusion/core/src/datasource/physical_plan/csv.rs` — is read with the wrong terminator once the plan has crossed a serialization boundary.
   
   The other scan-relevant CSV options all round-trip correctly. `CsvSource::builder` consumes `has_header`, `delimiter`, `quote`, `terminator`, `escape`, `comment` and `truncated_rows` (plus `newlines_in_values` for repartitioning); `CsvScanExecNode` carries every one of them except `terminator`.
   
   ### To Reproduce
   
   Add a terminator to the existing round-trip test (`datafusion/proto/tests/cases/plans/sources.rs`, `roundtrip_csv_scan_preserves_format_options`):
   
   ```rust
   Arc::new(CsvSource::new(table_schema).with_csv_options(CsvOptions {
       // ... existing options ...
       terminator: Some(b';'),
       ..Default::default()
   }));
   
   // after the round trip:
   assert_eq!(csv_source.terminator(), Some(b';'));
   ```
   
   On `main` this fails with `assertion `left == right` failed: left: None, right: Some(59)`.
   
   ### Expected behavior
   
   `terminator` survives serialization like every other CSV scan option.
   
   ### Additional context
   
   Found while applying the destructuring convention from #24164 to `CsvSource::try_to_proto`: an exhaustive `let Self { .. }` / `let CsvOptions { .. }` in the encoder makes an unserialized field impossible to miss.
   
   I have a fix ready (adds `oneof optional_terminator` as field 9 of `CsvScanExecNode`, wires both sides, and extends the round-trip test); PR to follow.
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] CsvSource::with_terminator is silently dropped when a physical plan is serialized [datafusion]

Posted by "DevShiba (via GitHub)" <gi...@apache.org>.
DevShiba commented on issue #24609:
URL: https://github.com/apache/datafusion/issues/24609#issuecomment-5570195697

   This looks already fixed by #24233 (merged 2026-08-26), which closed #24180 but didn't reference this issue. Verified on current `main`: `CsvSource::try_to_proto` now serializes `terminator` (`source.rs` around line 359) and `try_from_proto` reads it back and applies `with_terminator` (around line 650). Looks safe to close as a duplicate of #24180.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] CsvSource::with_terminator is silently dropped when a physical plan is serialized [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb closed issue #24609: CsvSource::with_terminator is silently dropped when a physical plan is serialized
URL: https://github.com/apache/datafusion/issues/24609


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org