zhuqi-lucas opened a new issue, #10381:
URL: https://github.com/apache/arrow-rs/issues/10381

   cc @alamb
   
   ## Which issue does this help address?
   
   PR #8715 (merged 2025-11-14) shipped a new `OrdinalAssigner::ensure` 
invariant that rejects Parquet files whose row-groups disagree on whether 
`ordinal` is populated. Before #8715, the reader silently tolerated mixed 
row-group ordinal metadata (either uniform ordinals, no ordinals, or a mix), 
assigning sequential ordinals when needed.
   
   The `ordinal` field is defined as **optional** on `RowGroup` in the 
parquet-format Thrift spec, and there is no clause requiring it to be uniformly 
present or absent across a file. Any file that was valid pre-#8715 is still 
valid per spec today.
   
   ## Real-world impact
   
   Our product hit this on production reference-data parquet produced by a 
Go-based writer (Go parquet libraries commonly emit mixed-ordinal RowGroup 
metadata as they flush row-groups incrementally). The exact error:
   
   ```
   Parquet error: Inconsistent ordinal assignment: first_has_ordinal is set to
   false but row-group with actual ordinal 1 has rg_has_ordinal set to true
   ```
   
   Files that read fine on parquet 57.0.0 stopped reading once we bumped to 
57.1+ / 58.x. There is no downgrade path that keeps DataFusion 54/55 available.
   
   This is likely to bite other users too — cross-language Parquet ecosystems 
(Go parquet libs, older Java, custom writers) produce mixed-ordinal files in 
the wild.
   
   ## Why #8715 tightened this
   
   #8715 added row-number computation as a virtual column (for Delta Lake / 
Iceberg deletion vectors, per issue #7299). Correct row-numbering requires 
knowing each row-group's position in the file, so the PR added strict 
validation that ordinals are consistent.
   
   That reasoning holds only when a caller **actually uses** the row-number 
virtual column. For the vast majority of readers that never touch that feature, 
the pre-#8715 tolerant behavior was correct and useful.
   
   ## Proposal
   
   Add a reader option (e.g. 
`ArrowReaderOptions::with_lenient_row_group_ordinal(bool)`, default `false` to 
preserve #8715 semantics) that:
   
   - On `true`: `OrdinalAssigner::ensure` sequential-fills any missing ordinal 
and never errors on mismatch, matching pre-#8715 behavior.
   - On `false` (current default): keep the strict check.
   
   Callers that need row-number virtual columns keep the guarantee they rely 
on; callers that just want to read parquet get the wire-format tolerance that 
used to be default.
   
   **Alternative:** downgrade the hard error to a `tracing::warn!` unless the 
caller enables row-number virtual columns. This flips the default back to 
tolerant but keeps the invariant for the code path that actually depends on it.
   
   I'm happy to submit the PR if the direction is agreed. Wanted to open an 
issue first because this affects the semantics of the default reader path.
   
   ## References
   
   - Introducing PR: #8715
   - Motivating issue: #7299
   - Row-group `ordinal` field: `RowGroup.ordinal` in 
[parquet.thrift](https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift),
 marked `optional`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to