parquet: Convert page indexes to Vec<Vec<Option<T>>>

[etseidl]

Which issue does this PR close?

• Closes Explore changing the form of the Parquet page indexes in ParquetMetaData #8818.

Rationale for this change

See issue, but in short the page indexes should allow for missing column and page indexes in a more natural way.

What changes are included in this PR?

The major change is converting ParquetOffsetIndex from Vec<Vec<OffsetIndexMetaData>> to Vec<Vec<Option<OffsetIndexMetaData>>> and ParquetColumnIndex from Vec<Vec<ColumnIndexMetaData>> to Vec<Vec<Option<ColumnIndexMetaData>>>.

The ColumnIndexMetaData::NONE variant is now redundant and is removed.

Then needed changes to tests and places where these indexes are used.

Are these changes tested?

Should be covered by existing tests

Are there any user-facing changes?

Yes, changes are made to the public page index API.

Note that Claude Sonnet 4.5 was used to generate parts of this PR (most notable changes in statsistics.rs), but those changes have been modified by me, so I own the final product.

[etseidl]

This and write_column_index below could be refactored to return Result<()>

[etseidl]

The changes here are the big change...the rest is dealing with the consequences

[alamb]

I have always found this structure to be very confusing (as it is a Vec of Vecs). Adding Option makes it even more confusing in my mind.

What would you think about at least encapsulating the PageIndex into a structure of its own (rather than two parallel structure)?

struct PageIndex {
  column_indexes: Vec<Vec<Option<ColumnIndexMetaData>>>,
  offset_indexes:  Vec<Vec<Option<OffsetIndexMetaData>>>,
}

🤔

Then we could add accessors like

if let Some(offset_index) = page_index.offset_index(rg_idx) { 
  // use offset index for rg_idx
}

That might also allow us to tweak the internal representation of these indexes to support options, etc without breaking the structure again

[etseidl]

There is a great deal of repetition in this module

[etseidl]

@alamb this is me scratching an old itch. I think it's ready to go, but am not sure what the downstream implications are. Let me know if you think this can go into 60.0.0.

Note that most of the changes are either highly repetitive (like in statistics.rs) or changes to make tests work. The actual code changes are pretty small.

[alamb]

Thanks -- I will check this out later today

[alamb]

Thank you @etseidl - I think this is a great idea and a long time coming. If we are going to mess with the APIs I think it may be worth considering some more drastic changes, but what you have here I think is the minimum change we would need

[alamb]

If we are going to change the page index representation (and force a breaking change downstream on the users) I wonder if we can think bigger than just adding an Option here and making it align with the parquet-format names

For example what do you think about making it a struct so that we have a better chance of evolving it over time (and make it easier to document)?

For example:

https://github.com/apache/parquet-format/blob/2076361bb64e2de9ca6a8d06eda025a6fa4e9df6/src/main/thrift/parquet.thrift#L1261

struct ParquetOffsetIndex {
  page_locations: Vec<ParquetPageLocation>,
  unencoded_byte_array_data_bytes: Option<Vec<i64>,
}

And https://github.com/apache/parquet-format/blob/2076361bb64e2de9ca6a8d06eda025a6fa4e9df6/src/main/thrift/parquet.thrift#L1236

struct ParquetPageLocation {
  offset: i64,
  compressed_page_size: i32,
  first_row_index: i64,
 }

[alamb]

We could also just do something slightly more encapsulated rather than a typedef

struct ParquetOffsetIndex {
  inner: Vec<Vec<Option<OffsetIndexMetaData>>>;
}

🤔

[alamb]

I have always found this structure to be very confusing (as it is a Vec of Vecs). Adding Option makes it even more confusing in my mind.

What would you think about at least encapsulating the PageIndex into a structure of its own (rather than two parallel structure)?

struct PageIndex {
  column_indexes: Vec<Vec<Option<ColumnIndexMetaData>>>,
  offset_indexes:  Vec<Vec<Option<OffsetIndexMetaData>>>,
}

🤔

Then we could add accessors like

if let Some(offset_index) = page_index.offset_index(rg_idx) { 
  // use offset index for rg_idx
}

That might also allow us to tweak the internal representation of these indexes to support options, etc without breaking the structure again

[etseidl]

Thanks @alamb. Let me think some about your suggestions. I'm all for further encapsulating things as I too struggle with these two structures. My only hesitation is how that will impact the stats converter, but I think it will be fine so long as the converter has access to the vecs.

It also might be worth doing this in two PRs...this one to effect the additional Option change, and then a second to introduce a new PageIndex struct.

[alamb]

It also might be worth doing this in two PRs...this one to effect the additional Option change, and then a second to introduce a new PageIndex struct.

I like this idea -- and it keeps the changes somewhat scoped and doesn't require a refactor the world type PR)