dhruvarya-db commented on code in PR #3388:
URL: https://github.com/apache/iceberg-rust/pull/3388#discussion_r4252133569


##########
crates/iceberg/src/arrow/manifest.rs:
##########
@@ -0,0 +1,1315 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Reading manifest entries into Arrow.
+
+use std::collections::HashMap;
+use std::sync::Arc;
+
+use arrow_arith::boolean::{and, is_null};
+use arrow_array::builder::Int64Builder;
+use arrow_array::cast::AsArray;
+use arrow_array::types::{TimestampMicrosecondType, TimestampNanosecondType};
+use arrow_array::{
+    Array, ArrayRef, BooleanArray, Int32Array, Int64Array, ListArray, 
MapArray, RecordBatch,
+    StructArray, new_null_array,
+};
+use arrow_avro::reader::ReaderBuilder;
+use arrow_avro::schema::AvroSchema;
+use arrow_cast::{CastOptions, cast_with_options};
+use arrow_ord::cmp::eq;
+use arrow_schema::{ArrowError, DataType, Field, Fields, TimeUnit};
+use arrow_select::zip::zip;
+use parquet::arrow::PARQUET_FIELD_ID_META_KEY;
+use serde_json::Value as JsonValue;
+
+use crate::arrow::schema_to_arrow_schema;
+use crate::error::invalid_data;
+use crate::spec::{
+    ManifestContentType, ManifestFile, ManifestMetadata, ManifestStatus, 
Schema,
+    manifest_entry_fields,
+};
+use crate::{Error, Result};
+
+/// Field ID of `data_file`.
+const DATA_FILE_FIELD_ID: i64 = 2;
+/// Field ID of `data_file.partition`.
+const PARTITION_FIELD_ID: i64 = 102;
+/// Field ID of `data_file.content`.
+const CONTENT_FIELD_ID: i32 = 134;
+/// Field ID of `sequence_number`.
+const SEQUENCE_NUMBER_FIELD_ID: i32 = 3;
+/// Field ID of `file_sequence_number`.
+const FILE_SEQUENCE_NUMBER_FIELD_ID: i32 = 4;
+
+/// Reads the entries of a manifest file into a [`RecordBatch`] with one row
+/// per entry.
+///
+/// `bytes` is the manifest file and `manifest_file` is its entry in the
+/// manifest list. The schema is the spec's `manifest_entry` struct for the
+/// latest format version, with field IDs in the Arrow field metadata, so
+/// manifests of every format version read into the same schema apart from
+/// `data_file.partition`, which follows the manifest's partition spec.
+///
+/// Fields are matched to the manifest's Avro schema by field ID. A v1
+/// manifest reads with the spec's v2+ defaults, and null snapshot IDs,
+/// sequence numbers, and first row IDs are inherited from `manifest_file`.
+/// Lower and upper bounds keep the binary single-value serialization they were
+/// written with.
+///
+/// # Example
+///
+/// ```
+/// use iceberg::arrow::read_manifest_entries;
+/// use iceberg::spec::{ManifestContentType, ManifestFile};
+///
+/// let bytes = std::fs::read(concat!(
+///     env!("CARGO_MANIFEST_DIR"),
+///     "/testdata/manifests/pyiceberg-v2-data.avro"
+/// ))?;
+/// let manifest_file = ManifestFile {
+///     manifest_path: "s3://bucket/metadata/manifest.avro".to_string(),
+///     manifest_length: bytes.len() as i64,
+///     partition_spec_id: 0,
+///     content: ManifestContentType::Data,
+///     sequence_number: 1,
+///     min_sequence_number: 1,
+///     added_snapshot_id: 1,
+///     added_files_count: None,
+///     existing_files_count: None,
+///     deleted_files_count: None,
+///     added_rows_count: None,
+///     existing_rows_count: None,
+///     deleted_rows_count: None,
+///     partitions: None,
+///     key_metadata: None,
+///     first_row_id: None,
+/// };
+///
+/// let batch = read_manifest_entries(&bytes, &manifest_file)?;
+/// assert_eq!(batch.num_rows(), 2);
+/// # Ok::<(), Box<dyn std::error::Error>>(())
+/// ```
+pub fn read_manifest_entries(bytes: &[u8], manifest_file: &ManifestFile) -> 
Result<RecordBatch> {
+    // usize::MAX decodes the whole manifest into one batch. First row ID
+    // inheritance is a running sum over every entry in the file, and an empty
+    // manifest still returns its schema. arrow-avro sizes its buffers
+    // independently of the batch size, so this doesn't preallocate.
+    let builder = || ReaderBuilder::new().with_batch_size(usize::MAX);
+    let mut reader = builder().build(bytes).map_err(invalid_manifest)?;
+    let header = reader.avro_header();
+    let metadata: HashMap<String, Vec<u8>> = header
+        .metadata()
+        .map(|(key, value)| (String::from_utf8_lossy(key).into_owned(), 
value.to_vec()))
+        .collect();
+    let metadata = ManifestMetadata::parse(&metadata)?;
+    let mut avro_schema: JsonValue = serde_json::from_slice(
+        header
+            .get("avro.schema")
+            .ok_or_else(|| invalid_data!("manifest has no avro.schema"))?,
+    )?;
+    let partition_type = metadata
+        .partition_spec()
+        .partition_type(metadata.schema())?;
+    let schema = Arc::new(schema_to_arrow_schema(
+        &Schema::builder()
+            .with_fields(manifest_entry_fields(&partition_type))
+            .build()?,
+    )?);
+
+    // TODO(#3257): arrow-avro 59 can't decode a record with no fields, which
+    // is the partition of every unpartitioned manifest, so project it out and
+    // build it in `missing_field`. arrow-avro 60 fixes this
+    // (apache/arrow-rs#10771).
+    if remove_empty_partition(&mut avro_schema) {
+        reader = builder()
+            .with_reader_schema(AvroSchema::new(avro_schema.to_string()))
+            .build(bytes)
+            .map_err(invalid_manifest)?;
+    }
+
+    let Some(batch) = reader.next() else {
+        return Ok(RecordBatch::new_empty(schema));
+    };

Review Comment:
   [nit] Would it make sense to add a defensive check here, even though we know 
that usize::Max should do the job?
   
   ```
   // batch_size=usize::MAX should result in only one batch being created.
   if reader.next().is_some() {
       return Err(invalid_data!("arrow-avro split the manifest into more than 
one batch"));
   }
   ```



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to