iliasabk opened a new pull request, #51394:
URL: https://github.com/apache/arrow/pull/51394

   ### Rationale for this change
   
   With `newlines_in_values = true`, a quoted CRLF sequence split across two 
input buffers was silently corrupted: `"…\r` at the end of one buffer and `\n…` 
at the start of the next lost the `\n` (it stayed inside the field as `\r` 
only). Reported in #51368.
   
   The root cause is in `CSVBufferIterator`, which unconditionally skipped a 
leading `\n` whenever the previous buffer ended with `\r`. At that layer there 
is no quoting context, so the code cannot distinguish a `\r\n` that is a 
physical line separator from `\r\n` that is the contents of an unfinished 
quoted field.
   
   ### What changes are included in this PR?
   
   - `CSVBufferIterator` no longer resolves straddling CRLF sequences (it still 
strips a leading UTF-8 BOM).
   - `BlockReader` now owns the decision via `trailing_cr_` + a 
`SkipStraddlingCRLF` helper: the next buffer's leading `\n` is skipped only 
when the previous buffer's trailing `\r` was fully consumed as a line 
separator. When the `\r` belongs to an unfinished quoted field, it is carried 
in `partial_` (non-empty), the flag stays false, and the `\n` is preserved as 
field contents.
   - The flag is updated in both `SerialBlockReader` (through `consume_bytes`, 
driven by how much of the buffer the parser consumed) and `ThreadedBlockReader` 
(through `next_partial` from the chunker), including the row-skipping paths. 
Empty buffers never overwrite the state.
   - An initial `prev_ended_cr` flag is plumbed through the block-reader 
factories for the edge case where a header row ends exactly at a buffer 
boundary with `\r` (the post-header buffer is then empty and the next buffer 
starts with `\n`).
   - New regression test `TestStraddlingCRLF` covering a quoted `\r\n` split 
across the boundary, a line-separator `\r\n` split across the boundary, and a 
header row ending with `\r` at the boundary. Registered for the serial, 
threaded-async and streaming readers.
   
   ### Are these changes tested?
   
   Yes — a new `StraddlingCRLF` test in `csv/reader_test.cc` covers all three 
cases above with a block size that forces the split. (Note: the change was 
developed and cross-validated against a transliteration of the 
reader/chunker/parser pipeline; the gtest cases should be run in CI.)
   
   ### Are there any user-facing changes?
   
   No API changes. Files with `newlines_in_values` enabled whose quoted fields 
contain `\r\n` straddling a read block boundary are now parsed correctly 
instead of losing the `\n`.
   
   Closes #51368


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

Reply via email to