-
Notifications
You must be signed in to change notification settings - Fork 4.3k
GH-51275: [C++] Allow the CSV Reader to accept multi-character delimiters - #51286
GH-51275: [C++] Allow the CSV Reader to accept multi-character delimiters #51286HuaHuaY wants to merge 2 commits into
Conversation
HuaHuaY
commented
Sep 10, 2026
@ursabot please benchmark lang=C++
rok
commented
Sep 10, 2026
Benchmark runs are scheduled for commit 31adcd2. Watch https://buildkite.com/apache-arrow and https://conbench.arrow-dev.org for updates. A comment will be posted here when the runs are complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
CsvFileFormat::Equals() currently compares delimiter even when delimiter_string overrides it, which can produce incorrect inequality for semantically identical parse options.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the Arrow C++ CSV reader to support multi-character field delimiters by introducing a new ParseOptions::delimiter_string that overrides the existing single-character delimiter, and updating the parser/chunker accordingly.
Changes:
- Add
ParseOptions::delimiter_string(higher priority thandelimiter) and validate delimiter interactions with quoting/escaping/line endings. - Implement multi-character delimiter matching in both the CSV parser and the chunker (lexing boundary finder).
- Add unit tests covering multi-character delimiters across reader, parser, and chunker.
File summaries
| File | Description |
|---|---|
| cpp/src/arrow/dataset/file_csv.cc | Include delimiter_string in format equality; reject CSV writing when delimiter_string is set. |
| cpp/src/arrow/csv/options.h | Add ParseOptions::delimiter_string with override semantics. |
| cpp/src/arrow/csv/options.cc | Extend ParseOptions::Validate() for delimiter-string constraints (escaping/quoting/CRLF). |
| cpp/src/arrow/csv/lexing_internal.h | Add delimiter helpers and a streaming delimiter matcher; adjust bulk filter delimiter byte selection. |
| cpp/src/arrow/csv/chunker.cc | Support multi-character delimiter detection (for quote recognition) during chunking when needed. |
| cpp/src/arrow/csv/parser.cc | Add multi-character delimiter matching in the field parser state machine. |
| cpp/src/arrow/csv/reader_test.cc | Add end-to-end reader test for multi-character delimiters. |
| cpp/src/arrow/csv/parser_test.cc | Add parser tests for multi-character delimiters, delimiter-prefix-at-block-end, and single-char delimiter_string. |
| cpp/src/arrow/csv/chunker_test.cc | Add chunker test for multi-character delimiter behavior with partial buffers. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether semantic equality is what we need here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
It changes core CSV parsing/chunking state machines and delimiter handling, which warrants final human review for subtle correctness/performance edge cases.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
HuaHuaY
commented
Sep 10, 2026
@ursabot please benchmark lang=C++
rok
commented
Sep 10, 2026
Benchmark runs are scheduled for commit a161eff. Watch https://buildkite.com/apache-arrow and https://conbench.arrow-dev.org for updates. A comment will be posted here when the runs are complete.
Uh oh!
There was an error while loading. Please reload this page.
Rationale for this change
Now the type of
delimiterinParseOptionsischar, and we can only read csv files whose delimiter is one character. I think we can lift this restriction.What changes are included in this PR?
Added a higher-priority
delimiter_stringtoParseOptions, and added support for multi-character delimiters in both the chunker and the parser.Are these changes tested?
Yes.
Are there any user-facing changes?
Yes. A new option
delimiter_stringis added toParseOptions.