feat: add row_group_is_[max/min]_value_exact to StatisticsConverter

[CookiePieWw]

Which issue does this PR close?

Rationale for this change

As described in apache/datafusion#15976 (comment), we can expose the is_[max/min]_value_exact flags in StatisticsConverter in order to justify whether the stats are exact.

What changes are included in this PR?

Add row_group_is_[max/min]_value_exact to StatisticsConverter, also with some changes in the corresponding test files.

Are there any user-facing changes?

[alamb]

Thank you @CookiePieWw -- this looks really close

I think we just need to tweak the tests a bit and this PR will be good to go

[alamb]

This will not be needed after #7578
(but that won't be merged for a while)

[alamb]

Thank you @CookiePieWw -- this is looking close

[etseidl]

Suggested change

	Some(&("c".repeat(64) + "3")),
	None,

Try throwing a null in the first batch, that seems to clear up the issue with empty string showing up in the stats. It seems like the schema is inferred from the first batch, so all values are not nullable. Putting a null in the first batch fixes that.

[CookiePieWw]

Sounds reasonable for me. But I still wonder if its as expected that a first batch without null values will cause the Nones in following batches are converted into empty strings.

[etseidl]

I'll have to study the API used here in more detail, but I would assume one would usually provide a schema prior to writing batches. The issue here AFAICT is that a null value is written, but the schema says values are not nullable, so the null is converted to an empty string. I think I'd prefer an error be thrown in this instance.

[etseidl]

Ahh, with sleep I now see the schema is set in make_test_file_rg...the first batch is used.

arrow-rs/parquet/tests/arrow_reader/mod.rs

Lines 1037 to 1039 in 0ae9f66

	let schema = batches[0].schema();
	let mut writer = ArrowWriter::try_new(&mut output_file, schema, Some(props)).unwrap();

Perhaps a note should be added that if nulls are desired, they need to be included in the first batch.

[CookiePieWw]

Ah thanks! So it's not a bug then.

[etseidl]

Thanks @CookiePieWw! Looks good now.

[CookiePieWw]

Hi @alamb , since #7578 will not be merged till next major release, seems we need to wait for it and then remove the code setting default max_statistics_truncate_length here.

Separately, regarding the handling of None values in a non-nullable schema (thanks to @etseidl for catching this!), we currently convert None to empty strings implicitly. Would it be better to throw an error in such cases, or is this behavior by design?

[alamb]

Separately, regarding the handling of None values in a non-nullable schema (thanks to @etseidl for catching this!), we currently convert None to empty strings implicitly. Would it be better to throw an error in such cases, or is this behavior by design?

I don't think it is by design per se -- it is due to a lack of error checking

However, given that we don't have a strong discipline around verifying the nullable flag in Field I am somewhat wary about potentially adding additional checking

[alamb]

Thanks again @CookiePieWw and @etseidl