GH-47030: [C++][Parquet] Add setting to limit the number of rows written per page

[wgtmac]

Rationale for this change

Currently only page size is limited. We need to limit number of rows per page too.

What changes are included in this PR?

Add parquet::WriterProperties::max_rows_per_page(int64_t max_rows) to limit number of rows per data page.

Are these changes tested?

TODO

Are there any user-facing changes?

Yes, users are allowed to set this config value.

• GitHub Issue: [C++][Parquet] Add setting to limit the number of rows written per page #47030

[github-actions]

⚠️ GitHub issue #40730 has been automatically assigned in GitHub to PR creator.

[wgtmac]

Please check if this is the right direction. @pitrou @mapleFU @adamreeve

BTW, some existing test cases will break if I switch the default value to limit 20,000 rows per page. I'm not sure if it is a good idea to use 20,000 as the default value to surprise the downstream.

[github-actions]

⚠️ GitHub issue #47030 has been automatically assigned in GitHub to PR creator.

[adamreeve]

This approach looks correct to me thanks @wgtmac.

I'm not sure if it is a good idea to use 20,000 as the default value to surprise the downstream.

A default of 100k would still change behaviour though, and most of the time result in smaller pages being written. I think it probably makes sense to use 20k to align with Java and Rust, but it could be done as a separate PR if there are a lot of test changes needed.

I don't imagine this should break any downstream code, but we'd definitely want to call it out in the release notes as something for users to be aware of.

[adamreeve]

I should also mention that #47032 touches the same part of the code. It looks like the fix from that PR can easily be ported to this new code though.

[pitrou]

Should this PR be set to draft until it's ready?

[wgtmac]

I think this is ready for review. @pitrou

[pitrou]

Thanks. I'm on vacation so I'm going to be a bit slow, sorry!

[wgtmac]

IIUC, this PR may slightly affect CDC. Let me know if you have any feedback. @kszucs

[kszucs]

It depends on the value of the length limit. The current size limit is 1MB calculated after encoding while CDC default size range calculated on the logical values before encoding is between 256KB and 1MB. CDC chunking is applied before the size limit check, so using the default parameters should trigger a data page write before the size limit check. If the size limit is set to a smaller value, then there will be two data pages, a larger one cut at the size limit, and a smaller one cut at the CDC boundary because the CDC hash is not being reset if size limit is triggered. So basically there are two cases:

a) size limits are bigger than the cdc limits, then the pages are cut earlier than the size limits would happen:
page1 [cdc-cut] page2 [cdc-cut] page3
b) size limits are smaller than the cdc limits, then the previous cdc cuts will happen nonetheless, but the pages are going to be split further according to the size limits
page1 [cdc-cut] page2/a [size-cut] page2/b [cdc-cut] page3

So in theory it shouldn't affect the CDC's effectiveness. We can also check this before merging using https://github.com/huggingface/dataset-dedupe-estimator

[kszucs]

Maybe we should have this feature as opt-in? Otherwise should we choose a bigger value to have the data page size triggered first?

Given the parquet type sizes we could end up much smaller data pages (even before encoding) than 1MB which could be unexpected to the users and also increase the overall metadata size:

  - BOOLEAN: 1 bit boolean
  - INT32: 32 bit signed ints
  - INT64: 64 bit signed ints
  - INT96: 96 bit signed ints (deprecated; only used by legacy implementations)
  - FLOAT: IEEE 32-bit floating point values
  - DOUBLE: IEEE 64-bit floating point values
  - BYTE_ARRAY: arbitrarily long byte arrays
  - FIXED_LEN_BYTE_ARRAY: fixed length byte arrays

[wgtmac]

Right, defaulting to 20000 will definitely create smaller pages of numeric types. 20000 is also used by parquet-java and arrow-rs and considered to be a good value per the discussion at https://lists.apache.org/thread/vsxmbvnx9gy5414cfo25mnwcj17h1xyp