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

   ### Rationale for this change
   
   Fixes #50481. `arrow::csv::TableReader`/`BlockParser` can silently mis-split 
a row when a text field contains an embedded NUL (`0x00`) byte, once the reader 
has processed enough data to switch into its SIMD "bulk filter" scanning path.
   
   ### What changes are included in this PR?
   
   `SSE42Filter::Matches` (`cpp/src/arrow/csv/lexing_internal.h`) used 
`_mm_cmpistrc`, an **implicit-length** SSE4.2 string-compare intrinsic that 
treats `0x00` as a terminator in both operands. Since the caller feeds it 8 raw 
CSV bytes at a time, a real quote/comma/newline sharing an 8-byte word with an 
embedded NUL becomes invisible to the filter — `RunBulkFilter` then trusts the 
filter's "no special chars" answer and bulk-copies the whole word, silently 
swallowing the structural character.
   
   This switches to the explicit-length `_mm_cmpestrc`, passing the true length 
of each operand (`sizeof(WordType)` for the 8-byte data word, 
`sizeof(BulkFilterType)` for the 16-byte filter — derived from the type rather 
than hardcoded, so it stays correct if this class is ever widened to a larger 
SIMD register) instead of relying on NUL-termination.
   
   Also adds a regression test 
(`BlockParser.EmbeddedNulInQuotedFieldAfterBulkFilterActivates` in 
`parser_test.cc`) that reproduces the exact trigger: enough filler rows to 
cross the bulk filter's average-bytes-per-value activation threshold, followed 
by a row whose first field has a real embedded NUL immediately before its 
closing quote.
   
   ### Are these changes tested?
   
   Yes. Verified locally (not just by inspection):
   - Reverting just the header fix while keeping the new test reproduces the 
exact predicted failure: `Expected 64 columns, got 1: "abc\0def",...` and a 
wrong row count.
   - Restoring the fix makes that test pass.
   - The full `arrow-csv-test` suite (272 tests, 47 suites — parser, chunker, 
converter, reader, writer, column_builder, column_decoder) passes with the fix, 
with no regressions.
   
   ### Are there any user-facing changes?
   
   No API changes. This only affects internal CSV byte-scanning correctness on 
SSE4.2-capable x86 builds; behavior for data without embedded NUL bytes is 
unchanged (verified via a direct intrinsic-level comparison of both 
instructions against the actual filter construction, with and without a NUL 
present).
   
   ---
   This PR (fix, test, and description) was AI-generated (Claude), under human 
review and local verification described above.


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