compiler-segfault opened a new pull request, #27283: URL: https://github.com/apache/datafusion/pull/27283
- Closes #26490. ## Rationale for this change A plain `SELECT` of a Parquet `Float32`/`Float64` column can return values that are not in the file: rows holding `NaN` come back as the statistics constant. Parquet statistics exclude `NaN` from `min`/`max`, so a chunk holding `1.0, NaN, 1.0` reports `min == max == 1.0` with `null_count == 0`. The skip-scan path in `datafusion/datasource-parquet` treats `min == max` as proof that the column is constant and substitutes the statistic for the data, so the `NaN` row is silently rewritten as `1.0`. `-0.0` and `0.0` are affected the same way, because they compare equal. `COPY ... TO` from such a table writes the wrong values too. ## What changes are included in this PR? - `constant_value_from_stats` no longer derives a constant from `min == max` when the column type (or a dictionary's value type) is floating point. The all-null case is unchanged. - Regression test `datafusion/core/tests/parquet/constant_columns.rs`, which writes a Parquet file with `1.0, NaN, 1.0` and reads it back through `SessionContext`. `min == max` still proves constancy for exact types, so the shortcut is only given up where it is not a proof. ## What is the testing strategy for this PR? - The new test fails before the change (`[1.0, 1.0, 1.0]` instead of `[1.0, NaN, 1.0]`) and passes after. - `cargo test -p datafusion --test parquet_integration` (263 passed) and `cargo test -p datafusion-datasource-parquet --lib` (295 passed) are green, along with `cargo fmt --all -- --check` and `cargo clippy -p datafusion-datasource-parquet --all-targets -- -D warnings`. ## Are there any user-facing changes? No API change. Floating point columns whose statistics happen to have `min == max` are now read from the file instead of being replaced by a constant; that costs a read of those chunks and returns the correct values. ## AI Disclosure This change was prepared with an AI coding assistant; I reproduced the bug locally and reviewed the diff end to end, and can answer questions about it. Open question I would flag for review: the alternative is to plumb Parquet's `nan_count` into `ColumnStatistics` and keep the shortcut for float columns that report `nan_count == 0`. That is a public API change, and files written by older writers do not carry the count, so this PR takes the conservative route instead. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
