Parquet: tolerate mixed row-group ordinal metadata on read

parquet/src/arrow/array_reader/row_number.rs

@@ -47,14 +47,30 @@ impl RowNumberReader {
         // This is O(M) where M is the total number of row groups in the file
         let mut ordinal_to_offset: HashMap<i16, i64> = HashMap::new();
         let mut first_row_index: i64 = 0;
+        let mut missing_ordinals: usize = 0;
 
         for rg in parquet_metadata.row_groups() {
             if let Some(ordinal) = rg.ordinal() {
                 ordinal_to_offset.insert(ordinal, first_row_index);
+            } else {
+                missing_ordinals += 1;
             }
             first_row_index += rg.num_rows();
         }
 
+        // Mixed ordinals: refuse to compute row numbers for the whole file,
+        // even if every *selected* row group carries an ordinal. Otherwise the
+        // same file would yield row numbers or an error depending on which row
+        // groups a query's pruning happens to select.
+        if missing_ordinals > 0 && !ordinal_to_offset.is_empty() {
+            return Err(ParquetError::General(format!(
+                "Cannot compute row numbers: file has inconsistent row-group \
+                 ordinals ({} of {} row groups are missing the ordinal field)",
+                missing_ordinals,
+                parquet_metadata.num_row_groups(),
+            )));
+        }
+
         // Pass 2: Build ranges in the order specified by the row_groups iterator
         // This is O(N) where N is the number of selected row groups
         // This preserves the user's requested order instead of sorting by ordinal
@@ -182,6 +198,15 @@ mod tests {
     }
 
     fn create_test_parquet_metadata(row_groups: Vec<(i16, i64)>) -> ParquetMetaData {
+        create_test_parquet_metadata_opt(
+            row_groups
+                .into_iter()
+                .map(|(ordinal, num_rows)| (Some(ordinal), num_rows))
+                .collect(),
+        )
+    }
+
+    fn create_test_parquet_metadata_opt(row_groups: Vec<(Option<i16>, i64)>) -> ParquetMetaData {
         let schema_descr = create_test_schema();
 
         let mut row_group_metas = vec![];
@@ -192,14 +217,14 @@ mod tests {
                 .map(|col| ColumnChunkMetaData::builder(col.clone()).build().unwrap())
                 .collect();
 
-            let row_group = RowGroupMetaData::builder(schema_descr.clone())
+            let mut builder = RowGroupMetaData::builder(schema_descr.clone())
                 .set_num_rows(num_rows)
-                .set_ordinal(ordinal)
                 .set_total_byte_size(100)
-                .set_column_metadata(columns)
-                .build()
-                .unwrap();
-            row_group_metas.push(row_group);
+                .set_column_metadata(columns);
+            if let Some(ordinal) = ordinal {
+                builder = builder.set_ordinal(ordinal);
+            }
+            row_group_metas.push(builder.build().unwrap());
         }
 
         let total_rows: i64 = row_group_metas.iter().map(|rg| rg.num_rows()).sum();
@@ -302,4 +327,47 @@ mod tests {
         assert_eq!(reader.read_records(4).unwrap(), 4);
         assert_eq!(consume_row_numbers(&mut reader), vec![3, 4, 5, 6]);
     }
+
+    /// A file with *mixed* row-group ordinals must never produce row numbers —
+    /// even when every selected row group carries an ordinal. Otherwise the
+    /// same file would succeed or fail depending on which row groups a query's
+    /// pruning selects.
+    #[test]
+    fn test_mixed_ordinals_always_error() {
+        let metadata = create_test_parquet_metadata_opt(vec![
+            (Some(0), 2), // has ordinal
+            (None, 2),    // missing
+            (Some(2), 2), // has ordinal
+        ]);
+
+        // Select only row groups WITH ordinals — must still fail.
+        let selected = vec![&metadata.row_groups()[0], &metadata.row_groups()[2]];
+        let Err(err) = RowNumberReader::try_new(&metadata, selected.into_iter()) else {
+            panic!("mixed ordinals with ordinal-only selection must fail");
+        };
+        assert!(
+            err.to_string().contains("inconsistent row-group ordinals"),
+            "unexpected error: {err}"
+        );
+
+        // Selecting a row group without an ordinal fails too.
+        let selected = vec![&metadata.row_groups()[1]];
+        assert!(RowNumberReader::try_new(&metadata, selected.into_iter()).is_err());
+    }
+
+    /// A file where *no* row group carries an ordinal fails for any selection
+    /// (fresh decode sequential-fills, so this only occurs for
+    /// programmatically-built metadata).
+    #[test]
+    fn test_no_ordinals_error() {
+        let metadata = create_test_parquet_metadata_opt(vec![(None, 2), (None, 2)]);
+        let selected = vec![&metadata.row_groups()[0]];
+        let Err(err) = RowNumberReader::try_new(&metadata, selected.into_iter()) else {
+            panic!("row numbers without any ordinals must fail");
+        };
+        assert!(
+            err.to_string().contains("missing ordinal field"),
+            "unexpected error: {err}"
+        );
+    }
 }

parquet/src/arrow/arrow_reader/mod.rs

@@ -5614,6 +5614,85 @@ pub(crate) mod tests {
         Ok(())
     }
 
+    /// A file with *mixed* row-group ordinal metadata (spec-valid — the
+    /// `RowGroup.ordinal` thrift field is optional; Go parquet writers emit
+    /// such files) must read fine without row numbers, and must fail
+    /// deterministically with them — even when every *selected* row group
+    /// carries an ordinal. See #10381.
+    #[test]
+    fn test_mixed_row_group_ordinals() -> Result<()> {
+        use crate::file::metadata::{ParquetMetaDataReader, RowGroupMetaData};
+
+        // 100 rows split across 4 row groups of 25
+        let array = Int64Array::from_iter_values(5000..5100);
+        let batch = RecordBatch::try_from_iter([("col", Arc::new(array) as ArrayRef)])?;
+        let mut buffer = Vec::new();
+        let props = WriterProperties::builder()
+            .set_max_row_group_row_count(Some(25))
+            .build();
+        let mut writer = ArrowWriter::try_new(&mut buffer, batch.schema().clone(), Some(props))?;
+        for batch_chunk in (0..10).map(|i| batch.slice(i * 10, 10)) {
+            writer.write(&batch_chunk)?;
+        }
+        writer.close()?;
+        let buffer = Bytes::from(buffer);
+
+        // Strip the ordinal from row group 1 to simulate a mixed-ordinal
+        // writer (the builder starts with no ordinal; copy everything else).
+        let metadata = ParquetMetaDataReader::new().parse_and_finish(&buffer)?;
+        let schema_descr = metadata.file_metadata().schema_descr_ptr();
+        let mut row_groups = metadata.row_groups().to_vec();
+        let stripped = row_groups[1].clone();
+        let mut builder = RowGroupMetaData::builder(schema_descr)
+            .set_num_rows(stripped.num_rows())
+            .set_total_byte_size(stripped.total_byte_size())
+            .set_sorting_columns(stripped.sorting_columns().cloned())
+            .set_column_metadata(stripped.columns().to_vec());
+        if let Some(offset) = stripped.file_offset() {
+            builder = builder.set_file_offset(offset);
+        }
+        row_groups[1] = builder.build()?;
+        assert_eq!(row_groups[1].ordinal(), None);
+        let metadata = Arc::new(metadata.into_builder().set_row_groups(row_groups).build());
+
+        // Plain read (no row numbers): succeeds and returns all values.
+        let arrow_metadata =
+            ArrowReaderMetadata::try_new(Arc::clone(&metadata), ArrowReaderOptions::new())?;
+        let reader =
+            ParquetRecordBatchReaderBuilder::new_with_metadata(buffer.clone(), arrow_metadata)
+                .build()?;
+        let values: Vec<i64> = reader
+            .flat_map(|batch| {
+                let batch = batch.expect("could not read batch");
+                batch
+                    .column(0)
+                    .as_primitive::<types::Int64Type>()
+                    .values()
+                    .to_vec()
+            })
+            .collect();
+        assert_eq!(values, (5000..5100).collect::<Vec<_>>());
+
+        // Row-number read: fails deterministically, even when selecting only
+        // row groups that DO carry ordinals.
+        let row_number_field = Arc::new(
+            Field::new("row_number", ArrowDataType::Int64, false).with_extension_type(RowNumber),
+        );
+        let options = ArrowReaderOptions::new().with_virtual_columns(vec![row_number_field])?;
+        let arrow_metadata = ArrowReaderMetadata::try_new(Arc::clone(&metadata), options)?;
+        let result = ParquetRecordBatchReaderBuilder::new_with_metadata(buffer, arrow_metadata)
+            .with_row_groups(vec![0]) // row group 0 has an ordinal
+            .build()
+            .and_then(|mut reader| reader.next().transpose().map_err(|e| e.into()));
+        let err = result.expect_err("row numbers over mixed ordinals must fail");
+        assert!(
+            err.to_string().contains("inconsistent row-group ordinals"),
+            "unexpected error: {err}"
+        );
+
+        Ok(())
+    }
+
     #[derive(Debug, PartialEq)]
     struct ValuesAndRowNumbers {
         values: Vec<i64>,

parquet/src/file/metadata/thrift/encryption.rs

@@ -163,10 +163,21 @@ fn row_group_from_encrypted_thrift(
                 }
             };
 
+            // The ordinal is part of the AAD for encrypted column metadata.
+            // It can be missing here only for files with *mixed* row-group
+            // ordinals, which decode leaves untouched — fail cleanly rather
+            // than panic (see `ensure_row_group_ordinals`).
+            let rg_ordinal = rg.ordinal.ok_or_else(|| {
+                general_err!(
+                    "Row group ordinal is required to decrypt column metadata for \
+                     column '{}', but the file's row-group ordinals are inconsistent",
+                    d.path().string()
+                )
+            })?;
             let column_aad = crate::encryption::modules::create_module_aad(
                 decryptor.file_aad(),
                 crate::encryption::modules::ModuleType::ColumnMetaData,
-                rg.ordinal.unwrap() as usize,
+                rg_ordinal as usize,
                 i,
                 None,
             )?;