Skip to content

Report the number of rows padded by with_truncated_rows - #10579

Open
AndreaBozzo wants to merge 1 commit into
apache:mainfrom
AndreaBozzo:csv-truncated-row-count
Open

Report the number of rows padded by with_truncated_rows#10579
AndreaBozzo wants to merge 1 commit into
apache:mainfrom
AndreaBozzo:csv-truncated-row-count

Conversation

@AndreaBozzo

Copy link
Copy Markdown
Contributor

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 by
padding 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 NullRegex later turns those into nulls, so a padded field
is byte-identical to a genuinely empty trailing field:

name,age,city
Alice,25,Rome  ->  ["Alice", "25", "Rome"]
Carol,35,      ->  ["Carol", "35", NULL]   three fields, the last one empty
Bob,30         ->  ["Bob",   "30", NULL]   two fields, padded

Disabling the null regex does not help, both cases become "" and stay identical.
Nothing on ReaderBuilder, Format, Reader, BufReader or Decoder exposes the
information 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 csv crate purely to recover the count, which
costs 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:

  • RecordDecoder gains a truncated_row_count field, incremented in the one branch
    of decode that pads a short row.
  • RecordDecoder::truncated_row_count() returns it.
  • flush deliberately leaves the counter alone, so it accumulates across batches.
    clear resets it, because clear discards 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() and BufReader::truncated_row_count() forward it.
    Reader<R> is an alias for BufReader<StdBufReader<R>>, so the accessor covers
    both.

Two semantics worth calling out, both documented on the accessors:

  • The count is cumulative, not per batch. RecordDecoder is reused across
    flush calls and the counter survives them. Reading it between batches gives a
    running 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.
  • Skipped rows do not contribute. The header row and any rows before the start
    bound go through RecordDecoder::decode too, and would otherwise be counted if
    they were short. They are discarded via clear, which now resets the counter with
    them, 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_rows extended 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 genuinely
    empty 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 before
    and the accessor still returns 0.
  • test_truncated_row_count_accumulates_across_batches, batch_size 2 over 6 short
    rows, 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 schema
    is padded while being skipped and does not count.
  • test_truncated_row_count_on_decoder, the push-based Decoder path.

The two parts of the change, the increment and the clear reset, were reverted
separately to confirm each is independently covered.

cargo test -p arrow-csv, cargo fmt --all and
cargo clippy -p arrow-csv --all-targets -- -D warnings are clean.

Are there any user-facing changes?

Yes, one additive accessor, truncated_row_count(), on BufReader (and therefore
Reader) and on Decoder. There are no breaking changes, no behaviour changes to
parsing, and no new configuration.

`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.
@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-csv labels Aug 7, 2026
@AndreaBozzo

Copy link
Copy Markdown
Contributor Author

One decision here is worth flagging separately, since the issue did not cover it.

Skipped rows go through RecordDecoder::decode as well, so a header row shorter than the schema gets padded like any other short row. I chose not to count those: Decoder::decode discards skipped rows via clear(), and clear() now resets the counter along with the buffered rows it refers to, so the total only covers rows that reached a batch. That seemed right for the reporting use case, but the opposite reading is defensible, that the count should be a raw tally of every row the decoder padded.

If you prefer that, it is a one line revert of the reset in clear() plus a doc change, and test_truncated_row_count_excludes_skipped_rows inverts to assert 1. Happy to switch it either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate arrow-csv enhancement Any new improvement worthy of a entry in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

arrow-csv: no way to tell that with_truncated_rows padded a row

2 participants