PARQUET-372: Do not write stats larger than 4k.

[rdblue]

This updates the stats conversion to check whether the min and max
values for page stats are larger than 4k. If so, no statistics for a
page are written.

[rdblue]

@spena, @julienledem, @isnotinvain what do you guys think of this change? We're hitting cases where the min/max stats are about the size of a page, so we end up with pages 3x larger than they should be.

I think the best solution is to not write stats over some limit, which I've set at 4k here. Scanning would be quick when this is the case because if values are that large there wouldn't be many of them in a page. Also, since the large values must be min and/or max, we don't expect many cases where just one huge value prevents stats from being written.

I had suggested on the ticket that we might want to modify the value, but that gets messy and wouldn't allow us to satisfy queries from stats alone (select max(col) from table).

[rdblue]

Test failures are again due to flaky memory manager tests, fixed in #263.

[rdblue]

I looked at stats merging and I think this is safe because statistics are aggregated before they are written to the file. A column chunk will accumulate the min and max values for all pages accurately, but when individual pages are written, the stats may be discarded instead of written to the file.

[spena]

@rdblue The patch looks good.

I just have a question about the limit for the statistics. Why not limiting the size to the current page size used on the row group (or pagesize / 4 or something)? If a user ends setting a big page size, then they won't have the advantage of using the statistics to check whether the row group should be read or not.

[rdblue]

Thanks for taking a look, @spena. There are a couple of motivations for the 4k static limit. First, I'd like to keep a hard limit on the page size so they don't get unexpectedly large again. The other reason is just to keep the code simple and not need to pass the current page size through unless we know it's going to be beneficial. We can always make it more fancy later.

[rdblue]

@julienledem, I think this (or something similar) should go into 1.9.0. Could you have a look at it?

[rdblue]

I've rebased this and added a comment to explain why stats are ignored rather than truncated.

@julienledem could you take a look when you have time? Thanks!

[lw-lin]

hi @rdblue
Although I wonder if we could just stop updating the stats once we encounter large-sized max+min (to save the max/min compassion for upcoming writes), I'm +1 for this. I've looked through the code changes as well as the added unit tests -- LGTM.

[danielcweeks]

Is it possible to have null count without min/max. This is a nit, but engines could use null count even without min/max present.

[rdblue]

I just looked into this. There's nothing preventing us from doing this in the format, but the implementation assumes that there will be non-null min and max if numNulls != numRows. I think it makes sense to add this as a follow-up instead of making this patch larger.

[danielcweeks]

+1 I agree with all of the follow up to my questions.