AndreaBozzo opened a new pull request, #10579:
URL: https://github.com/apache/arrow-rs/pull/10579
# 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.
--
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]
