[Parquet] Prototype: PARQUET-2249: Introduce IEEE 754 total order & NaN-counts #514

[alamb]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

• Related to Parquet spec (parquet.thrift) is inconsistent w.r.t. ColumnIndex + NaNs parquet-format#406

@JFinis has been working on a proposal to better store statistics for floating point values in Parquet. The most recent proposal is here

• PARQUET-2249: Introduce IEEE 754 total order & NaN-counts parquet-format#514

In order to change the format, there needs to be at least 2 open source implementations of a proposal

There is also some question (see this link from @tustvold ) about how complex this would be to implement / get right.

Describe the solution you'd like

I would like to implement a draft of the specification in apache/parquet-format#514 in arrow-rs to show it is possible and keep the Rust implementation on the leading edge of implementation.

Describe alternatives you've considered

• @etseidl has implemented the IEEE 754 total order in a draft PR here: [Not for Merge] PoC implementation of PARQUET-2249: Introduce IEEE 754 total order #7408

We would also need to implement the nan_count field along with filtering out nans when writing statistics for floats.

Some good tests would be to

1. Write floating point data (specified below) to a parquet file

2. Read the metadata back and verify min/max values and nan_count for the following cases

3. A column with no Nan values,

4. A column with a single +Nan value (should not appear in stats)

5. A column with a single -Nan value (should not appear in stats)

6. A column of Only Nan values

7. A column with Inf and some +/- Nans

8. A column with -Inf and some +/- Nans

Additional context

• Original JIRA issue: https://issues.apache.org/jira/browse/PARQUET-2249
• Mailing list discussion: https://lists.apache.org/thread/lzh0dvrvnsy8kvflvl61nfbn6f9js81s

[Xuanwo]

Let me take a look 😘

[alamb]

@Xuanwo to the open source resuce!

[etseidl]

I'd add that a complete test of the proposal should include how datafusion will consume the new info. I worked with @adriangb on apache/datafusion#15821 for this reason.

[alamb]

I'd add that a complete test of the proposal should include how datafusion will consume the new info. I worked with @adriangb on apache/datafusion#15821 for this reason.

I am wondering why, if Nans are no longer included in the min/max statistics, we would need to change DataFusion for Nans. It seems like maybe DataFusion would only need to be changed to handle -inf / +inf correctly

[tustvold]

For example, if you had a predicate of the form col > 34 it would only be valid to push this down if the nan_counts are zero. This is the major downside of excluding Nan's from the statistics, DF now needs to understand in what circumstances the min/max are "complete".

Edit: A simple solution would be to inflate the range to [NAN_MAX, NAN_MIN] if nan_counts are non-zero, which also highlights the downside of this formulation of nan_counts.

Edit edit: Although that also is potentially incorrect if the ranges are treated as being exact, which will also run into issues for pages where all values are NaN... It gets rather messy 😅

[alamb]

For example, if you had a predicate of the form col > 34 it would only be valid to push this down if the nan_counts are zero.

I think this is true if the system treats '-NanandNan` differently for comparisons

It does appear to be the case that DataFusion treats Nanand -Nan differently:

> create or replace  table foo(x double) as values ('nan'), (34.0), (34.1), ('-nan');
0 row(s) fetched.
Elapsed 0.001 seconds.

> select x, x>34 from foo;
+------+-------------------+
| x    | foo.x > Int64(34) |
+------+-------------------+
| NaN  | true              |
| 34.0 | false             |
| 34.1 | true              |
| NaN  | false             |
+------+-------------------+
4 row(s) fetched.
Elapsed 0.004 seconds.

However, Postgres does not 🤔

postgres=# create table foo (x real);
CREATE TABLE
postgres=# insert into foo values ('Nan'), (34.0), (34.1), ('-Nan');
INSERT 0 4
postgres=# select x, x>34 from foo;
  x   | ?column?
------+----------
  NaN | t
   34 | f
 34.1 | t
  NaN | t
(4 rows)

postgres=#

[alamb]

For example, if you had a predicate of the form col > 34 it would only be valid to push this down if the nan_counts are zero.

FWIW I think that is the case today anyways, but since we have no statistics on Nans DataFusion may incorrectly prune the whole thing

[tustvold]

I think this is true if the system treats '-NanandNan` differently for comparisons

Arrow-rs and by extension DataFusion consistently use total ordering for floating point comparisons.

FWIW I think that is the case today anyways, but since we have no statistics on Nans DataFusion may incorrectly prune the whole thing

DataFusion shouldn't push down floating point predicates, as there are so many buggy implementations (including I believe historical versions of arrow-rs) that the results will likely be incorrect. If DF is pushing down such predicates, it is a bug that should be fixed. FWIW this is the whole motivator for trying to standardise something that is both a clean slate free of buggy writers, whilst also being sufficiently simple that we don't end up with a repeat of the current issues.

[alamb]

DataFusion shouldn't push down floating point predicates, as there are so many buggy implementations (including I believe historical versions of arrow-rs) that the results will likely be incorrect.

I belive the issue is tracked in

• Pruning of floating point Parquet columns is incorrect when NaN is present datafusion#15812

[etseidl]

I belive the issue is tracked in

Yes, that issue grew out trying to implement apache/parquet-format#221. The thing I like most about that proposal vs nan_counts is that it makes a clean break from the underdefined past. If the new column order is detected, you can trust the statistics, if not, then you can't.