Report the number of rows padded by with_truncated_rows - #10579
Report the number of rows padded by with_truncated_rows#10579AndreaBozzo wants to merge 1 commit into
with_truncated_rows#10579Conversation
`with_truncated_rows(true)` repairs a row with fewer fields than the schema by padding it and reports nothing. The information cannot be recovered after decoding, because a padded field is byte-identical to a genuinely empty trailing field once `NullRegex` has turned both into nulls. Count the rows padded in `RecordDecoder::decode` and expose the total as `truncated_row_count()` on `Decoder` and `BufReader`, and therefore `Reader`. The count is cumulative rather than per batch, so `flush` leaves it alone. `clear` resets it, which keeps skipped rows such as a short header out of the total, since those are discarded rather than read into a batch.
|
One decision here is worth flagging separately, since the issue did not cover it. Skipped rows go through If you prefer that, it is a one line revert of the reset in |
Which issue does this PR close?
Closes #10577.
Rationale for this change
with_truncated_rows(true)repairs a row that has fewer fields than the schema bypadding it, and reports nothing about having done so. For a consumer that reports on
data quality, a repaired parse and a clean parse are different outcomes, and today
they are indistinguishable.
The count cannot be recovered after decoding. Padding fills offsets to produce
zero-length fields, and
NullRegexlater turns those into nulls, so a padded fieldis byte-identical to a genuinely empty trailing field:
Disabling the null regex does not help, both cases become
""and stay identical.Nothing on
ReaderBuilder,Format,Reader,BufReaderorDecoderexposes theinformation either.
This came out of dataprof, where the Arrow-backed CSV engine was the only path that
reported a file of short rows with a perfect consistency score. The workaround
shipped there is a pre-scan with the
csvcrate purely to recover the count, whichcosts a second full read of the file. On a 218 MB, 2M-row file that measured at
roughly 3% of profiling wall time, because per-value analysis dominates the parse.
So the workaround is viable, but it is a whole extra pass to recover a number the
decoder already had and threw away.
What changes are included in this PR?
A single counter, threaded up to the public types.
arrow-csv/src/reader/records.rs:RecordDecodergains atruncated_row_countfield, incremented in the one branchof
decodethat pads a short row.RecordDecoder::truncated_row_count()returns it.flushdeliberately leaves the counter alone, so it accumulates across batches.clearresets it, becausecleardiscards the buffered rows the count refers to.That is what keeps skipped rows out of the total, see below.
arrow-csv/src/reader/mod.rs:Decoder::truncated_row_count()andBufReader::truncated_row_count()forward it.Reader<R>is an alias forBufReader<StdBufReader<R>>, so the accessor coversboth.
Two semantics worth calling out, both documented on the accessors:
RecordDecoderis reused acrossflushcalls and the counter survives them. Reading it between batches gives arunning total of the rows decoded so far, reading it after the input is exhausted
gives the total for the whole input. It is meaningful at either point as long as
that is understood, so it is stated rather than restricted.
bound go through
RecordDecoder::decodetoo, and would otherwise be counted ifthey were short. They are discarded via
clear, which now resets the counter withthem, so the total only ever covers rows that reached a batch.
Are these changes tested?
Yes, at both levels, and each test was confirmed to fail against unpatched code.
records.rs:test_truncated_rowsextended to assert the count.test_truncated_row_count_not_reset_by_flush.test_truncated_row_count_reset_by_clear.mod.rs:test_truncated_row_count_counts_padded_rows, one short row, count is 1.test_truncated_row_count_ignores_empty_trailing_field, a row with a genuinelyempty trailing field produces the same null as a padded row but the count stays 0.
This is the case that proves the count is not inferred from nulls.
test_truncated_row_count_clean_file, count is 0.test_truncated_row_count_without_truncated_rows, the short row errors as beforeand the accessor still returns 0.
test_truncated_row_count_accumulates_across_batches,batch_size2 over 6 shortrows, asserting the running total is 2, 4, 6 rather than 2 each time.
test_truncated_row_count_excludes_skipped_rows, a header shorter than the schemais padded while being skipped and does not count.
test_truncated_row_count_on_decoder, the push-basedDecoderpath.The two parts of the change, the increment and the
clearreset, were revertedseparately to confirm each is independently covered.
cargo test -p arrow-csv,cargo fmt --allandcargo clippy -p arrow-csv --all-targets -- -D warningsare clean.Are there any user-facing changes?
Yes, one additive accessor,
truncated_row_count(), onBufReader(and thereforeReader) and onDecoder. There are no breaking changes, no behaviour changes toparsing, and no new configuration.