adriangb opened a new issue, #11172:
URL: https://github.com/apache/arrow-rs/issues/11172
**Describe the bug**
When `WriterProperties::set_write_row_group_number_distinct_values(true)` is
set, `ArrowWriter` writes a wrong `distinct_count` for dictionary-encoded
columns.
`ArrowColumnWriter::write_internal` hashes the dictionary keys instead of
the dictionary values that the keys point to.
This is wrong for three reasons.
Each `RecordBatch` can carry its own dictionary, so the same key value in
two batches can point at two different values, and the same value can appear
under two different keys.
A dictionary can also contain duplicate values under different keys.
A valid key can point at a null dictionary value, and that gets counted as a
distinct value even though the row itself is not null.
**To Reproduce**
```rust
use std::sync::Arc;
use arrow_array::{ArrayRef, DictionaryArray, RecordBatch, StringArray,
types::Int32Type};
use bytes::Bytes;
use parquet::arrow::ArrowWriter;
use parquet::file::properties::WriterProperties;
use parquet::file::reader::{FileReader, SerializedFileReader};
fn ndv(batches: Vec<ArrayRef>) -> Option<u64> {
let schema = RecordBatch::try_from_iter([("c",
batches[0].clone())]).unwrap().schema();
let props =
WriterProperties::builder().set_write_row_group_number_distinct_values(true).build();
let mut buf = Vec::new();
let mut w = ArrowWriter::try_new(&mut buf, schema.clone(),
Some(props)).unwrap();
for a in batches {
w.write(&RecordBatch::try_new(schema.clone(),
vec![a]).unwrap()).unwrap();
}
w.close().unwrap();
let r = SerializedFileReader::new(Bytes::from(buf)).unwrap();
r.metadata().row_group(0).column(0).statistics().and_then(|s|
s.distinct_count_opt())
}
fn dict(keys: Vec<i32>, values: Vec<&str>) -> ArrayRef {
Arc::new(DictionaryArray::<Int32Type>::try_new(keys.into(),
Arc::new(StringArray::from(values))).unwrap())
}
fn main() {
// Two batches, each with its own dictionary, same key values (0, 1),
four different string values.
println!("{:?}", ndv(vec![dict(vec![0, 1], vec!["a", "b"]), dict(vec![0,
1], vec!["c", "d"])]));
// Two batches, same two string values, different key values.
println!("{:?}", ndv(vec![dict(vec![0, 1], vec!["a", "b"]), dict(vec![2,
3], vec!["x", "y", "a", "b"])]));
// One batch, two keys pointing at the same duplicated dictionary value.
println!("{:?}", ndv(vec![dict(vec![0, 1], vec!["a", "a"])]));
}
```
Actual output, on `main` at commit
`70fa5bcf21924c5a9ea34ee2df846ffae43f2bc6`:
```
different dicts, same keys: Some(2) // should be 4
same values, different keys: Some(4) // should be 2
duplicate dict entries: Some(2) // should be 1
```
A plain (non-dictionary) `StringArray` with the same values gives the
correct count, `Some(4)`, for comparison.
A dictionary array with a valid key that points at a null dictionary value
also over-counts: a single-row-group file with dictionary values `["a", null]`
and keys `[0, 1, 0]` (no key is itself null) gives `distinct_count = 2` instead
of the correct `1`.
**Expected behavior**
`distinct_count` for a dictionary-encoded column should equal the number of
distinct non-null logical values across the row group, the same as it would for
the equivalent plain-encoded column.
**Additional context**
The bug is in `ArrowColumnWriter::write_internal`,
[parquet/src/arrow/arrow_writer/mod.rs#L1146-L1163](https://github.com/apache/arrow-rs/blob/70fa5bcf21924c5a9ea34ee2df846ffae43f2bc6/parquet/src/arrow/arrow_writer/mod.rs#L1146-L1163).
The comment there, "Key cardinality equals value cardinality", is not true
across batches, and not true for a dictionary with duplicate values.
This was introduced by #10654, which closed #8608.
The wrong count also has a real downstream effect in DataFusion, inferred
from reading the code and not yet confirmed end to end.
Since apache/datafusion#19957 and apache/datafusion#20845, DataFusion's
`summarize_distinct_counts` (`datasource-parquet/src/metadata.rs`) marks a
Parquet file's `distinct_count` as `Precision::Exact` whenever the file has a
single row group, and `Count::value_from_stats`
(`functions-aggregate/src/count.rs`) then returns that value directly as the
answer to `COUNT(DISTINCT col)`, without scanning any rows.
A single-row-group Parquet file written from a dictionary array with this
writer option on can therefore make DataFusion return a wrong `COUNT(DISTINCT
col)`.
Two smaller issues in the same code path are worth noting, though they are
not the main bug.
Values are hashed into a 64-bit `XxHash64` value stored in a `HashSet<u64>`,
so two distinct values that collide to the same hash will under-count; the
probability of this is negligible in practice.
Values are hashed from their raw bytes, so `-0.0` and `+0.0` count as two
distinct values, and different `NaN` bit patterns count as separate distinct
values, instead of being treated as equal.
Two possible fixes: hash the dictionary values that each key actually points
to, for example by resolving and hashing each distinct value referenced in a
batch once per batch instead of hashing the raw key bytes; or, more simply,
skip distinct-value counting for dictionary-encoded columns until a correct
approach is implemented.
cc @Rich-T-kid, who authored #10654.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
